Skip to content

Commit 131f1e8

Browse files
authored
Add test to validate duplicate tool execution in multi-step mode (#26) (#29)
Created integration test that demonstrates tools being executed twice when max_steps > 1. The test uses execution counters to track tool invocations and asserts that each tool call is executed exactly once. Test currently fails, showing that tools are executed both in base_provider_client.cpp and multi_step_coordinator.cpp, validating the bug reported in issue #26.
1 parent 315f17b commit 131f1e8

5 files changed

Lines changed: 239 additions & 16 deletions

File tree

src/tools/multi_step_coordinator.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,9 @@ GenerateResult MultiStepCoordinator::execute_multi_step(
102102

103103
if (step_result.finish_reason == kFinishReasonToolCalls &&
104104
step_result.has_tool_calls()) {
105-
// Execute tools and prepare for next step
106-
// Use sequential execution to avoid thread-safety issues
107-
std::vector<ToolResult> tool_results =
108-
ToolExecutor::execute_tools_with_options(step_result.tool_calls,
109-
initial_options, false);
110-
111-
// Store tool results in the step
112-
final_result.steps.back().tool_results = tool_results;
105+
// Tools have already been executed by generate_text_single_step()
106+
// Use the tool results from step_result instead of re-executing
107+
const std::vector<ToolResult>& tool_results = step_result.tool_results;
113108

114109
// Check if all tools failed
115110
bool all_failed = true;

src/tools/tool_executor.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,9 @@ bool ToolExecutor::validate_json_schema(const JsonValue& data,
185185
std::string expected_type = schema["type"];
186186

187187
if (expected_type == "object") {
188-
if (!data.is_object())
188+
if (!data.is_object()) {
189189
return false;
190+
}
190191

191192
// Check required properties
192193
if (schema.contains("required") && schema["required"].is_array()) {
@@ -270,9 +271,9 @@ std::string generate_tool_call_id() {
270271
for (int i = 0; i < 24; ++i) {
271272
int val = dis(gen);
272273
if (val < 10) {
273-
id += char('0' + val);
274+
id += static_cast<char>('0' + val);
274275
} else {
275-
id += char('a' + val - 10);
276+
id += static_cast<char>('a' + val - 10);
276277
}
277278
}
278279

tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_executable(ai_tests
1212
integration/anthropic_integration_test.cpp
1313
integration/tool_calling_integration_test.cpp
1414
integration/clickhouse_integration_test.cpp
15+
integration/multi_step_duplicate_execution_test.cpp
1516

1617
# Utility classes
1718
utils/mock_openai_client.cpp
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
#include "../utils/test_fixtures.h"
2+
#include "ai/anthropic.h"
3+
#include "ai/logger.h"
4+
#include "ai/openai.h"
5+
#include "ai/tools.h"
6+
#include "ai/types/generate_options.h"
7+
#include "ai/types/tool.h"
8+
9+
#include <atomic>
10+
#include <memory>
11+
#include <optional>
12+
#include <string>
13+
14+
#include <gmock/gmock.h>
15+
#include <gtest/gtest.h>
16+
17+
namespace ai {
18+
namespace test {
19+
20+
// Test for issue #26: Tools executed twice in multi-step mode
21+
// https://github.com/ClickHouse/ai-sdk-cpp/issues/26
22+
23+
// Parameterized test class for multi-step tool execution
24+
class MultiStepDuplicateExecutionTest
25+
: public ::testing::TestWithParam<std::string> {
26+
protected:
27+
void SetUp() override {
28+
std::string provider = GetParam();
29+
30+
if (provider == "openai") {
31+
const char* api_key = std::getenv("OPENAI_API_KEY");
32+
if (api_key) {
33+
use_real_api_ = true;
34+
client_ = ai::openai::create_client(api_key);
35+
model_ = ai::openai::models::kGpt4oMini;
36+
} else {
37+
use_real_api_ = false;
38+
}
39+
} else if (provider == "anthropic") {
40+
const char* api_key = std::getenv("ANTHROPIC_API_KEY");
41+
if (api_key) {
42+
use_real_api_ = true;
43+
client_ = ai::anthropic::create_client(api_key);
44+
model_ = ai::anthropic::models::kClaudeSonnet35;
45+
} else {
46+
use_real_api_ = false;
47+
}
48+
}
49+
}
50+
51+
bool use_real_api_ = false;
52+
std::optional<ai::Client> client_;
53+
std::string model_;
54+
};
55+
56+
// Test that tools are executed only once per step in multi-step mode
57+
TEST_P(MultiStepDuplicateExecutionTest, ToolsExecutedOncePerStepNotTwice) {
58+
if (!use_real_api_) {
59+
GTEST_SKIP() << "No API key set for " << GetParam();
60+
}
61+
62+
// Create a shared counter to track tool executions
63+
auto execution_count = std::make_shared<std::atomic<int>>(0);
64+
65+
// Create a tool that increments the counter each time it's called
66+
Tool counter_tool = create_simple_tool(
67+
"get_counter", "Returns the current count", {{"message", "string"}},
68+
[execution_count](const JsonValue& args,
69+
const ToolExecutionContext& context) {
70+
int count = execution_count->fetch_add(1) + 1;
71+
std::string message = args["message"].get<std::string>();
72+
ai::logger::log_info("Counter tool executed! Count: {}, Message: {}",
73+
count, message);
74+
return JsonValue{{"count", count}, {"message", message}};
75+
});
76+
77+
ToolSet tools = {{"get_counter", counter_tool}};
78+
79+
// Configure options with multi-step mode enabled (max_steps > 1)
80+
GenerateOptions options(model_,
81+
"Please use the get_counter tool with message "
82+
"'test' to get the current count.");
83+
options.tools = tools;
84+
options.max_steps = 2; // Enable multi-step mode
85+
options.max_tokens = 300;
86+
87+
// Reset counter before test
88+
execution_count->store(0);
89+
90+
// Execute the request
91+
auto result = client_->generate_text(options);
92+
93+
// Verify result is successful
94+
EXPECT_TRUE(result.is_success())
95+
<< "Expected successful result but got error: " << result.error_message();
96+
97+
// Verify the tool was called
98+
EXPECT_TRUE(result.has_tool_calls()) << "Expected tool calls to be made";
99+
EXPECT_GT(result.tool_calls.size(), 0) << "Expected at least one tool call";
100+
101+
// Verify tool results exist
102+
EXPECT_TRUE(result.has_tool_results())
103+
<< "Expected tool results to be present";
104+
105+
// Log the execution count
106+
int final_count = execution_count->load();
107+
ai::logger::log_info(
108+
"Final execution count: {} (expected 1 per tool call, got {})",
109+
final_count, result.tool_calls.size());
110+
111+
// CRITICAL ASSERTION: Each tool call should be executed exactly once
112+
// If this fails, it means tools are being executed multiple times
113+
EXPECT_EQ(final_count, static_cast<int>(result.tool_calls.size()))
114+
<< "Tool execution count (" << final_count
115+
<< ") does not match tool call count (" << result.tool_calls.size()
116+
<< "). This indicates duplicate execution! "
117+
<< "See https://github.com/ClickHouse/ai-sdk-cpp/issues/26";
118+
119+
// Additional verification: check the tool results
120+
for (const auto& tool_result : result.tool_results) {
121+
EXPECT_TRUE(tool_result.is_success())
122+
<< "Tool execution failed: " << tool_result.error_message();
123+
124+
if (tool_result.is_success() && tool_result.result.contains("count")) {
125+
ai::logger::log_info("Tool result count: {}",
126+
tool_result.result["count"].get<int>());
127+
}
128+
}
129+
}
130+
131+
// Test with multiple steps to verify the issue persists across steps
132+
TEST_P(MultiStepDuplicateExecutionTest, MultipleStepsNoDuplicateExecution) {
133+
if (!use_real_api_) {
134+
GTEST_SKIP() << "No API key set for " << GetParam();
135+
}
136+
137+
// Create a shared counter to track tool executions
138+
auto execution_count = std::make_shared<std::atomic<int>>(0);
139+
140+
// Create a calculator tool that also tracks executions
141+
Tool tracked_calculator = create_simple_tool(
142+
"add", "Add two numbers together", {{"a", "number"}, {"b", "number"}},
143+
[execution_count](const JsonValue& args,
144+
const ToolExecutionContext& context) {
145+
int exec_num = execution_count->fetch_add(1) + 1;
146+
double a = args["a"].get<double>();
147+
double b = args["b"].get<double>();
148+
double result = a + b;
149+
150+
ai::logger::log_info(
151+
"Calculator executed (execution #{}): {} + {} = {}", exec_num, a, b,
152+
result);
153+
154+
return JsonValue{{"result", result},
155+
{"execution_number", exec_num},
156+
{"a", a},
157+
{"b", b}};
158+
});
159+
160+
ToolSet tools = {{"add", tracked_calculator}};
161+
162+
// Configure options with higher max_steps
163+
GenerateOptions options(model_,
164+
"Calculate 5 + 3 using the add tool. "
165+
"Then tell me the result.");
166+
options.tools = tools;
167+
options.max_steps = 3; // Allow multiple steps
168+
options.max_tokens = 500;
169+
170+
// Reset counter before test
171+
execution_count->store(0);
172+
173+
// Execute the request
174+
auto result = client_->generate_text(options);
175+
176+
// Verify result is successful
177+
EXPECT_TRUE(result.is_success())
178+
<< "Expected successful result but got error: " << result.error_message();
179+
180+
// Verify the tool was called
181+
EXPECT_TRUE(result.has_tool_calls()) << "Expected tool calls to be made";
182+
183+
// Log step information
184+
ai::logger::log_info("Total steps executed: {}", result.steps.size());
185+
ai::logger::log_info("Total tool calls: {}", result.tool_calls.size());
186+
187+
int final_count = execution_count->load();
188+
ai::logger::log_info("Total tool executions: {}", final_count);
189+
190+
// CRITICAL ASSERTION: Total executions should equal total tool calls
191+
EXPECT_EQ(final_count, static_cast<int>(result.tool_calls.size()))
192+
<< "Tool execution count (" << final_count
193+
<< ") does not match tool call count (" << result.tool_calls.size()
194+
<< "). This indicates duplicate execution across steps! "
195+
<< "See https://github.com/ClickHouse/ai-sdk-cpp/issues/26";
196+
197+
// Verify each step's tool results show unique execution numbers
198+
for (size_t i = 0; i < result.steps.size(); ++i) {
199+
const auto& step = result.steps[i];
200+
if (!step.tool_results.empty()) {
201+
ai::logger::log_info("Step {} had {} tool results", i + 1,
202+
step.tool_results.size());
203+
for (const auto& tool_result : step.tool_results) {
204+
if (tool_result.is_success() &&
205+
tool_result.result.contains("execution_number")) {
206+
int exec_num = tool_result.result["execution_number"].get<int>();
207+
ai::logger::log_info(" - Execution number: {}", exec_num);
208+
}
209+
}
210+
}
211+
}
212+
}
213+
214+
// Instantiate tests for both providers
215+
INSTANTIATE_TEST_SUITE_P(
216+
ProviderTests,
217+
MultiStepDuplicateExecutionTest,
218+
::testing::Values("openai", "anthropic"),
219+
[](const ::testing::TestParamInfo<
220+
MultiStepDuplicateExecutionTest::ParamType>& info) {
221+
return info.param;
222+
});
223+
224+
} // namespace test
225+
} // namespace ai

tests/integration/tool_calling_integration_test.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,16 @@ class ToolTestFixtures {
5656
double b = args["b"].get<double>();
5757

5858
double result = 0.0;
59-
if (op == "add")
59+
if (op == "add") {
6060
result = a + b;
61-
else if (op == "subtract")
61+
} else if (op == "subtract") {
6262
result = a - b;
63-
else if (op == "multiply")
63+
} else if (op == "multiply") {
6464
result = a * b;
65-
else if (op == "divide") {
66-
if (b == 0.0)
65+
} else if (op == "divide") {
66+
if (b == 0.0) {
6767
throw std::runtime_error("Division by zero");
68+
}
6869
result = a / b;
6970
}
7071

0 commit comments

Comments
 (0)