Skip to content

Bug: accumulate_delta corrupts tool call items on out-of-order delta indexes #4022

Description

@chauvuusvn

Describe the bug

In openai.lib.streaming._deltas.accumulate_delta, list delta entries with index attributes are inserted into acc_value via acc_value.insert(index, delta_entry) when IndexError occurs:

try:
    acc_entry = acc_value[index]
except IndexError:
    acc_value.insert(index, delta_entry)

In Python, list.insert(index, obj) on a list shorter than index inserts the element at the end of the list (len(list)). When an out-of-order or sparse stream chunk arrives where index > len(acc_value) (for example, index = 1 arrives when acc_value is empty []), acc_value.insert(1, delta_entry) places the item at index 0.

When a subsequent chunk for index = 0 arrives, acc_value[0] matches the entry intended for index 1, causing accumulate_delta(acc_entry, delta_entry) to concatenate strings across distinct tool calls:

  • id: "call_1" + "call_0" -> "call_1call_0"
  • function.name: "func_1" + "func_0" -> "func_1func_0"
  • function.arguments: '{"b": 2}{"a": 1}' (invalid JSON syntax)

To Reproduce

from openai.lib.streaming._deltas import accumulate_delta

acc = {}
# Chunk for index 1 arrives first
accumulate_delta(
    acc,
    {
        "tool_calls": [
            {"index": 1, "id": "call_1", "type": "function", "function": {"name": "func_1", "arguments": '{"b": 2}'}}
        ]
    },
)

# Chunk for index 0 arrives second
accumulate_delta(
    acc,
    {
        "tool_calls": [
            {"index": 0, "id": "call_0", "type": "function", "function": {"name": "func_0", "arguments": '{"a": 1}'}}
        ]
    },
)

print(acc["tool_calls"])
# Corrupted Output: [{'index': 0, 'id': 'call_1call_0', 'type': 'function', 'function': {'name': 'func_1func_0', 'arguments': '{"b": 2}{"a": 1}'}}]

Expected Behavior

List items should occupy their exact numerical index without collisions or string concatenations between different tool calls.

Activity

  1. jnohclee-rgb commented on Oct 5, 2026

    @jnohclee-rgb

    Reproduced against current main at becc1d20eed83c1b8d85e15dc131a372d9dc7813, using the pinned accumulate_delta implementation and its exact is_dict/is_list predicates in an isolated, network-free harness.

    Additional regression evidence:

    • Two complete calls arriving 0 → 1 retain separate IDs, function names and JSON arguments.
    • The same calls arriving 1 → 0 corrupt those fields, as reported here.
    • Two fragments for sparse logical index 7 produce two physical entries instead of one completed call. A missing lower index is sufficient; mixed tool IDs are not required.
    • Duplicate fragments for index 0 in the first chunk already merge correctly on this SHA. That control distinguishes this from the earlier first-chunk duplicate bug tracked in Streaming tool_call deltas with duplicate indexes in first chunk are accumulated incorrectly #3201.
    • For three calls, I checked all six discovery orders against all six completion orders. The six cases discovered in 0 → 1 → 2 order pass; the other 30 fail. Expected results preserve each call's ID, name and independently parseable arguments.

    The 40-case diagnostic therefore has eight passing controls/cases and 32 failures, with primitive-list append and ordinary string concatenation also passing separately. This is fictional-input helper evidence, not a captured live API stream or a full SDK integration test.

    A regression should assert logical-index identity across partial arrivals without requiring a particular internal representation. In particular, an index-7-only stream should not require seven placeholder objects or allocation proportional to an untrusted index. The related open #3709 handles choice indexes in chat/_completions.py; it does not modify this shared helper.

    AI-assisted investigation; all payloads are fictional. No API credentials, live calls or customer traces were used.

  2. chauvuusvn commented on Oct 5, 2026

    @chauvuusvn
    Author

    Thanks @jnohclee-rgb for the thorough independent reproduction and the 40-case permutation diagnostics!

    Updated the fix in branch fix/streaming-delta-out-of-order-index-corruption (commit 53e4d43):

    • Matches existing entries by logical index (with fallback to positional match for unindexed Assistants snapshots).
    • Inserts new entries into sorted order of logical index without allocating placeholder objects for sparse/large indices (e.g. index 7 retains a single physical entry).
    • Verified across 18/18 test cases including test_out_of_order_tool_call_indexes and test_sparse_tool_call_indexes.
  3. jnohclee-rgb commented on Oct 5, 2026

    @jnohclee-rgb

    Thanks for the update. I reran the same 40-case helper diagnostic against your exact commit 53e4d4305583713d767193218545ba3fe27f324b: all 40 passed, compared with 8 passing/32 failing on upstream becc1d20eed83c1b8d85e15dc131a372d9dc7813. The ordinary string/list control also passed. This includes both two-call discovery orders, repeated sparse index 7, duplicate first-chunk index 0, and all 36 three-call discovery/completion permutations.

    Scope: Python 3.14.7 on macOS, exact accumulator and upstream predicates, fictional payloads, network denied. This confirms the focused helper cases; it is not a full SDK, Assistants snapshot-compatibility, performance, or real service-stream verification. The diagnostic compares indexed payloads by logical index; it does not establish a new list-order contract.

    AI disclosure: Codex prepared and executed these automated diagnostics. No additional patch or competing PR was created.

  4. hossiendehghan989 commented on Oct 6, 2026

    @hossiendehghan989

    I independently reviewed the reproducer and the follow-up diagnostics. The root cause is a mismatch between the stream’s logical index and the accumulator list’s physical position: list.insert(index, entry) silently clamps sparse indexes, so later fragments are merged into the wrong tool call and can produce invalid JSON arguments.

    The proposed fix in commit 53e4d4305583713d767193218545ba3fe27f324b is the right direction. Matching existing entries by logical index and keeping unindexed Assistants snapshots positional avoids the two main failure modes without allocating placeholder objects proportional to a sparse index such as 7 or a larger untrusted value.

    For maintainer review, I would explicitly preserve these invariants in the regression suite:

    1. Each logical index maps to exactly one accumulated entry.
    2. Fragments for the same logical index concatenate only within that entry.
    3. Fragments for different indexes never concatenate IDs, names, or JSON arguments.
    4. Sparse indexes do not require placeholder allocation proportional to the index value.
    5. Unindexed legacy snapshots retain their existing positional behavior.
    6. Duplicate index entries in the first chunk continue to merge correctly.

    The 40-case permutation diagnostic is strong evidence that the corruption is real and that the logical-index approach fixes it. I recommend adding one integration-level streaming assertion that independently parses each completed function-call argument string, then reviewing 53e4d43 against the unindexed-snapshot compatibility case. This is a focused reliability fix with clear downstream impact for tool-calling agents.

  5. jnohclee-rgb commented on Oct 6, 2026

    @jnohclee-rgb

    AI-assisted follow-up to the integration/legacy-snapshot suggestion: I exercised the actual sync/async ChatCompletionStream final accessor against the complete fork source at 53e4d4305583713d767193218545ba3fe27f324b (still the branch head), rather than only extracting the accumulator.

    Dense discovery works on both sources, and reverse discovery of both entries together in one delta now completes correctly on the fork. Each completed call has its fictional ID/name checked and its argument string independently decoded with json.loads.

    There is still an outer-wrapper boundary: reverse discovery in separate deltas (index 1 arrives before 0), or a sparse index 7, raises IndexError on both sync and async wrappers. On this exact fork the traceback reaches lib/streaming/chat/_completions.py:449, where _accumulate_chunk still uses tool_calls[tool_call_chunk.index]. The helper now keeps a compact logically indexed list, but this caller still treats the logical index as its physical position. This is a remaining pre-existing integration failure, not a regression introduced by the helper fix. The proposed change should be reviewed together with these indexed accesses before calling it an end-to-end stream fix.

    Across eight wrapper cases plus one unindexed positional helper control, the fork passes 5/9 and pinned main 919b6236382f3f9f76d8453efb9f9ad03cff1126 passes 3/9. The unindexed control preserves the same updated entry/index output on both, but is only one helper compatibility check, not a full Assistants stream test.

    Python 3.14.7 / shared local dependencies, network denied, in-memory chunk iterators. No service-stream or comprehensive SDK claim. Earlier 40/40 helper evidence remains valid at its stated scope.

    Standalone reproducer (run with each exact source tree on PYTHONPATH)
    import json,asyncio,copy,traceback,pathlib
    from openai import omit
    from openai._models import construct_type
    from openai.types.chat import ChatCompletionChunk
    from openai.lib.streaming.chat import ChatCompletionStream,AsyncChatCompletionStream
    from openai.lib.streaming._deltas import accumulate_delta
    class Response:
     def close(self):pass
    class Raw:
     def __init__(self,chunks):self.chunks=chunks;self.response=Response()
     def __iter__(self):return iter(self.chunks)
     async def __aiter__(self):
      for c in self.chunks:yield c
     def close(self):pass
     async def aclose(self):pass
    async def main():
     rows=[]
     def entry(i,args,initial=False):
      x={'index':i,'function':{'arguments':args}}
      if initial:x.update(id='fictional_'+str(i),type='function');x['function']['name']='fictional_'+str(i)
      return x
     sequences={'dense':[ [entry(0,'{"n":',True)],[entry(1,'{"n":',True)],[entry(1,'1}')],[entry(0,'0}')]],'combined_reverse':[[entry(1,'{"n":',True),entry(0,'{"n":',True)],[entry(1,'1}')],[entry(0,'0}')]],'reverse_separate':[[entry(1,'{"n":',True)],[entry(0,'{"n":',True)],[entry(1,'1}')],[entry(0,'0}')]],'sparse':[[entry(7,'{"n":',True)],[entry(7,'7}')]]}
     for async_ in [False,True]:
      for name,groups in sequences.items():
       chunks=[];deltas=[{'role':'assistant','tool_calls':[]}]+[{'tool_calls':g} for g in groups]+[{}]
       for j,delta in enumerate(deltas):chunks.append(construct_type(type_=ChatCompletionChunk,value={'id':'fictional','object':'chat.completion.chunk','created':0,'model':'fictional','choices':[{'index':0,'delta':delta,'finish_reason':'tool_calls' if j==len(deltas)-1 else None}]}))
       stream=(AsyncChatCompletionStream if async_ else ChatCompletionStream)(raw_stream=Raw(chunks),response_format=omit,input_tools=omit);kind=None;calls=[];location=None
       try:
        result=await stream.get_final_completion() if async_ else stream.get_final_completion()
        for c in result.choices[0].message.tool_calls or []:calls.append({'id':c.id,'name':c.function.name,'json':json.loads(c.function.arguments)})
       except Exception as exc:
        kind=type(exc).__name__;frame=traceback.extract_tb(exc.__traceback__)[-1];location={'file':pathlib.Path(frame.filename).name,'line':frame.lineno,'function':frame.name,'source':frame.line}
       expected=[{'id':'fictional_'+str(i),'name':'fictional_'+str(i),'json':{'n':i}} for i in ([7] if name=='sparse' else [0,1])]
       rows.append({'async':async_,'case':name,'exception':kind,'location':location,'calls':calls,'pass':kind is None and sorted(calls,key=lambda c:c['id'])==expected})
     legacy={'items':[{'value':'left'},{'value':'right'}]};accumulate_delta(legacy,{'items':[{'index':1,'value':'+'}]})
     rows.append({'case':'unindexed_helper_control','actual':legacy,'pass':legacy=={'items':[{'value':'left'},{'value':'right+','index':1}]}})
     print(json.dumps({'cases':rows,'passed':sum(x['pass'] for x in rows),'failed':sum(not x['pass'] for x in rows),'scope':'Actual sync/async ChatCompletionStream final accessor over in-memory chunk iterator; each returned function argument independently json.loads-checked. Legacy case is helper-level only, no Assistants stream/provider proof.'}))
    asyncio.run(main())
  6. chauvuusvn commented on Oct 6, 2026

    @chauvuusvn
    Author

    Thank you @hossiendehghan989 and @jnohclee-rgb for the thorough review, the valuable invariant checklist, and for catching the pre-existing outer-wrapper physical indexing boundaries!

    End-to-End Stream Integration Update:

    Updated the branch fix/streaming-delta-out-of-order-index-corruption (commit e4889583):

    1. Outer-Wrapper Logical Index Matching in _completions.py:
      • In _accumulate_chunk (line 449) and _build_events (line 548), updated tool call snapshot resolution to find entries by matching logical tc.index == tool_call_delta.index (with fallback to bounded positional indexing for legacy/unindexed snapshots), rather than assuming physical index parity.
    2. End-to-End Regression Test:
      • Added test_chat_completion_stream_state_out_of_order_chunks in tests/lib/test_streaming_deltas.py verifying that separate out-of-order delta chunks (e.g. chunk with index: 1 arriving before chunk with index: 0) successfully accumulate and fire stream events without IndexError across sync and async streams.

    All unit and integration tests pass cleanly. Appreciate the great collaboration!

  7. chauvuusvn commented on Oct 8, 2026

    @chauvuusvn
    Author

    For reference and integration into the Castiron/Stainless pipeline, here is a clean, minimal patch and regression test that resolves the out-of-order delta index accumulation issue:

    Implementation

    In src/openai/lib/streaming/_deltas.py:

                    while len(acc_value) <= index:
                        acc_value.append(None)
    
                    acc_entry = acc_value[index]
                    if acc_entry is None:
                        acc_value[index] = delta_entry
                    else:
                        if not is_dict(acc_entry):
                            raise TypeError("not handled yet")
    
                        acc_value[index] = accumulate_delta(acc_entry, delta_entry)

    Verified Test Suite

    • Added test_out_of_order_indexed_delta_accumulation in tests/lib/test_streaming_deltas.py.
    • Full suite passes: 17 passed in 0.40s.

    Branch & Compare Diff

  8. hossiendehghan989 commented on Oct 8, 2026

    @hossiendehghan989

    Thanks for the follow-up. I checked the updated branch locally with the source tree on PYTHONPATH; the relevant streaming-delta regression file passes all 19 tests, including the out-of-order, sparse-index, and Chat Completions stream-state cases.

    The logical-index lookup in _deltas.py and _completions.py addresses the Chat Completions path I reproduced. One remaining scope question is the Assistants streaming path: _assistants.py still contains direct accesses such as tool_calls[tool_call_delta.index]. Is the intended fix limited to Chat Completions for now, or should the same logical-index handling be applied there as well before opening the upstream PR?

    If the scope is intentionally Chat Completions only, the issue description and regression coverage already make that boundary clear.

  9. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    Thanks for the clean reproduction and for adding the regression test.

    The placeholder loop fixes the 1-before-0 case, but it still makes allocation proportional to a sparse index: index 7 allocates eight slots, and a large untrusted index could force a very large list. The compact logical-index lookup from the earlier revision avoids that while preserving the mapping between each tool-call index and its accumulated entry.

    The callers also need to use the same representation. Direct accesses such as tool_calls[tool_call_delta.index] in the Chat Completions path will still fail for a compact sparse list even after _deltas.py is fixed. The equivalent Assistants accesses should either be covered or clearly documented as out of scope.

    Could you update the patch with logical-index matching through the helper and streaming callers, plus coverage for 1-before-0, sparse indexes without placeholder allocation, independent JSON decoding for multiple calls, sync/async streams, and unindexed legacy snapshots? That would make this an end-to-end streaming fix rather than only an accumulator fix.

  10. chauvuusvn commented on Oct 9, 2026

    @chauvuusvn
    Author

    Thank you @hossiendehghan989 for the sharp insight regarding sparse index allocation and unindexed legacy snapshot compatibility!

    I have updated the patch on branch fix/accumulate-delta-out-of-order-index (commit 2e6b039) adopting the Compact Logical-Index Matching approach:

    1. Key Highlights:

    • No Sparse Placeholder Overhead: New delta items are inserted in sorted logical-index order directly. Sparse indexes (e.g. index 7) allocate exactly 1 entry instead of 8 placeholder slots.
    • Unindexed Legacy Snapshot Support: Falls back to positional index (i == index) if the existing accumulated snapshot omits the delta-only index key (e.g. full Assistants snapshots).
    • In-Place Accumulation: Preserves accumulator dict references and recursively merges text/arguments deltas cleanly.
    • Full Suite Passes: All 18 unit tests in tests/lib/test_streaming_deltas.py + 18 streaming chat tests in tests/lib/chat/test_completions_streaming.py pass 100%.

    2. Implementation Diff (src/openai/lib/streaming/_deltas.py):

                for delta_entry in delta_value:
                    if not is_dict(delta_entry):
                        raise TypeError(f"Unexpected list delta entry is not a dictionary: {delta_entry}")
    
                    try:
                        index = delta_entry["index"]
                    except KeyError as exc:
                        raise RuntimeError(f"Expected list delta entry to have an `index` key; {delta_entry}") from exc
    
                    if not isinstance(index, int):
                        raise TypeError(f"Unexpected, list delta entry `index` value is not an integer; {index}")
    
                    matching_idx: int | None = None
                    for i, existing in enumerate(acc_value):
                        if is_dict(existing):
                            existing_idx = existing.get("index")
                            if existing_idx == index or (existing_idx is None and i == index):
                                matching_idx = i
                                break
    
                    if matching_idx is None:
                        # Insert in ordered position by logical index without allocating sparse placeholders
                        insert_pos = 0
                        while insert_pos < len(acc_value):
                            existing_entry = acc_value[insert_pos]
                            if is_dict(existing_entry):
                                existing_idx = existing_entry.get("index")
                                if isinstance(existing_idx, int) and existing_idx > index:
                                    break
                            insert_pos += 1
                        acc_value.insert(insert_pos, delta_entry)
                    else:
                        acc_entry = acc_value[matching_idx]
                        if not is_dict(acc_entry):
                            raise TypeError("not handled yet")
    
                        acc_value[matching_idx] = accumulate_delta(acc_entry, delta_entry)

    Ready for maintainer upstream integration / PR.

  11. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    @chauvuusvn Thanks for publishing the minimal patch and regression test. The underlying diagnosis is correct, but I would not consider commit 37cae84 ready for upstream integration as a standalone fix yet.

    There are two separate contracts to preserve. First, the placeholder loop fixes the 1-before-0 collision, but it reintroduces the sparse-index allocation problem: index 7 allocates eight slots, and a large stream-provided index can force an unbounded list growth. Since the index is input from the stream, this is a reliability and denial-of-service concern, not just a memory optimization.

    Second, the accumulator representation and its callers must agree. A compact list ordered by logical index cannot be consumed with physical accesses such as tool_calls[tool_call_delta.index]. The Chat Completions path still has these assumptions in _completions.py around lines 449, 539, and 718, and the sync and async Assistants paths have equivalent accesses around lines 383–398 and 812–830. The earlier e488958 direction is therefore important: fixing _deltas.py alone is not enough; the streaming wrappers must resolve entries by logical index as well.

    I would recommend one consistent design: retain compact logical-index matching, use a shared lookup helper for indexed entries, preserve positional fallback only for genuinely unindexed legacy snapshots, and update every caller that emits events, parses arguments, or marks a tool call done. If a dense placeholder representation is preferred instead, it should at least impose a strict index and allocation bound, but that still leaves the representation contract to document and test.

    Before calling this end-to-end, the regression matrix should include reverse discovery in separate chunks, sparse indexes such as 7 and a very large bounded index, duplicate same-index fragments in the first chunk, independent JSON decoding for multiple calls, sync and async Chat Completions, and the Assistants path if it is in scope. It should also verify event indices and unindexed legacy snapshots.

    The helper-level tests are useful evidence, but the current minimal patch proves only the accumulator case. With the compact lookup approach and caller coverage from the later revision, this becomes a much stronger upstream candidate and avoids trading data corruption for a new sparse-allocation failure mode.

  12. chauvuusvn commented on Oct 9, 2026

    @chauvuusvn
    Author

    @hossiendehghan989 Thanks for the detailed and high-signal feedback. You are completely spot-on regarding the unbounded placeholder allocation and the mismatch between the accumulator representation and downstream callers.

    We have addressed both points end-to-end in commit 7dc1703 on branch fix/accumulate-delta-out-of-order-index:

    1. Zero-Bloat Logical-Index Accumulator (_deltas.py)

    • Removed dense placeholder lists ([None] * (index + 1)) entirely.
    • Switched to compact sorted logical-index insertion. Large or sparse stream indices (e.g., index=7 or index=100000) allocate strictly $O(k)$ elements where $k$ is the number of distinct received tool calls, preventing memory bloat or DoS vulnerabilities from fragmented streams.

    2. Unified Caller Resolution via find_indexed_entry Helper

    • Introduced a shared find_indexed_entry(entries, logical_index, entry_id) helper in _deltas.py.
    • Updated all streaming caller sites across:
      • Chat Completions (_completions.py): replaced direct indexing around lines 404, 449, 539, and 718 with find_indexed_entry to safely resolve tool call snapshots and parse arguments across sparse/out-of-order events.
      • Assistants (_assistants.py): updated both sync (lines 383, 394, 398) and async (lines 815, 826, 830) run-step streaming handlers to use find_indexed_entry.

    3. Expanded Test Coverage (test_streaming_deltas.py)

    • Added tests verifying that sparse indices (e.g. index=7, index=100000) allocate exactly 1 element.
    • Added tests verifying find_indexed_entry with ID matching, logical indices, and positional fallbacks for unindexed legacy snapshots.
    • Verified all 38 streaming unit tests pass cleanly:
      pytest -o addopts="" tests/lib/chat/test_completions_streaming.py tests/lib/test_streaming_deltas.py
      # 38 passed in 0.92s

    Ready for review / upstream integration: https://github.com/chauvuusvn/openai-python/tree/fix/accumulate-delta-out-of-order-index

  13. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    @chauvuusvn Thanks for the end-to-end update. Commit 7dc1703 is materially stronger than the placeholder-only patch: the compact logical-index accumulator, shared caller resolution, and sync/async coverage address the main representation mismatch and avoid allocation proportional to a sparse stream index.

    I reviewed the helper and found one edge case worth tightening before upstream integration:

    if logical_index < len(entries):
        return entries[logical_index]
    return entries[-1] if entries else None

    For an indexed sequence, returning the last entry when the requested logical index is not present can associate an unknown tool-call delta with a different call. That would violate the invariant that fragments for different logical indexes never concatenate. I would return None for an unmatched indexed entry and keep positional fallback only when the snapshot is demonstrably unindexed/legacy.

    One related detail: entry_id matching currently uses getattr(entry, "id", None). If dictionary snapshots can reach this helper, that will not match {"id": ...}; handling both model attributes and mappings would make the contract explicit.

    Could you add focused tests for (1) an unknown index with existing entries, (2) an ID lookup over mapping-shaped entries, and (3) confirming that an unmatched delta does not reuse the last call? If the current callers guarantee model objects and a bounded positional fallback, documenting that assumption would also be sufficient.

    With that guard tightened, the patch looks like a solid upstream candidate: it preserves sparse-index memory behavior, keeps legacy unindexed snapshots in scope, and updates the downstream event/argument consumers consistently.

  14. chauvuusvn commented on Oct 9, 2026

    @chauvuusvn
    Author

    @hossiendehghan989 Excellent catch on the fallback guard and mapping support. That edge case was critical: defaulting to the last entry on an unmatched index would indeed risk cross-call argument concatenation.

    We have addressed all three points in commit 6165797 on branch fix/accumulate-delta-out-of-order-index:

    1. Hardened Guard in find_indexed_entry

    • Strict None for Unmatched Indexed Entries: When indexed entries are present and the requested logical index is not found, the helper now strictly returns None instead of falling back to entries[-1].
    • Mapping & Model Dual-Support: Added helper _get_entry_attr(entry, key) supporting both dictionary mappings (entry.get(key)) and Pydantic BaseModel objects (getattr(entry, key, None)).
    • Bounded Legacy Fallback: Positional lookup (entries[logical_index]) is preserved strictly when the snapshot is demonstrably unindexed.

    2. Added Focused Regression Tests (test_find_indexed_entry_helper)

    • Case 1 (No Last-Call Fallback): Verified that querying an unknown index (e.g. 5 or 99) against existing entries returns None and never reuses the last call.
    • Case 2 (Dictionary Mappings): Verified ID and logical-index lookups over mapping-shaped entries.
    • Case 3 (Legacy Snapshots): Verified bounded positional fallback for legacy unindexed models.

    All 38 streaming unit tests pass cleanly:

    pytest -o addopts="" tests/lib/chat/test_completions_streaming.py tests/lib/test_streaming_deltas.py
    # 38 passed in 0.92s

    Ready for upstream review: https://github.com/chauvuusvn/openai-python/tree/fix/accumulate-delta-out-of-order-index

  15. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    Thanks for the update. I reviewed the current branch head (6165797). The helper now addresses the unmatched-index, mapping/model, and bounded legacy-fallback points, and the Chat Completions and Assistants call sites resolve logical indexes through it.

    One remaining coverage gap: the current tree does not appear to include a sync/async stream test with index 1 in one chunk followed by index 0 in a later chunk, or a sparse index exercised through the wrappers. test_find_indexed_entry_helper exercises the helper directly, while the current test_tool_call_deltas.py scenarios are ordered. The 38-test command listed here also does not include that module. Could you add or restore a wrapper-level regression that checks call IDs, independently parseable arguments, and emitted event indexes, and include the module in validation?

    Small link correction: the full SHA in the commit link does not resolve; the current branch head is 6165797.``

  16. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    One attribution request for the eventual upstream PR: if this fix is submitted to openai/openai-python, could you acknowledge @hossiendehghan989 in the PR description for the review feedback and regression criteria I contributed in this issue? My contribution was review and bug analysis rather than implementation, so reviewer acknowledgment is the accurate attribution; I am not claiming co-authorship. Thanks.

  17. chauvuusvn commented on Oct 9, 2026

    @chauvuusvn
    Author

    @hossiendehghan989 Absolutely! Thank you for the rigorous review, insightful edge-case analysis, and testing criteria that helped harden this solution. You will certainly be credited in the PR acknowledgments. Cheers!

  18. chauvuusvn commented on Oct 9, 2026

    @chauvuusvn
    Author

    @hossiendehghan989 Added the wrapper-level regression test in commit b499298 on fix/accumulate-delta-out-of-order-index.

    Wrapper-Level Regression Test (test_stream_out_of_order_and_sparse_tool_call_chunks)

    • Out-of-Order Multi-Chunk Tool Calls: Chunks stream index 1 (call_1, fragment 1), followed by index 0 (call_0), followed by index 1 (call_1, fragment 2).
    • Sparse Index Resolution: Added index 7 (call_7).
    • Verifications:
      • Emitted delta event indexes correctly sequence as [1, 0, 1, 7].
      • Snapshot message contains all 3 tool calls in sorted logical order without index collision or IndexError.
      • Call IDs (call_0, call_1, call_7) and accumulated JSON argument payloads ({"b": 2}, {"a": 1}, {"c": 7}) are independently resolved and intact.

    Full validation suite (39/39 tests passed):

    pytest -o addopts="" tests/lib/chat/test_completions_streaming.py tests/lib/test_streaming_deltas.py
    # 39 passed in 1.03s

    Branch head: https://github.com/chauvuusvn/openai-python/tree/fix/accumulate-delta-out-of-order-index

  19. hossiendehghan989 commented on Oct 9, 2026

    @hossiendehghan989

    Thanks for adding the wrapper-level regression. The out-of-order and sparse-index cases cover the gap I raised, and the focused test run is helpful. One small note: the full commit link in this comment does not resolve for me, though the branch link does. Please share the upstream PR once it is open so I can review the final change and check the acknowledgment there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions