fix(acp): avoid duplicate thought chunks around tool calls (#8215)
Signed-off-by: Kevin Lam <kevklam@users.noreply.github.com>
This commit is contained in:
@@ -395,10 +395,12 @@ impl Agent {
|
|||||||
&self,
|
&self,
|
||||||
response: &Message,
|
response: &Message,
|
||||||
tools: &[rmcp::model::Tool],
|
tools: &[rmcp::model::Tool],
|
||||||
|
suppress_replayed_thinking: bool,
|
||||||
) -> ToolCategorizeResult {
|
) -> ToolCategorizeResult {
|
||||||
// Categorize tool requests
|
// Categorize tool requests
|
||||||
let (frontend_requests, remaining_requests, filtered_response) =
|
let (frontend_requests, remaining_requests, filtered_response) = self
|
||||||
self.categorize_tool_requests(response, tools).await;
|
.categorize_tool_requests(response, tools, suppress_replayed_thinking)
|
||||||
|
.await;
|
||||||
|
|
||||||
ToolCategorizeResult {
|
ToolCategorizeResult {
|
||||||
frontend_requests,
|
frontend_requests,
|
||||||
@@ -1224,6 +1226,11 @@ impl Agent {
|
|||||||
let mut did_recovery_compact_this_iteration = false;
|
let mut did_recovery_compact_this_iteration = false;
|
||||||
let mut exit_chat = false;
|
let mut exit_chat = false;
|
||||||
|
|
||||||
|
// Track whether this provider turn has already emitted visible
|
||||||
|
// thinking so a later tool-call chunk can suppress replayed
|
||||||
|
// reasoning without hiding final-only non-streaming thoughts.
|
||||||
|
let mut surfaced_thinking_in_turn = false;
|
||||||
|
|
||||||
while let Some(next) = stream.next().await {
|
while let Some(next) = stream.next().await {
|
||||||
if is_token_cancelled(&cancel_token) || exit_chat {
|
if is_token_cancelled(&cancel_token) || exit_chat {
|
||||||
break;
|
break;
|
||||||
@@ -1242,7 +1249,23 @@ impl Agent {
|
|||||||
frontend_requests,
|
frontend_requests,
|
||||||
remaining_requests,
|
remaining_requests,
|
||||||
filtered_response,
|
filtered_response,
|
||||||
} = self.categorize_tools(&response, &tools).await;
|
} = self
|
||||||
|
.categorize_tools(
|
||||||
|
&response,
|
||||||
|
&tools,
|
||||||
|
surfaced_thinking_in_turn,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
surfaced_thinking_in_turn |= filtered_response.content.iter().any(
|
||||||
|
|content| {
|
||||||
|
matches!(
|
||||||
|
content,
|
||||||
|
MessageContent::Thinking(_)
|
||||||
|
| MessageContent::RedactedThinking(_)
|
||||||
|
)
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
yield AgentEvent::Message(filtered_response.clone());
|
yield AgentEvent::Message(filtered_response.clone());
|
||||||
tokio::task::yield_now().await;
|
tokio::task::yield_now().await;
|
||||||
|
|||||||
@@ -339,6 +339,7 @@ impl Agent {
|
|||||||
&self,
|
&self,
|
||||||
response: &Message,
|
response: &Message,
|
||||||
tools: &[Tool],
|
tools: &[Tool],
|
||||||
|
suppress_replayed_thinking: bool,
|
||||||
) -> (Vec<ToolRequest>, Vec<ToolRequest>, Message) {
|
) -> (Vec<ToolRequest>, Vec<ToolRequest>, Message) {
|
||||||
// First collect all tool requests with coercion applied
|
// First collect all tool requests with coercion applied
|
||||||
let tool_requests: Vec<ToolRequest> = response
|
let tool_requests: Vec<ToolRequest> = response
|
||||||
@@ -367,7 +368,16 @@ impl Agent {
|
|||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
// Create a filtered message with frontend tool requests removed
|
let has_tool_requests = !tool_requests.is_empty();
|
||||||
|
let should_suppress_replayed_thinking = suppress_replayed_thinking && has_tool_requests;
|
||||||
|
|
||||||
|
// Create a filtered message with frontend tool requests removed.
|
||||||
|
// When a response contains tool calls, keep reasoning in the original
|
||||||
|
// message for provider/state purposes but only suppress it from the
|
||||||
|
// user-visible filtered message if the caller already surfaced
|
||||||
|
// thinking earlier in this provider turn. That avoids replaying full
|
||||||
|
// accumulated reasoning after streamed thought chunks while still
|
||||||
|
// preserving final-only non-streaming thoughts.
|
||||||
let mut filtered_content = Vec::new();
|
let mut filtered_content = Vec::new();
|
||||||
let mut tool_request_index = 0;
|
let mut tool_request_index = 0;
|
||||||
|
|
||||||
@@ -389,6 +399,8 @@ impl Agent {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
MessageContent::Thinking(_) | MessageContent::RedactedThinking(_)
|
||||||
|
if should_suppress_replayed_thinking => {}
|
||||||
_ => {
|
_ => {
|
||||||
filtered_content.push(content.clone());
|
filtered_content.push(content.clone());
|
||||||
}
|
}
|
||||||
@@ -662,6 +674,52 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn categorize_tool_requests_keeps_thinking_when_not_previously_streamed() {
|
||||||
|
let agent = crate::agents::Agent::new();
|
||||||
|
let response = Message::assistant()
|
||||||
|
.with_thinking("final-only reasoning", "")
|
||||||
|
.with_tool_request(
|
||||||
|
"tool-1",
|
||||||
|
Ok(rmcp::model::CallToolRequestParams::new("test_tool")),
|
||||||
|
);
|
||||||
|
|
||||||
|
let (_frontend_requests, other_requests, filtered_message) =
|
||||||
|
agent.categorize_tool_requests(&response, &[], false).await;
|
||||||
|
|
||||||
|
assert_eq!(other_requests.len(), 1);
|
||||||
|
assert_eq!(filtered_message.content.len(), 2);
|
||||||
|
assert!(matches!(
|
||||||
|
filtered_message.content[0],
|
||||||
|
MessageContent::Thinking(_)
|
||||||
|
));
|
||||||
|
assert!(matches!(
|
||||||
|
filtered_message.content[1],
|
||||||
|
MessageContent::ToolRequest(_)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn categorize_tool_requests_drops_replayed_thinking_after_streaming() {
|
||||||
|
let agent = crate::agents::Agent::new();
|
||||||
|
let response = Message::assistant()
|
||||||
|
.with_thinking("replayed reasoning", "")
|
||||||
|
.with_tool_request(
|
||||||
|
"tool-1",
|
||||||
|
Ok(rmcp::model::CallToolRequestParams::new("test_tool")),
|
||||||
|
);
|
||||||
|
|
||||||
|
let (_frontend_requests, other_requests, filtered_message) =
|
||||||
|
agent.categorize_tool_requests(&response, &[], true).await;
|
||||||
|
|
||||||
|
assert_eq!(other_requests.len(), 1);
|
||||||
|
assert_eq!(filtered_message.content.len(), 1);
|
||||||
|
assert!(matches!(
|
||||||
|
filtered_message.content[0],
|
||||||
|
MessageContent::ToolRequest(_)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
fn make_tool_with_meta(meta_json: Option<serde_json::Value>) -> Tool {
|
fn make_tool_with_meta(meta_json: Option<serde_json::Value>) -> Tool {
|
||||||
let mut tool = Tool::new("test_tool", "a test tool", object!({ "type": "object" }));
|
let mut tool = Tool::new("test_tool", "a test tool", object!({ "type": "object" }));
|
||||||
if let Some(v) = meta_json {
|
if let Some(v) = meta_json {
|
||||||
|
|||||||
Reference in New Issue
Block a user