fix(streaming): merge tool_call deltas by logical index, not physical position#3245
fix(streaming): merge tool_call deltas by logical index, not physical position#3245nileshpatil6 wants to merge 1 commit into
Conversation
… position Fixes accumulate_delta treating the \"index\" field as a physical list position. Servers using speculative decoding (e.g. vLLM) can emit two entries with the same logical index in a single chunk. The old code stored both and later merged subsequent deltas only into acc_value[0], leaving the second duplicate stranded with an incomplete arguments string. The fix builds a logical-index-to-physical-position map over acc_value and uses it for all lookups, so entries are always matched by their \"index\" field regardless of how many duplicates arrived in earlier chunks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42462de35b
ℹ️ 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".
| acc_index_map[index] = len(acc_value) | ||
| acc_value.append(delta_entry) |
There was a problem hiding this comment.
Preserve logical index ordering when adding new delta entries
Appending unseen logical indices to the end of acc_value breaks the invariant that list position matches each entry's index, so out-of-order first appearances (e.g., index 1 chunk arrives before index 0) produce misordered arrays. Downstream streaming code indexes these arrays by logical index (for example choice_snapshot.message.tool_calls[tool_call_chunk.index] in src/openai/lib/streaming/chat/_completions.py), which can then read the wrong tool call or raise IndexError for valid streams from providers that interleave indices. The previous insert(index, ...) behavior maintained positional alignment.
Useful? React with 👍 / 👎.
Problem
accumulate_deltainsrc/openai/lib/streaming/_deltas.pylooks up existing list entries using the\"index\"field as a physical list position (acc_value[index]). This breaks when a single streamed chunk contains multiple tool_calls entries that share the same logicalindex.Servers that use speculative decoding (e.g. vLLM with Kimi K2.6) emit a first chunk like:
{ \"delta\": { \"tool_calls\": [ {\"index\": 0, \"id\": \"functions.list_files:0\", \"function\": {\"name\": \"list_files\"}, \"type\": \"function\"}, {\"index\": 0, \"function\": {\"arguments\": \" {\\\"\"}} ] } }Because
key not in accis true on the first chunk, the list is stored as-is with two physical entries both carryingindex: 0. Later chunks withindex: 0get merged intoacc_value[0](the first physical entry), leaving the second entry permanently stranded. The final arguments JSON is split across two objects and is invalid.Fix
Instead of
acc_value[index](positional), build alogical index -> physical positionmap overacc_valuebefore processing each delta chunk. Subsequent entries are matched by logical index through this map, so duplicates within the same chunk are merged into one entry and later deltas always land in the right place.This matches the approach used by the Go SDK, which uses the index directly.
Fixes #3201