diff --git a/src/tools/multi_step_coordinator.cpp b/src/tools/multi_step_coordinator.cpp index 713babf..f655968 100644 --- a/src/tools/multi_step_coordinator.cpp +++ b/src/tools/multi_step_coordinator.cpp @@ -5,6 +5,18 @@ #include #include +namespace { +// Terminal steps never reach the tool-call feedback path below, so their +// assistant reply has to be appended here for response_messages to satisfy +// its contract of holding the complete conversation continuation. +void append_terminal_assistant_reply(const ai::GenerateResult& step_result, + ai::Messages& response_messages) { + if (!step_result.text.empty()) { + response_messages.push_back(ai::Message::assistant(step_result.text)); + } +} +} // namespace + namespace ai { GenerateResult MultiStepCoordinator::execute_multi_step( @@ -101,6 +113,7 @@ GenerateResult MultiStepCoordinator::execute_multi_step( step_result.finish_reason == kFinishReasonLength || step_result.finish_reason == kFinishReasonContentFilter || step_result.finish_reason == kFinishReasonError) { + append_terminal_assistant_reply(step_result, response_messages); break; } @@ -138,6 +151,7 @@ GenerateResult MultiStepCoordinator::execute_multi_step( } else { // No tool calls and a non-terminal finish reason: nothing to feed back, // so we're done. + append_terminal_assistant_reply(step_result, response_messages); break; } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1178bf2..7ece7c7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -7,6 +7,7 @@ add_executable(ai_tests unit/openai_stream_test.cpp unit/anthropic_stream_test.cpp unit/openai_embeddings_test.cpp + unit/multi_step_coordinator_test.cpp # Integration tests integration/openai_integration_test.cpp diff --git a/tests/unit/multi_step_coordinator_test.cpp b/tests/unit/multi_step_coordinator_test.cpp new file mode 100644 index 0000000..611969a --- /dev/null +++ b/tests/unit/multi_step_coordinator_test.cpp @@ -0,0 +1,84 @@ +#include "ai/tools.h" +#include "ai/types/enums.h" +#include "ai/types/generate_options.h" + +#include +#include +#include + +#include + +namespace ai { +namespace test { + +namespace { + +GenerateResult make_tool_call_step() { + GenerateResult result; + result.finish_reason = kFinishReasonToolCalls; + result.tool_calls.emplace_back("call_1", "add", + JsonValue{{"a", 3}, {"b", 5}}); + result.tool_results.emplace_back( + "call_1", "add", JsonValue{{"a", 3}, {"b", 5}}, JsonValue{{"sum", 8}}); + return result; +} + +GenerateResult make_stop_step(const std::string& text) { + GenerateResult result; + result.text = text; + result.finish_reason = kFinishReasonStop; + return result; +} + +GenerateOptions make_options() { + GenerateOptions options("test-model", "What is 3 + 5?"); + options.max_steps = 4; + return options; +} + +} // namespace + +// Regression test: the terminal assistant reply used to be dropped from +// response_messages because only the tool-call feedback path appended to the +// accumulator, so callers continuing the conversation lost the final answer. +TEST(MultiStepCoordinatorTest, TerminalReplyRecordedAfterToolStep) { + std::vector scripted = {make_tool_call_step(), + make_stop_step("The result is 8")}; + std::size_t call = 0; + + auto result = MultiStepCoordinator::execute_multi_step( + make_options(), [&](const GenerateOptions&) { return scripted[call++]; }); + + ASSERT_EQ(result.steps.size(), 2u); + ASSERT_EQ(result.response_messages.size(), 3u); + const auto& reply = result.response_messages.back(); + EXPECT_EQ(reply.role, kMessageRoleAssistant); + EXPECT_EQ(reply.get_text(), "The result is 8"); +} + +TEST(MultiStepCoordinatorTest, ImmediateStopRecordsAssistantReply) { + auto result = MultiStepCoordinator::execute_multi_step( + make_options(), + [](const GenerateOptions&) { return make_stop_step("Hello"); }); + + ASSERT_EQ(result.response_messages.size(), 1u); + EXPECT_EQ(result.response_messages.back().role, kMessageRoleAssistant); + EXPECT_EQ(result.response_messages.back().get_text(), "Hello"); +} + +TEST(MultiStepCoordinatorTest, EmptyTerminalTextAppendsNoMessage) { + std::vector scripted = {make_tool_call_step(), + make_stop_step("")}; + std::size_t call = 0; + + auto result = MultiStepCoordinator::execute_multi_step( + make_options(), [&](const GenerateOptions&) { return scripted[call++]; }); + + // The assistant tool-call turn and the tool results stay, but no empty + // assistant reply is appended. + ASSERT_EQ(result.response_messages.size(), 2u); + EXPECT_NE(result.response_messages.back().role, kMessageRoleAssistant); +} + +} // namespace test +} // namespace ai