fix(streaming): merge duplicate-index tool_call deltas in first chunk#3496
fix(streaming): merge duplicate-index tool_call deltas in first chunk#3496Chukwuebuka-2003 wants to merge 2 commits into
Conversation
When the first streamed chunk carries multiple tool_calls entries with the same `index` (e.g. tool name + start of arguments at index 0), the accumulator stored them as separate physical list entries. Every subsequent delta merged only into the first entry, leaving the orphan fragment stranded and producing invalid final JSON. The root cause is two early-return paths in `accumulate_delta` that assign the raw list before checking for duplicate logical indices. The same path exists in the assistants API's copy of the function. Fix: 1. Add `_normalize_indexed_list()` that merges entries sharing the same integer `index` into a single entry before first storage. 2. Apply it at both early-return branches (key absent, value None). 3. Also normalize the initial snapshot in `_convert_initial_chunk_into_snapshot` so that a first chunk with duplicate indices doesn't seed bad state. Applies to both `_deltas.py` (chat completions) and the duplicated copy in `_assistants.py`. Fixes openai#3201
![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: 5fd5c221b3
ℹ️ 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 isinstance(tool_calls, list): | ||
| message["tool_calls"] = _normalize_indexed_list(tool_calls) |
There was a problem hiding this comment.
This only normalizes tool_calls for the first SSE chunk of the whole completion. In _accumulate_chunk, the IndexError path that creates a snapshot for a choice that appears after the initial chunk still stores choice.delta.to_dict() directly; with interleaved/multi-choice streams, if that choice's first tool_calls delta has duplicate entries for the same tool index, the duplicate physical entries remain and later argument fragments merge into only the first one, corrupting the final tool-call JSON. Apply the same normalization in that new-choice path as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. The except IndexError path in _accumulate_chunk had the same raw choice.delta.to_dict() assignment. Applied _normalize_indexed_list there too in ec1c5ac.
There was a problem hiding this comment.
Note
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
The IndexError path in _accumulate_chunk creates a snapshot for a choice that appears after the first chunk (multi-choice streams). The raw choice.delta.to_dict() was stored directly without normalizing duplicate-index tool_call entries, leaving the same orphan bug as the initial snapshot. Apply _normalize_indexed_list() here as well.
1 participant
Fixes #3201
When the first streamed chunk carries multiple tool_calls entries with the same index, the accumulator stores them as separate physical list entries via early-return paths. Every subsequent delta merges only into the first entry, fragmenting arguments and producing invalid JSON.
Adds
_normalize_indexed_list()that merges entries sharing the same integer index before first storage, applied at both early-return branches inaccumulate_delta. Also normalizes the initial chunk snapshot.