From febc3ebcb4df347e1e273b5ccb428940672e9fb2 Mon Sep 17 00:00:00 2001 From: kangxl Date: Mon, 11 May 2026 21:47:30 +0800 Subject: [PATCH] agent: Fix Preserve tool call ID/name across DashScope empty delta chunks (#54872) Release Notes: - Fixed: DashScope (Aliyun) tool calls now preserve id and name across streaming delta chunks --------------------------------------------------------------------------------------------------- Aliyun (DashScope) SSE streaming sends id="" and name="" in subsequent tool_calls delta chunks after the first chunk. Previously these empty strings would unconditionally overwrite the accumulated id and name values, causing tool calls to lose identity and fail. Add is_empty() guards so id and name are only updated when the delta provides a non-empty value (falsy guard pattern), matching how Hermes Agent and OpenAI SDK handle this provider edge case. Test stream_maps_preserves_tool_id_and_name_across_empty_deltas simulates DashScope's actual streaming behavior and asserts that the completed ToolUse retains the correct id, name, and arguments. Files changed: 1 (+148/-2) - crates/open_ai/src/completion.rs CLA signed. - [x] I've reviewed my own diff for quality, security, and reliability - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable CleanShot 2026-04-26 at 00 03 20@2x After FIx CleanShot 2026-04-26 at 00 02 15@2x --------- Co-authored-by: Bennet Bo Fenner --- crates/open_ai/src/completion.rs | 150 ++++++++++++++++++++++++++++++- 1 file changed, 148 insertions(+), 2 deletions(-) diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index 4abc752c4d..53f0f35054 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -461,12 +461,16 @@ impl OpenAiEventMapper { for tool_call in tool_calls { let entry = self.tool_calls_by_index.entry(tool_call.index).or_default(); - if let Some(tool_id) = tool_call.id.clone() { + if let Some(tool_id) = tool_call.id.clone() + && !tool_id.is_empty() + { entry.id = tool_id; } if let Some(function) = tool_call.function.as_ref() { - if let Some(name) = function.name.clone() { + if let Some(name) = function.name.clone() + && !name.is_empty() + { entry.name = name; } @@ -848,6 +852,9 @@ mod tests { use serde_json::json; use super::*; + use crate::{ + ChoiceDelta, FunctionChunk, ResponseMessageDelta, ResponseStreamEvent, ToolCallChunk, + }; fn map_response_events(events: Vec) -> Vec { block_on(async { @@ -861,6 +868,17 @@ mod tests { }) } + fn map_completion_events( + events: Vec, + ) -> Vec { + let mut mapper = OpenAiEventMapper::new(); + let mut all_events = Vec::new(); + for event in events { + all_events.extend(mapper.map_event(event)); + } + all_events.into_iter().filter_map(|e| e.ok()).collect() + } + fn response_item_message(id: &str) -> ResponseOutputItem { ResponseOutputItem::Message(ResponseOutputMessage { id: Some(id.to_string()), @@ -1722,4 +1740,132 @@ mod tests { ]) ); } + + #[test] + fn stream_maps_preserves_tool_id_and_name_across_empty_deltas() { + // DashScope sends id="" and name="" in subsequent tool_calls delta + // chunks after the first chunk. OpenAiEventMapper must not overwrite + // the accumulated id and name with these empty strings. + + let events = vec![ + // First chunk: id and name are present + ResponseStreamEvent { + choices: vec![ChoiceDelta { + index: 0, + delta: Some(ResponseMessageDelta { + role: None, + content: None, + tool_calls: Some(vec![ToolCallChunk { + index: 0, + id: Some("call_dashscope_test".into()), + function: Some(FunctionChunk { + name: Some("list_directory".into()), + arguments: Some("".into()), + }), + }]), + reasoning_content: None, + }), + finish_reason: None, + }], + usage: None, + }, + // Subsequent chunks: DashScope sends id="" and name="" + ResponseStreamEvent { + choices: vec![ChoiceDelta { + index: 0, + delta: Some(ResponseMessageDelta { + role: None, + content: None, + tool_calls: Some(vec![ToolCallChunk { + index: 0, + id: Some("".into()), + function: Some(FunctionChunk { + name: Some("".into()), + arguments: Some("{\"path\": \"".into()), + }), + }]), + reasoning_content: None, + }), + finish_reason: None, + }], + usage: None, + }, + ResponseStreamEvent { + choices: vec![ChoiceDelta { + index: 0, + delta: Some(ResponseMessageDelta { + role: None, + content: None, + tool_calls: Some(vec![ToolCallChunk { + index: 0, + id: Some("".into()), + function: Some(FunctionChunk { + name: Some("".into()), + arguments: Some("blog-scraper\"}".into()), + }), + }]), + reasoning_content: None, + }), + finish_reason: None, + }], + usage: None, + }, + // Final chunk: finish_reason = "tool_calls" + ResponseStreamEvent { + choices: vec![ChoiceDelta { + index: 0, + delta: None, + finish_reason: Some("tool_calls".into()), + }], + usage: None, + }, + ]; + + let mapped = map_completion_events(events); + + // Events emitted: + // 1. Partial ToolUse from chunk 1 (fix_json("") → "{}", parseable) + // 2. Partial ToolUse from chunk 3 (arguments fully assembled) + // 3. Complete ToolUse from finish_reason="tool_calls" drain + // 4. Stop(ToolUse) + assert_eq!(mapped.len(), 4); + + // Verify the complete ToolUse event (from finish_reason drain) + // has the correct id, name, and accumulated arguments. + let complete_tool_use = mapped.iter().find_map(|event| { + if let LanguageModelCompletionEvent::ToolUse(tool_use) = event { + if tool_use.is_input_complete { + return Some(tool_use); + } + } + None + }); + assert!( + complete_tool_use.is_some(), + "expected a completed ToolUse event" + ); + let tool_use = complete_tool_use.unwrap(); + assert_eq!( + tool_use.id.to_string(), + "call_dashscope_test", + "id must survive empty-string overwrites" + ); + assert_eq!( + tool_use.name.as_ref(), + "list_directory", + "name must survive empty-string overwrites" + ); + assert_eq!( + tool_use.raw_input, "{\"path\": \"blog-scraper\"}", + "arguments should accumulate across chunks" + ); + + // Verify the Stop event + assert!(mapped.iter().any(|event| { + matches!( + event, + LanguageModelCompletionEvent::Stop(StopReason::ToolUse) + ) + })); + } }