Chat API: skip tool calls with missing function names#4159
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa971205e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
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.
hueifeng
force-pushed
the
fix/chat-tool-call-missing-name
branch
from
34967ea to
00ad284
Compare
|
@farion1231 friendly ping — rebased to latest main, could you take another look? |
farion1231
left a comment
farion1231
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Could you please check whether the following issue exists:
One possible issue: the new guard only covers the tool_calls[] path, but the legacy message.function_call branch can still produce a Responses tool item with an empty name.
chat_legacy_function_call_to_response_item still falls back to "" when function_call.name is missing or empty, so non-streaming legacy Chat API responses can still generate the invalid empty-name tool item that this PR is trying to avoid.
Could you apply the same guard there as well? One clean option would be to make the legacy helper return Option<Value> and return None when the name is empty.
Return Option<Value> 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.
hueifeng
force-pushed
the
fix/chat-tool-call-missing-name
branch
from
9d02f2d to
98d6b19
Compare
|
@farion1231 Good catch, added the same guard for the legacy path as well. |
farion1231
left a comment
farion1231
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3526aae85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !state.added && !state.call_id.is_empty() && !state.name.is_empty() { | ||
| should_add = true; | ||
| pending_arguments = state.arguments.clone(); |
There was a problem hiding this comment.
When an OpenAI-compatible provider omits tool_call.id but sends a valid function.name (which this converter still supports via the call_{index} fallback in finalize_tools), this new condition prevents the tool from being added during streaming. Arguments are buffered silently and the item is only added/done at finalization, so Responses clients that consume response.function_call_arguments.delta no longer see incremental tool-call output for an otherwise valid tool call. The missing-name guard should not also require a non-empty upstream id, or the no-id fallback should emit pending argument deltas when finalizing.
Useful? React with 👍 / 👎.
2 participants
Some providers send empty or absent function names in streaming tool call deltas. Previously these produced invalid output items that could not be handled by downstream clients.
Changes: