Skip to content

Commit 334214d

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 b8eab7c commit 334214d

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
@@ -226,13 +226,31 @@ pub async fn process_chat_sse<S>(
226226

227227
for call_id in tool_call_order.drain(..) {
228228
let state = tool_calls.remove(&call_id).unwrap_or_default();
229-
let item = ResponseItem::FunctionCall {
230-
id: None,
231-
name: state.name.unwrap_or_default(),
232-
arguments: state.arguments,
233-
call_id: call_id.clone(),
234-
};
235-
let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await;
229+
// Skip tool calls with empty names to avoid API errors
230+
match state.name {
231+
Some(name) if !name.is_empty() => {
232+
let item = ResponseItem::FunctionCall {
233+
id: None,
234+
name,
235+
arguments: state.arguments,
236+
call_id: call_id.clone(),
237+
};
238+
let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await;
239+
}
240+
Some(name) if name.is_empty() => {
241+
debug!(
242+
"Skipping tool call with empty name: call_id={}, arguments={}",
243+
call_id, state.arguments
244+
);
245+
}
246+
None => {
247+
debug!(
248+
"Skipping tool call with missing name: call_id={}, arguments={}",
249+
call_id, state.arguments
250+
);
251+
}
252+
_ => {}
253+
}
236254
}
237255
}
238256
}
@@ -544,4 +562,66 @@ mod tests {
544562
}));
545563
assert_matches!(events.last(), Some(ResponseEvent::Completed { .. }));
546564
}
565+
566+
#[tokio::test]
567+
async fn skips_tool_calls_with_empty_or_missing_names() {
568+
// Test case for tool call with empty name
569+
let delta_empty_name = json!({
570+
"choices": [{
571+
"delta": {
572+
"tool_calls": [{
573+
"id": "call_empty",
574+
"function": { "name": "", "arguments": "{}" }
575+
}]
576+
}
577+
}]
578+
});
579+
580+
// Test case for tool call with missing name
581+
let delta_no_name = json!({
582+
"choices": [{
583+
"delta": {
584+
"tool_calls": [{
585+
"id": "call_no_name",
586+
"function": { "arguments": "{}" }
587+
}]
588+
}
589+
}]
590+
});
591+
592+
// Valid tool call for comparison
593+
let delta_valid = json!({
594+
"choices": [{
595+
"delta": {
596+
"tool_calls": [{
597+
"id": "call_valid",
598+
"function": { "name": "valid_tool", "arguments": "{}" }
599+
}]
600+
}
601+
}]
602+
});
603+
604+
let finish = json!({
605+
"choices": [{
606+
"finish_reason": "tool_calls"
607+
}]
608+
});
609+
610+
let body = build_body(&[delta_empty_name, delta_no_name, delta_valid, finish]);
611+
let events = collect_events(&body).await;
612+
613+
// Should only emit the valid tool call
614+
let function_calls: Vec<_> = events
615+
.iter()
616+
.filter_map(|ev| match ev {
617+
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { name, .. }) => {
618+
Some(name.clone())
619+
}
620+
_ => None,
621+
})
622+
.collect();
623+
624+
assert_eq!(function_calls.len(), 1);
625+
assert_eq!(function_calls[0], "valid_tool");
626+
}
547627
}

0 commit comments

Comments
 (0)