From 455556380b52c18d3d444a751a6c17de6d4ee5b0 Mon Sep 17 00:00:00 2001 From: Huei Feng <695979933@qq.com> Date: Fri, 19 Jun 2026 23:38:25 +0800 Subject: [PATCH] Chat API: skip tool calls with missing function names (#4159) * Chat API: skip tool calls with missing function names Some providers send empty or absent function names in streaming tool call deltas. Previously these produced invalid output items. - Don't overwrite accumulated state.name with empty deltas - Skip tool calls that never received a valid name (instead of falling back to 'unknown_tool') - Apply the same defensive guard in finalize_tools and the non-streaming path * Address review: defer empty-name skip to finalization Require both call_id and name before triggering should_add, instead of skipping eagerly when name is absent in the first delta. This handles providers that send id before name, as suggested in the Codex review. * Guard legacy function_call against empty name Return Option from chat_legacy_function_call_to_response_item, returning None when function_call.name is missing or empty. This covers the legacy message.function_call path that the original guard missed. * Remove unreachable unknown_tool fallback --------- Co-authored-by: Jason --- .../proxy/providers/streaming_codex_chat.rs | 29 +++++++++++++------ .../proxy/providers/transform_codex_chat.rs | 28 ++++++++++++++---- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src-tauri/src/proxy/providers/streaming_codex_chat.rs b/src-tauri/src/proxy/providers/streaming_codex_chat.rs index 7039c1b27..a5326ab13 100644 --- a/src-tauri/src/proxy/providers/streaming_codex_chat.rs +++ b/src-tauri/src/proxy/providers/streaming_codex_chat.rs @@ -429,8 +429,10 @@ impl ChatToResponsesState { if let Some(id) = id_delta { state.call_id = id; } - if let Some(name) = name_delta { - state.name = name; + if let Some(ref name) = name_delta { + if !name.is_empty() { + state.name.clone_from(name); + } } if !args_delta.is_empty() { state.arguments.push_str(&args_delta); @@ -442,7 +444,7 @@ impl ChatToResponsesState { } } - if !state.added && (!state.call_id.is_empty() || !state.name.is_empty()) { + if !state.added && !state.call_id.is_empty() && !state.name.is_empty() { should_add = true; pending_arguments = state.arguments.clone(); } else if state.added { @@ -464,9 +466,6 @@ impl ChatToResponsesState { if state.call_id.is_empty() { state.call_id = format!("call_{chat_index}"); } - if state.name.is_empty() { - state.name = "unknown_tool".to_string(); - } state.output_index = Some(assigned); let is_custom_tool = self.tool_context.is_custom_tool_chat_name(&state.name); state.item_id = response_tool_call_item_id_from_chat_name( @@ -699,6 +698,21 @@ impl ChatToResponsesState { continue; } + // Skip tool calls with missing names (defensive: some models generate + // tool call deltas without providing a valid function name) + let has_bad_name = self + .tools + .get(&key) + .map(|state| state.name.is_empty()) + .unwrap_or(true); + if has_bad_name { + if let Some(state) = self.tools.get_mut(&key) { + state.done = true; + } + log::warn!("[Codex] Skipping streaming tool call with missing name"); + continue; + } + if self .tools .get(&key) @@ -713,9 +727,6 @@ impl ChatToResponsesState { if state.call_id.is_empty() { state.call_id = format!("call_{key}"); } - if state.name.is_empty() { - state.name = "unknown_tool".to_string(); - } state.output_index = Some(assigned); state.item_id = response_tool_call_item_id_from_chat_name( &state.call_id, diff --git a/src-tauri/src/proxy/providers/transform_codex_chat.rs b/src-tauri/src/proxy/providers/transform_codex_chat.rs index d43809f0f..1edc23d70 100644 --- a/src-tauri/src/proxy/providers/transform_codex_chat.rs +++ b/src-tauri/src/proxy/providers/transform_codex_chat.rs @@ -1398,6 +1398,14 @@ fn chat_tool_calls_to_response_output_items( if let Some(tool_calls) = message.get("tool_calls").and_then(|v| v.as_array()) { for (index, tool_call) in tool_calls.iter().enumerate() { + // Skip tool calls with missing function names (defensive: some models + // may generate tool calls without providing a valid name) + let function = tool_call.get("function").unwrap_or(&Value::Null); + let name = function.get("name").and_then(|v| v.as_str()).unwrap_or(""); + if name.is_empty() { + log::warn!("[Codex] Skipping tool call with missing name"); + continue; + } output.push(chat_tool_call_to_response_item( tool_call, index, @@ -1406,11 +1414,13 @@ fn chat_tool_calls_to_response_output_items( )); } } else if let Some(function_call) = message.get("function_call") { - output.push(chat_legacy_function_call_to_response_item( + if let Some(item) = chat_legacy_function_call_to_response_item( function_call, reasoning, tool_context, - )); + ) { + output.push(item); + } } output @@ -1448,7 +1458,7 @@ fn chat_legacy_function_call_to_response_item( function_call: &Value, reasoning: Option<&str>, tool_context: &CodexToolContext, -) -> Value { +) -> Option { let call_id = function_call .get("id") .and_then(|v| v.as_str()) @@ -1458,10 +1468,18 @@ fn chat_legacy_function_call_to_response_item( .get("name") .and_then(|v| v.as_str()) .unwrap_or(""); + + // Skip legacy function calls with missing names (defensive: some models + // may generate function_call without providing a valid name) + if name.is_empty() { + log::warn!("[Codex] Skipping legacy function_call with missing name"); + return None; + } + let arguments = canonicalize_tool_arguments(function_call.get("arguments")); let item_id = response_tool_call_item_id_from_chat_name(call_id, name, tool_context); - response_tool_call_item_from_chat_name( + Some(response_tool_call_item_from_chat_name( &item_id, "completed", call_id, @@ -1469,7 +1487,7 @@ fn chat_legacy_function_call_to_response_item( &arguments, reasoning, tool_context, - ) + )) } pub(crate) fn response_tool_call_item_id_from_chat_name(