Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e69bf216fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if acc_entry.get("index") == index: | ||
| acc_value[acc_index] = accumulate_delta(acc_entry, delta_entry) |
There was a problem hiding this comment.
Preserve positional fallback for unindexed assistant snapshots
For Assistants run steps, run_step_snapshots are seeded from thread.run.step.created with final RunStep tool-call objects, whose generated types do not include an index; only subsequent delta objects carry one. With this new lookup, an existing unindexed entry never matches delta_entry["index"], so the first delta for tool call 0 is appended as a duplicate instead of being merged into the existing slot. The event handler later reads step_snapshot.step_details.tool_calls[tool_call_delta.index], so code-interpreter/function deltas for created run steps can surface stale or empty snapshots rather than the accumulated arguments/input.
Useful? React with 👍 / 👎.
|
Still valid on current main — when a provider interleaves deltas from several parallel tool calls, the stream merger keys on arrival order rather than the tool-call |
e69bf21 to
b234505
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b234505c7c
ℹ️ 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".
| choices[choice.index] = { | ||
| **choice.model_dump(exclude_unset=True, exclude={"delta"}), | ||
| "message": choice.delta.to_dict(), | ||
| "message": accumulate_delta({}, cast("dict[object, object]", choice.delta.to_dict())), |
There was a problem hiding this comment.
Normalize duplicate indexes for later-arriving choices
This normalization only runs for choices present in the stream's first chunk. When streaming multiple choices, a choice may first appear in a later chunk; the except IndexError path in _accumulate_chunk still constructs its message directly from choice.delta.to_dict(). If that choice's first delta contains duplicate tool-call entries for one logical index, they remain separate, subsequent events read only the first physical entry, and the final completion retains a stale duplicate. Apply the same accumulator initialization when constructing a newly encountered choice.
Useful? React with 👍 / 👎.
|
Thanks for taking the time to work on this! We’ve merged the fix for duplicate-index tool-call deltas in #3425, so I’m closing this as a duplicate. We appreciate the contribution. |
Summary
Fix the streaming delta accumulator so indexed lists are merged by their logical
indexfield even when the first chunk already contains duplicate entries for the same index.This keeps OpenAI-compatible streaming tool-call chunks like this from being stranded as two physical list entries:
index: 0index: 0index: 0The chat-completions initial snapshot now runs the initial delta through the same accumulator path as later chunks, and the Assistants accumulator gets the same first-chunk handling.
Closes #3201.
To verify
I also ran
python -m pyright tests\lib\chat\test_completions_streaming.py; it still reports existinginline_snapshotunknown-type noise in that file, unrelated to this patch.