diff --git a/src/crates/adapters/ai-adapters/src/stream/stream_handler/openai.rs b/src/crates/adapters/ai-adapters/src/stream/stream_handler/openai.rs index f0e093ff65..5ec482f0ff 100644 --- a/src/crates/adapters/ai-adapters/src/stream/stream_handler/openai.rs +++ b/src/crates/adapters/ai-adapters/src/stream/stream_handler/openai.rs @@ -1,7 +1,7 @@ use super::inline_think::InlineThinkParser; use super::stream_stats::StreamStats; use super::{next_stream_item, TimedStreamItem}; -use crate::stream::types::openai::{OpenAISSEData, OpenAIToolCallArgumentsNormalizer}; +use crate::stream::types::openai::OpenAISSEData; use crate::stream::types::unified::UnifiedResponse; use anyhow::{anyhow, Result}; use eventsource_stream::Eventsource; @@ -20,22 +20,16 @@ const AI_STREAM_RESPONSE_TARGET: &str = "ai::openai_stream_response"; #[derive(Debug)] struct OpenAIResponseNormalizer { - tool_arguments_normalizer: OpenAIToolCallArgumentsNormalizer, inline_think_parser: InlineThinkParser, } impl OpenAIResponseNormalizer { fn new(inline_think_in_text: bool) -> Self { Self { - tool_arguments_normalizer: OpenAIToolCallArgumentsNormalizer::default(), inline_think_parser: InlineThinkParser::new(inline_think_in_text), } } - fn normalize_sse_data(&mut self, sse_data: &mut OpenAISSEData) { - sse_data.normalize_tool_call_arguments(&mut self.tool_arguments_normalizer); - } - fn normalize_response(&mut self, response: UnifiedResponse) -> Vec { self.inline_think_parser.normalize_response(response) } @@ -175,7 +169,7 @@ pub async fn handle_openai_stream( } stats.increment("chunk:chat_completion"); - let mut sse_data: OpenAISSEData = match serde_json::from_value(event_json) { + let sse_data: OpenAISSEData = match serde_json::from_value(event_json) { Ok(event) => event, Err(e) => { let error_msg = format!("SSE data schema error: {}, data: {}", e, &raw); @@ -196,8 +190,6 @@ pub async fn handle_openai_stream( ); } - normalizer.normalize_sse_data(&mut sse_data); - let has_empty_choices = sse_data.is_choices_empty(); let unified_responses = sse_data.into_unified_responses(); trace!( diff --git a/src/crates/adapters/ai-adapters/src/stream/types/openai.rs b/src/crates/adapters/ai-adapters/src/stream/types/openai.rs index 8146969f99..5b26fa8d20 100644 --- a/src/crates/adapters/ai-adapters/src/stream/types/openai.rs +++ b/src/crates/adapters/ai-adapters/src/stream/types/openai.rs @@ -1,5 +1,5 @@ use super::unified::{UnifiedResponse, UnifiedTokenUsage, UnifiedToolCall}; -use serde::{Deserialize, Deserializer}; +use serde::Deserialize; #[derive(Debug, Deserialize)] struct PromptTokensDetails { @@ -60,8 +60,6 @@ struct Choice { #[serde(default)] delta: Delta, finish_reason: Option, - #[serde(default, deserialize_with = "deserialize_optional_stringish")] - stop_reason: Option, } /// MiniMax `reasoning_details` array element. @@ -133,70 +131,7 @@ pub struct OpenAISSEData { usage: Option, } -#[derive(Debug, Default)] -pub struct OpenAIToolCallArgumentsNormalizer; - -fn deserialize_optional_stringish<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - let value = Option::::deserialize(deserializer)?; - Ok(match value { - None | Some(serde_json::Value::Null) => None, - Some(serde_json::Value::String(value)) => Some(value), - Some(serde_json::Value::Number(value)) => Some(value.to_string()), - Some(serde_json::Value::Bool(value)) => Some(value.to_string()), - Some(other) => Some(other.to_string()), - }) -} - -impl OpenAIToolCallArgumentsNormalizer { - fn normalize_choice(&mut self, choice: &mut Choice) { - let has_stop_reason = choice.stop_reason.is_some(); - let Some(tool_calls) = choice.delta.tool_calls.as_mut() else { - return; - }; - - for tool_call in tool_calls.iter_mut() { - self.normalize_tool_call(tool_call, has_stop_reason); - } - } - - fn normalize_tool_call(&mut self, tool_call: &mut OpenAIToolCall, has_stop_reason: bool) { - let has_id = tool_call.id.as_ref().is_some_and(|value| !value.is_empty()); - let has_name = tool_call - .function - .as_ref() - .and_then(|function| function.name.as_ref()) - .is_some_and(|value| !value.is_empty()); - - let Some(function) = tool_call.function.as_mut() else { - return; - }; - let Some(arguments) = function.arguments.as_ref() else { - return; - }; - - if arguments.is_empty() { - return; - } - - if has_stop_reason && !has_id && !has_name { - tool_call.arguments_is_snapshot = true; - } - } -} - impl OpenAISSEData { - pub fn normalize_tool_call_arguments( - &mut self, - normalizer: &mut OpenAIToolCallArgumentsNormalizer, - ) { - if let Some(first_choice) = self.choices.first_mut() { - normalizer.normalize_choice(first_choice); - } - } - pub fn is_choices_empty(&self) -> bool { self.choices.is_empty() } @@ -334,7 +269,7 @@ impl From for UnifiedResponse { #[cfg(test)] mod tests { - use super::{OpenAISSEData, OpenAIToolCallArgumentsNormalizer}; + use super::OpenAISSEData; #[test] fn splits_multiple_tool_calls_in_first_choice() { @@ -619,50 +554,8 @@ mod tests { } #[test] - fn marks_stop_reason_tool_chunk_as_snapshot() { - let mut normalizer = OpenAIToolCallArgumentsNormalizer::default(); - - let mut first_chunk: OpenAISSEData = serde_json::from_str( - r#"{ - "id": "chatcmpl_test", - "created": 123, - "model": "gpt-test", - "choices": [{ - "index": 0, - "delta": { - "tool_calls": [{ - "index": 0, - "id": "call_1", - "type": "function", - "function": { - "name": "tool_a", - "arguments": "{\"city\":\"Bei" - } - }] - }, - "finish_reason": null - }] - }"#, - ) - .expect("valid first chunk"); - first_chunk.normalize_tool_call_arguments(&mut normalizer); - let first_responses = first_chunk.into_unified_responses(); - assert_eq!( - first_responses[0] - .tool_call - .as_ref() - .and_then(|tool| tool.arguments.as_deref()), - Some("{\"city\":\"Bei") - ); - assert!( - !first_responses[0] - .tool_call - .as_ref() - .expect("tool call") - .arguments_is_snapshot - ); - - let mut snapshot_chunk: OpenAISSEData = serde_json::from_str( + fn stop_reason_tool_chunk_keeps_default_non_snapshot_behavior() { + let data: OpenAISSEData = serde_json::from_str( r#"{ "id": "chatcmpl_test", "created": 123, @@ -682,31 +575,28 @@ mod tests { }] }"#, ) - .expect("valid snapshot chunk"); - snapshot_chunk.normalize_tool_call_arguments(&mut normalizer); - let snapshot_responses = snapshot_chunk.into_unified_responses(); + .expect("valid stop_reason chunk"); + let responses = data.into_unified_responses(); assert_eq!( - snapshot_responses[0] + responses[0] .tool_call .as_ref() .and_then(|tool| tool.arguments.as_deref()), Some("{\"city\":\"Beijing\"}") ); assert!( - snapshot_responses[0] + !responses[0] .tool_call .as_ref() .expect("tool call") .arguments_is_snapshot ); - assert!(snapshot_responses[0].finish_reason.is_none()); + assert!(responses[0].finish_reason.is_none()); } #[test] fn leaves_normal_tool_delta_chunks_as_non_snapshot() { - let mut normalizer = OpenAIToolCallArgumentsNormalizer::default(); - - let mut chunk: OpenAISSEData = serde_json::from_str( + let chunk: OpenAISSEData = serde_json::from_str( r#"{ "id": "chatcmpl_test", "created": 123, @@ -727,7 +617,6 @@ mod tests { }"#, ) .expect("valid chunk"); - chunk.normalize_tool_call_arguments(&mut normalizer); let responses = chunk.into_unified_responses(); assert_eq!(responses.len(), 1); assert!( @@ -740,7 +629,7 @@ mod tests { } #[test] - fn parses_numeric_stop_reason_as_string() { + fn accepts_numeric_stop_reason_payload() { let data: OpenAISSEData = serde_json::from_str( r#"{ "id": "chatcmpl_test", @@ -762,10 +651,6 @@ mod tests { }"#, ) .expect("valid numeric stop_reason payload"); - - let mut normalizer = OpenAIToolCallArgumentsNormalizer::default(); - let mut data = data; - data.normalize_tool_call_arguments(&mut normalizer); let responses = data.into_unified_responses(); assert_eq!(responses.len(), 1); @@ -773,7 +658,7 @@ mod tests { } #[test] - fn parses_string_stop_reason_unchanged() { + fn accepts_string_stop_reason_payload() { let data: OpenAISSEData = serde_json::from_str( r#"{ "id": "chatcmpl_test", @@ -795,10 +680,6 @@ mod tests { }"#, ) .expect("valid string stop_reason payload"); - - let mut normalizer = OpenAIToolCallArgumentsNormalizer::default(); - let mut data = data; - data.normalize_tool_call_arguments(&mut normalizer); let responses = data.into_unified_responses(); assert_eq!(responses.len(), 1); diff --git a/src/crates/adapters/ai-adapters/tests/fixtures/stream/openai/tool_args_snapshot_stop_reason.sse b/src/crates/adapters/ai-adapters/tests/fixtures/stream/openai/tool_args_snapshot_stop_reason.sse index f4e73b30fb..7478e535ad 100644 --- a/src/crates/adapters/ai-adapters/tests/fixtures/stream/openai/tool_args_snapshot_stop_reason.sse +++ b/src/crates/adapters/ai-adapters/tests/fixtures/stream/openai/tool_args_snapshot_stop_reason.sse @@ -1,6 +1,6 @@ data: {"id":"chatcmpl_test","object":"chat.completion.chunk","created":500,"model":"gpt-test","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","type":"function","function":{"name":"tool_a","arguments":"{\"city\":\"Bei"}}]},"finish_reason":null}],"usage":null} -data: {"id":"chatcmpl_test","object":"chat.completion.chunk","created":501,"model":"gpt-test","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":null,"type":"function","function":{"arguments":"{\"city\":\"Beijing\"}"}}]},"stop_reason":"stop"}],"usage":null} +data: {"id":"chatcmpl_test","object":"chat.completion.chunk","created":501,"model":"gpt-test","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":null,"type":"function","function":{"arguments":"jing\"}"}}]},"stop_reason":"stop"}],"usage":null} data: {"id":"chatcmpl_test","object":"chat.completion.chunk","created":502,"model":"gpt-test","choices":[],"usage":{"prompt_tokens":3,"completion_tokens":6,"total_tokens":9}} diff --git a/src/crates/adapters/ai-adapters/tests/stream_processor_openai.rs b/src/crates/adapters/ai-adapters/tests/stream_processor_openai.rs index 2c0d84ec21..e3ce1c4536 100644 --- a/src/crates/adapters/ai-adapters/tests/stream_processor_openai.rs +++ b/src/crates/adapters/ai-adapters/tests/stream_processor_openai.rs @@ -244,7 +244,7 @@ async fn openai_fixture_reattaches_id_only_prelude_to_following_payload_chunk() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn openai_fixture_replaces_snapshot_tool_args_after_stop_reason_chunk() { +async fn openai_fixture_keeps_appending_tool_args_after_stop_reason_chunk() { let output = run_stream_fixture( StreamFixtureProvider::OpenAi, "stream/openai/tool_args_snapshot_stop_reason.sse", @@ -275,10 +275,7 @@ async fn openai_fixture_replaces_snapshot_tool_args_after_stop_reason_chunk() { _ => None, }) .collect(); - assert_eq!( - partial_params, - vec!["{\"city\":\"Bei", "{\"city\":\"Beijing\"}"] - ); + assert_eq!(partial_params, vec!["{\"city\":\"Bei", "jing\"}"]); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs b/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs index 2ec39bd4d8..eb2bd7b083 100644 --- a/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs +++ b/src/crates/assembly/core/src/agentic/tools/pipeline/tool_pipeline.rs @@ -1545,6 +1545,12 @@ mod tests { .as_deref() .unwrap_or_default() .contains("Provided arguments: {\"operation\":\"log\"")); + assert!(!result + .result + .result_for_assistant + .as_deref() + .unwrap_or_default() + .contains("Raw arguments:")); } #[tokio::test] diff --git a/src/crates/execution/tool-contracts/src/tool_execution_presentation.rs b/src/crates/execution/tool-contracts/src/tool_execution_presentation.rs index f16e0827e9..89351d8998 100644 --- a/src/crates/execution/tool-contracts/src/tool_execution_presentation.rs +++ b/src/crates/execution/tool-contracts/src/tool_execution_presentation.rs @@ -102,9 +102,9 @@ pub fn build_invalid_tool_call_error_message( tool_name: &str, tool_is_error: bool, recovered_from_truncation: bool, - raw_arguments_preview: Option, + _raw_arguments_preview: Option, ) -> String { - let error_msg = if tool_name.is_empty() && tool_is_error { + if tool_name.is_empty() && tool_is_error { "Missing valid tool name and arguments are invalid JSON.".to_string() } else if tool_name.is_empty() { "Missing valid tool name.".to_string() @@ -114,11 +114,5 @@ pub fn build_invalid_tool_call_error_message( ) } else { "Arguments are invalid JSON.".to_string() - }; - - if let Some(raw_arguments_preview) = raw_arguments_preview { - format!("{error_msg} Raw arguments: {raw_arguments_preview}") - } else { - error_msg } } diff --git a/src/crates/execution/tool-contracts/tests/tool_contracts.rs b/src/crates/execution/tool-contracts/tests/tool_contracts.rs index ceb8d8cd3a..8fa6997910 100644 --- a/src/crates/execution/tool-contracts/tests/tool_contracts.rs +++ b/src/crates/execution/tool-contracts/tests/tool_contracts.rs @@ -163,7 +163,7 @@ fn invalid_tool_call_error_message_preserves_current_contract() { build_invalid_tool_call_error_message("", true, false, Some("{\"path\"".to_string())); assert_eq!( message, - "Missing valid tool name and arguments are invalid JSON. Raw arguments: {\"path\"" + "Missing valid tool name and arguments are invalid JSON." ); let message = build_invalid_tool_call_error_message("", false, false, None);