fix(chat-completions): tolerate a missing tool call index when buffering streamed tool calls #4824
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,14 +149,35 @@ def _open_thinking_block(self) -> dict[str, Any]: | |
| class _BufferedToolCall: | ||
| """Accumulates a streamed Chat Completions function tool call.""" | ||
|
|
||
| index: int | None | ||
| call_id: str | None = None | ||
| name: str | None = None | ||
| arguments: str = "" | ||
| provider_specific_fields: dict[str, Any] | None = None | ||
| extra_content: dict[str, Any] | None = None | ||
|
|
||
|
|
||
| def _buffered_tool_calls_in_replay_order( | ||
| buffered_calls: dict[int | None, _BufferedToolCall], | ||
| ) -> list[_BufferedToolCall]: | ||
| """Sort indexed calls while preserving where the index-less call first appeared.""" | ||
| indexed_calls = sorted( | ||
| (call for call in buffered_calls.values() if isinstance(call.index, int)), | ||
| key=lambda call: cast(int, call.index), | ||
| ) | ||
| if None not in buffered_calls: | ||
| return indexed_calls | ||
|
|
||
| unindexed_position = 0 | ||
| for index in buffered_calls: | ||
| if index is None: | ||
| break | ||
| unindexed_position += 1 | ||
|
|
||
| indexed_calls.insert(unindexed_position, buffered_calls[None]) | ||
| return indexed_calls | ||
|
|
||
|
|
||
| def _merge_buffered_metadata( | ||
| current: dict[str, Any] | None, | ||
| incoming: dict[str, Any], | ||
|
|
@@ -307,12 +328,94 @@ def _delta_has_passthrough_output(delta: ChoiceDelta | None) -> bool: | |
|
|
||
| @staticmethod | ||
| def _accumulate_tool_call_delta( | ||
| buffered_calls: dict[int | None, _BufferedToolCall], | ||
| tool_call_delta: ChoiceDeltaToolCall, | ||
| ) -> None: | ||
| tool_call_index = tool_call_delta.index | ||
| function_name = tool_call_delta.function.name if tool_call_delta.function else None | ||
| matching_indexes = [ | ||
| index | ||
| for index, buffered_call in buffered_calls.items() | ||
| if tool_call_delta.id and buffered_call.call_id == tool_call_delta.id | ||
| ] | ||
| if isinstance(tool_call_index, int): | ||
| if None in matching_indexes: | ||
| if len(matching_indexes) > 1: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta supplied an index and the same ID " | ||
| "matched multiple buffered calls." | ||
| ) | ||
| if tool_call_index in buffered_calls: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta supplied an index already used by " | ||
| "another buffered call." | ||
| ) | ||
|
|
||
| buffered_call = buffered_calls.pop(None) | ||
| buffered_call.index = tool_call_index | ||
| buffered_calls[tool_call_index] = buffered_call | ||
| elif ( | ||
| not tool_call_delta.id | ||
| and None in buffered_calls | ||
| and tool_call_index not in buffered_calls | ||
| ): | ||
| buffered_name = buffered_calls[None].name | ||
| if not function_name or not buffered_name or function_name == buffered_name: | ||
|
Comment on lines
+362
to
+363
There was a problem hiding this comment.
When a completed index-less call is followed by a distinct invocation whose opening supplies a new integer index but delays its ID, this condition treats an identical function name as proof that the delta belongs to the old call. The later ID then overwrites the first call's ID while both argument payloads are concatenated, silently dropping one invocation. Fresh evidence beyond the earlier late-index finding is the added different-name test at lines 1698-1742: changing both calls to invoke the same function turns that supported two-call shape into corruption, so this ambiguous case should fail rather than re-key. AGENTS.md reference: AGENTS.md:L147-L147 Useful? React with 👍 / 👎. |
||
| if len(buffered_calls) > 1: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta supplied a new index without an ID " | ||
| "while multiple function calls were being buffered." | ||
| ) | ||
| buffered_call = buffered_calls.pop(None) | ||
| buffered_call.index = tool_call_index | ||
| buffered_calls[tool_call_index] = buffered_call | ||
| else: | ||
| if len(matching_indexes) == 1: | ||
| tool_call_index = matching_indexes[0] | ||
| elif len(matching_indexes) > 1: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta omitted an index and the same ID " | ||
| "matched multiple buffered calls." | ||
| ) | ||
| elif tool_call_delta.id and None in buffered_calls and buffered_calls[None].call_id: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta omitted an index with a new ID while " | ||
| "another index-less call was being buffered." | ||
| ) | ||
| else: | ||
| if function_name and None in buffered_calls: | ||
| buffered_name = buffered_calls[None].name | ||
| if buffered_name and buffered_name != function_name: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta omitted an index and used a " | ||
| "different function name from the buffered index-less call." | ||
| ) | ||
|
|
||
| if not tool_call_delta.id and function_name: | ||
| if None in buffered_calls: | ||
| tool_call_index = None | ||
| elif len(buffered_calls) == 1: | ||
| sole_index = next(iter(buffered_calls)) | ||
| sole_name = buffered_calls[sole_index].name | ||
| if not sole_name or sole_name == function_name: | ||
| tool_call_index = sole_index | ||
| else: | ||
| tool_call_index = None | ||
| else: | ||
| tool_call_index = None | ||
| elif not tool_call_delta.id and None in buffered_calls: | ||
| tool_call_index = None | ||
| elif not tool_call_delta.id and len(buffered_calls) == 1: | ||
| tool_call_index = next(iter(buffered_calls)) | ||
| elif not tool_call_delta.id and len(buffered_calls) > 1: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta omitted an index while multiple " | ||
| "function tool calls were being buffered." | ||
| ) | ||
|
|
||
| buffered_call = buffered_calls.setdefault( | ||
| tool_call_index, | ||
| _BufferedToolCall(index=tool_call_index), | ||
| ) | ||
|
|
||
| if tool_call_delta.id: | ||
|
|
@@ -341,6 +444,7 @@ def _accumulate_tool_call_delta( | |
| @staticmethod | ||
| def _buffered_tool_call_delta( | ||
| buffered_call: _BufferedToolCall, | ||
| fallback_index: int = 0, | ||
| ) -> ChoiceDeltaToolCall: | ||
| if not buffered_call.call_id: | ||
| raise ModelBehaviorError( | ||
|
|
@@ -353,7 +457,7 @@ def _buffered_tool_call_delta( | |
| ) | ||
|
|
||
| tool_call_delta = ChoiceDeltaToolCall( | ||
| index=buffered_call.index if isinstance(buffered_call.index, int) else fallback_index, | ||
| id=buffered_call.call_id, | ||
| function=ChoiceDeltaToolCallFunction( | ||
| name=buffered_call.name, | ||
|
|
@@ -374,11 +478,17 @@ def _buffered_tool_call_delta( | |
| def _buffered_tool_calls_chunk( | ||
| cls, | ||
| template_chunk: ChatCompletionChunk, | ||
| buffered_calls: dict[int | None, _BufferedToolCall], | ||
| passthrough_tool_call_indexes: set[int], | ||
| ) -> ChatCompletionChunk: | ||
| ordered_calls = _buffered_tool_calls_in_replay_order(buffered_calls) | ||
| occupied_indexes = passthrough_tool_call_indexes | { | ||
| call.index for call in ordered_calls if isinstance(call.index, int) | ||
| } | ||
| fallback_index = max(occupied_indexes, default=-1) + 1 | ||
|
Comment on lines
+485
to
+488
There was a problem hiding this comment.
When default non-strict handling receives a custom/passthrough tool-call delta whose provider also omitted AGENTS.md reference: AGENTS.md:L147-L147 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Fixed in dacd81d. Passthrough identity is now recorded only when the provider supplies an integer index, so None cannot enter numeric fallback allocation or capture a later unindexed function call. I added parameterized regression coverage for a missing-index custom call followed by a function call with index 0 and with index omitted; both cases pass, and the complete streaming test module now has 115 passing tests. |
||
| tool_call_deltas = [ | ||
| cls._buffered_tool_call_delta(buffered_call, fallback_index=fallback_index) | ||
| for buffered_call in ordered_calls | ||
|
Comment on lines
489
to
+491
There was a problem hiding this comment.
When a provider supplies AGENTS.md reference: AGENTS.md:L147-L148 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Fixed in 9930941. An arguments-only unindexed continuation now merges into the existing unindexed slot or the sole active buffered function call. If multiple indexed calls make ownership ambiguous, buffering raises ModelBehaviorError before replay. I added regression tests for both the unambiguous merge and ambiguous rejection paths. |
||
| ] | ||
| choice = Choice( | ||
| index=0, | ||
|
|
@@ -393,8 +503,10 @@ async def buffer_tool_call_stream( | |
| stream: AsyncIterator[ChatCompletionChunk], | ||
| ) -> AsyncIterator[ChatCompletionChunk]: | ||
| """Buffer streamed function tool-call deltas until they are complete.""" | ||
| buffered_calls: dict[int | None, _BufferedToolCall] = {} | ||
| passthrough_tool_call_indexes: set[int] = set() | ||
| passthrough_tool_call_indexes_by_id: dict[str, int | None] = {} | ||
| saw_unindexed_passthrough_tool_call = False | ||
| saw_passthrough_tool_call = False | ||
| last_chunk: ChatCompletionChunk | None = None | ||
|
|
||
|
|
@@ -418,13 +530,137 @@ async def buffer_tool_call_stream( | |
| if tool_call_deltas := (delta.tool_calls if delta and delta.tool_calls else None): | ||
| remaining_tool_calls: list[ChoiceDeltaToolCall] = [] | ||
| for tool_call_delta in tool_call_deltas: | ||
| is_untyped_continuation = ( | ||
| getattr(tool_call_delta, "type", None) is None | ||
| and tool_call_delta.function is None | ||
| ) | ||
| is_unindexed_untyped_continuation = ( | ||
| not isinstance(tool_call_delta.index, int) and is_untyped_continuation | ||
| ) | ||
| buffered_id_matches = [ | ||
| buffered_call | ||
| for buffered_call in buffered_calls.values() | ||
| if tool_call_delta.id and buffered_call.call_id == tool_call_delta.id | ||
| ] | ||
| is_passthrough_continuation_by_id = ( | ||
| is_untyped_continuation | ||
| and bool(tool_call_delta.id) | ||
| and tool_call_delta.id in passthrough_tool_call_indexes_by_id | ||
| ) | ||
| is_unindexed_passthrough_continuation = ( | ||
| is_unindexed_untyped_continuation | ||
|
Comment on lines
+550
to
+551
There was a problem hiding this comment.
When a custom/passthrough opening has an ID but omits AGENTS.md reference: AGENTS.md:L147-L147 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Fixed in 122fda0. Untyped continuations now route by a known passthrough ID even when they introduce an integer index; a free index is promoted and restored for later deltas, while owner changes and function-index collisions fail before mutation. Added promotion and collision coverage. |
||
| and ( | ||
| is_passthrough_continuation_by_id | ||
| or (saw_unindexed_passthrough_tool_call and not tool_call_delta.id) | ||
| ) | ||
| ) | ||
| if is_passthrough_continuation_by_id and buffered_id_matches: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta ID matched both a buffered " | ||
| "function call and a passthrough call." | ||
| ) | ||
|
|
||
| unindexed_buffered_call = buffered_calls.get(None) | ||
| if ( | ||
| isinstance(tool_call_delta.index, int) | ||
| and tool_call_delta.index in passthrough_tool_call_indexes | ||
| and unindexed_buffered_call is not None | ||
| and ( | ||
| ( | ||
| tool_call_delta.id | ||
| and unindexed_buffered_call.call_id == tool_call_delta.id | ||
| ) | ||
| or ( | ||
| not tool_call_delta.id | ||
| and len(buffered_calls) == 1 | ||
| and tool_call_delta.function is not None | ||
| ) | ||
| ) | ||
| ): | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta supplied an index already used " | ||
| "by a passthrough call." | ||
| ) | ||
|
|
||
| if ( | ||
| tool_call_delta.index in passthrough_tool_call_indexes | ||
| or is_passthrough_continuation_by_id | ||
| or is_unindexed_passthrough_continuation | ||
| ): | ||
| if passthrough_id := tool_call_delta.id: | ||
| owner_index = passthrough_tool_call_indexes_by_id.get( | ||
| passthrough_id | ||
| ) | ||
| if isinstance(tool_call_delta.index, int): | ||
| promotes_unindexed_passthrough_owner = ( | ||
| is_passthrough_continuation_by_id | ||
| and not isinstance(owner_index, int) | ||
| ) | ||
| if isinstance(owner_index, int) and ( | ||
| owner_index != tool_call_delta.index | ||
| ): | ||
| raise ModelBehaviorError( | ||
| "Chat Completions passthrough tool call delta supplied " | ||
| "a different index from its buffered ID owner." | ||
| ) | ||
| if tool_call_delta.index in buffered_calls: | ||
| raise ModelBehaviorError( | ||
| "Chat Completions passthrough tool call delta supplied " | ||
| "an index already used by a buffered function call." | ||
| ) | ||
| passthrough_tool_call_indexes.add(tool_call_delta.index) | ||
| passthrough_tool_call_indexes_by_id[passthrough_id] = ( | ||
| tool_call_delta.index | ||
| ) | ||
| if promotes_unindexed_passthrough_owner: | ||
| tool_call_delta = tool_call_delta.model_copy( | ||
| update={"type": "custom"} | ||
| ) | ||
| elif isinstance(owner_index, int): | ||
| tool_call_delta = tool_call_delta.model_copy( | ||
| update={"index": owner_index} | ||
| ) | ||
| passthrough_tool_call_indexes_by_id.setdefault( | ||
| passthrough_id, | ||
| tool_call_delta.index | ||
| if isinstance(tool_call_delta.index, int) | ||
| else None, | ||
| ) | ||
| saw_passthrough_tool_call = True | ||
| remaining_tool_calls.append(tool_call_delta) | ||
| elif ( | ||
| is_unindexed_untyped_continuation | ||
| and saw_passthrough_tool_call | ||
| and ( | ||
| not tool_call_delta.id | ||
|
Comment on lines
+632
to
+635
There was a problem hiding this comment.
When a nonzero choice contains a tool call, it sets AGENTS.md reference: AGENTS.md:L147-L148 Useful? React with 👍 / 👎. |
||
| or ( | ||
| bool( | ||
| set(tool_call_delta.model_extra or {}) | ||
| - {"provider_specific_fields", "extra_content"} | ||
| ) | ||
| and not buffered_id_matches | ||
| ) | ||
| ) | ||
| ): | ||
| raise ModelBehaviorError( | ||
| "Chat Completions tool call delta omitted an index, type, and " | ||
| "function payload after a passthrough call, so it could not be " | ||
| "attributed safely." | ||
| ) | ||
| elif cls._should_buffer_tool_call_delta(tool_call_delta): | ||
| cls._accumulate_tool_call_delta(buffered_calls, tool_call_delta) | ||
| else: | ||
| if isinstance(tool_call_delta.index, int): | ||
| passthrough_tool_call_indexes.add(tool_call_delta.index) | ||
| else: | ||
| saw_unindexed_passthrough_tool_call = True | ||
| if tool_call_delta.id: | ||
| passthrough_tool_call_indexes_by_id.setdefault( | ||
| tool_call_delta.id, | ||
| tool_call_delta.index | ||
| if isinstance(tool_call_delta.index, int) | ||
| else None, | ||
| ) | ||
| saw_passthrough_tool_call = True | ||
| remaining_tool_calls.append(tool_call_delta) | ||
|
|
||
|
|
@@ -458,7 +694,11 @@ async def buffer_tool_call_stream( | |
| if buffered_calls: | ||
| if last_chunk is None: | ||
| return | ||
| yield cls._buffered_tool_calls_chunk( | ||
| last_chunk, | ||
| buffered_calls, | ||
| passthrough_tool_call_indexes, | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _merged_provider_data( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the sole opening delta contains the call ID and function name but omits
index, and an arguments continuation later supplies an unused integer index while omittingid,matching_indexesis empty, so this branch leaves the opening underNoneand creates a second buffered call under the integer index. Replay then raises because the continuation lacks an ID/name instead of returning the complete call. Fresh evidence beyond the prior repeated-ID late-index finding is the repository's established continuation shape intest_stream_response_buffers_tool_call_deltas_when_enabled, where subsequent argument deltas omitid; reconcile the soleNoneentry when ownership is unambiguous.AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 122fda0. A sole ID-less late-index continuation now migrates the unindexed call only when function names are compatible; a different explicit name remains a distinct call, and ambiguous multi-call ownership still fails before mutation. Added success, distinct-name, and atomic ambiguity coverage.