Skip to content

Commit 1e461ad

Browse files
committed
fix: skip tool calls with empty/missing names and add debug logging
This commit improves handling of malformed tool calls in SSE streams: 1. Defensive fix: Skip tool calls that have empty or missing names to prevent API errors downstream 2. Debug logging: Add detailed logging when skipping tool calls to help diagnose root cause (model issue vs streaming issue) 3. Test coverage: Add test case to verify empty/missing names are properly filtered out The issue can occur when: - The SSE stream from the API is incomplete or corrupted - The model sends tool call deltas without including the name field - Tool call streaming completes with finish_reason="tool_calls" but some tool calls never received their name in any delta The debug logs will show: - call_id of the problematic tool call - arguments that were received (if any) - whether the name was empty string or completely missing This helps identify if the issue is: 1. A model bug (consistently missing names for certain patterns) 2. A streaming issue (names lost during transmission) 3. A timing issue (finish_reason sent before all deltas arrived)
1 parent 8766669 commit 1e461ad

File tree

1 file changed

+87
-7
lines changed

1 file changed

+87
-7
lines changed

codex-rs/codex-api/src/sse/chat.rs

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,31 @@ pub async fn process_chat_sse<S>(
224224

225225
for call_id in tool_call_order.drain(..) {
226226
let state = tool_calls.remove(&call_id).unwrap_or_default();
227-
let item = ResponseItem::FunctionCall {
228-
id: None,
229-
name: state.name.unwrap_or_default(),
230-
arguments: state.arguments,
231-
call_id: call_id.clone(),
232-
};
233-
let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await;
227+
// Skip tool calls with empty names to avoid API errors
228+
match state.name {
229+
Some(name) if !name.is_empty() => {
230+
let item = ResponseItem::FunctionCall {
231+
id: None,
232+
name,
233+
arguments: state.arguments,
234+
call_id: call_id.clone(),
235+
};
236+
let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await;
237+
}
238+
Some(name) if name.is_empty() => {
239+
debug!(
240+
"Skipping tool call with empty name: call_id={}, arguments={}",
241+
call_id, state.arguments
242+
);
243+
}
244+
None => {
245+
debug!(
246+
"Skipping tool call with missing name: call_id={}, arguments={}",
247+
call_id, state.arguments
248+
);
249+
}
250+
_ => {}
251+
}
234252
}
235253
}
236254
}
@@ -501,4 +519,66 @@ mod tests {
501519
}));
502520
assert_matches!(events.last(), Some(ResponseEvent::Completed { .. }));
503521
}
522+
523+
#[tokio::test]
524+
async fn skips_tool_calls_with_empty_or_missing_names() {
525+
// Test case for tool call with empty name
526+
let delta_empty_name = json!({
527+
"choices": [{
528+
"delta": {
529+
"tool_calls": [{
530+
"id": "call_empty",
531+
"function": { "name": "", "arguments": "{}" }
532+
}]
533+
}
534+
}]
535+
});
536+
537+
// Test case for tool call with missing name
538+
let delta_no_name = json!({
539+
"choices": [{
540+
"delta": {
541+
"tool_calls": [{
542+
"id": "call_no_name",
543+
"function": { "arguments": "{}" }
544+
}]
545+
}
546+
}]
547+
});
548+
549+
// Valid tool call for comparison
550+
let delta_valid = json!({
551+
"choices": [{
552+
"delta": {
553+
"tool_calls": [{
554+
"id": "call_valid",
555+
"function": { "name": "valid_tool", "arguments": "{}" }
556+
}]
557+
}
558+
}]
559+
});
560+
561+
let finish = json!({
562+
"choices": [{
563+
"finish_reason": "tool_calls"
564+
}]
565+
});
566+
567+
let body = build_body(&[delta_empty_name, delta_no_name, delta_valid, finish]);
568+
let events = collect_events(&body).await;
569+
570+
// Should only emit the valid tool call
571+
let function_calls: Vec<_> = events
572+
.iter()
573+
.filter_map(|ev| match ev {
574+
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { name, .. }) => {
575+
Some(name.clone())
576+
}
577+
_ => None,
578+
})
579+
.collect();
580+
581+
assert_eq!(function_calls.len(), 1);
582+
assert_eq!(function_calls[0], "valid_tool");
583+
}
504584
}

0 commit comments

Comments
 (0)