Skip to content

Commit 68a1806

Browse files
stebbinsclaude
andcommitted
fix: stop merging tool calls across step boundaries in format_messages
The parallel tool call merge (merge #2) was too aggressive — it merged assistant+tool_call messages across separate model response boundaries. When a multi-step conversation had reasoning_content on each step's assistant message, merge #2 combined all tool_calls into a single assistant message, losing reasoning_content from later steps and creating an invalid conversation structure for Kimi thinking models. The fix adds a check: stop merging when the next assistant message has reasoning_content, which indicates a new model response rather than a split parallel tool call from the same response. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Harrison <hcstebbins@gmail.com>
1 parent 1051b46 commit 68a1806

File tree

1 file changed

+106
-1
lines changed

1 file changed

+106
-1
lines changed

crates/goose/src/providers/formats/openai.rs

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,8 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec<
325325
let next_is_assistant_tc = messages_spec[end + 1].get("role").and_then(|v| v.as_str())
326326
== Some("assistant")
327327
&& messages_spec[end + 1].get("tool_calls").is_some()
328-
&& messages_spec[end + 1].get("content").is_none();
328+
&& messages_spec[end + 1].get("content").is_none()
329+
&& messages_spec[end + 1].get("reasoning_content").is_none();
329330

330331
if is_tool && next_is_assistant_tc {
331332
end += 2;
@@ -2084,6 +2085,110 @@ data: [DONE]
20842085
Ok(())
20852086
}
20862087

2088+
#[test]
2089+
fn test_format_messages_multi_step_tool_calls_not_merged() -> anyhow::Result<()> {
2090+
// Simulates a 2-step conversation where both steps have reasoning + tool calls.
2091+
// The agent splits each step into: thinking_msg, tool_msg, resp_msg, ...
2092+
// Merge #2 should NOT merge across step boundaries.
2093+
2094+
// Step 1: reasoning + 2 parallel tool calls
2095+
let thinking1 = Message::new(
2096+
Role::Assistant,
2097+
chrono::Utc::now().timestamp(),
2098+
vec![MessageContent::thinking("Step 1 reasoning", "")],
2099+
);
2100+
let tool1a = Message::assistant().with_tool_request(
2101+
"tc1a",
2102+
Ok(CallToolRequestParams {
2103+
meta: None,
2104+
task: None,
2105+
name: "shell".into(),
2106+
arguments: Some(object!({"command": "ls"})),
2107+
}),
2108+
);
2109+
let resp1a = Message::user().with_tool_response(
2110+
"tc1a".to_string(),
2111+
Ok(CallToolResult {
2112+
content: vec![Content::text("file1.txt")],
2113+
structured_content: None,
2114+
is_error: Some(false),
2115+
meta: None,
2116+
}),
2117+
);
2118+
let tool1b = Message::assistant().with_tool_request(
2119+
"tc1b",
2120+
Ok(CallToolRequestParams {
2121+
meta: None,
2122+
task: None,
2123+
name: "shell".into(),
2124+
arguments: Some(object!({"command": "pwd"})),
2125+
}),
2126+
);
2127+
let resp1b = Message::user().with_tool_response(
2128+
"tc1b".to_string(),
2129+
Ok(CallToolResult {
2130+
content: vec![Content::text("/home")],
2131+
structured_content: None,
2132+
is_error: Some(false),
2133+
meta: None,
2134+
}),
2135+
);
2136+
2137+
// Step 2: new reasoning + 1 tool call (this is a NEW model response)
2138+
let thinking2 = Message::new(
2139+
Role::Assistant,
2140+
chrono::Utc::now().timestamp(),
2141+
vec![MessageContent::thinking("Step 2 reasoning", "")],
2142+
);
2143+
let tool2 = Message::assistant().with_tool_request(
2144+
"tc2",
2145+
Ok(CallToolRequestParams {
2146+
meta: None,
2147+
task: None,
2148+
name: "shell".into(),
2149+
arguments: Some(object!({"command": "cat result.txt"})),
2150+
}),
2151+
);
2152+
let resp2 = Message::user().with_tool_response(
2153+
"tc2".to_string(),
2154+
Ok(CallToolResult {
2155+
content: vec![Content::text("done")],
2156+
structured_content: None,
2157+
is_error: Some(false),
2158+
meta: None,
2159+
}),
2160+
);
2161+
2162+
let spec = format_messages(
2163+
&[thinking1, tool1a, resp1a, tool1b, resp1b, thinking2, tool2, resp2],
2164+
&ImageFormat::OpenAi,
2165+
);
2166+
2167+
// Step 1: assistant(reasoning1 + tc1a, tc1b) + 2 tool responses = 3
2168+
// Step 2: assistant(reasoning2 + tc2) + 1 tool response = 2
2169+
// Total = 5
2170+
assert_eq!(spec.len(), 5, "Expected 5 messages, got {}: {:#?}", spec.len(), spec);
2171+
2172+
// Step 1 assistant: reasoning1 + 2 tool_calls
2173+
assert_eq!(spec[0]["role"], "assistant");
2174+
assert_eq!(spec[0]["reasoning_content"], "Step 1 reasoning");
2175+
assert_eq!(spec[0]["tool_calls"].as_array().unwrap().len(), 2);
2176+
2177+
// Step 1 tool responses
2178+
assert_eq!(spec[1]["role"], "tool");
2179+
assert_eq!(spec[2]["role"], "tool");
2180+
2181+
// Step 2 assistant: reasoning2 + 1 tool_call (NOT merged with step 1)
2182+
assert_eq!(spec[3]["role"], "assistant");
2183+
assert_eq!(spec[3]["reasoning_content"], "Step 2 reasoning");
2184+
assert_eq!(spec[3]["tool_calls"].as_array().unwrap().len(), 1);
2185+
2186+
// Step 2 tool response
2187+
assert_eq!(spec[4]["role"], "tool");
2188+
2189+
Ok(())
2190+
}
2191+
20872192
#[test]
20882193
fn test_create_request_kimi_thinking_defaults() -> anyhow::Result<()> {
20892194
let model_config = ModelConfig {

0 commit comments

Comments
 (0)