diff --git a/NIGHTLY_CODEX_FINAL_ATTEMPT_1.md b/NIGHTLY_CODEX_FINAL_ATTEMPT_1.md new file mode 100644 index 0000000000..cc9af0dde4 --- /dev/null +++ b/NIGHTLY_CODEX_FINAL_ATTEMPT_1.md @@ -0,0 +1,14 @@ +Implemented a small fix for `openai/openai-python#2806`. + +Changed: +- `src/openai/lib/streaming/chat/_completions.py`: normalizes streamed tool-call delta indices so `index: null` falls back to the delta’s position before accumulation/event emission. +- `tests/lib/chat/test_completions_streaming.py`: adds a regression test for tool-call argument streaming with missing/null indices. +- `NIGHTLY_REPORT.md`: added summary, tests, and remaining risk. + +Verified: +- `pytest tests/lib/chat/test_completions_streaming.py` passes: 16 passed. +- `ruff format` and `ruff check` pass on touched files. +- `mypy src/openai/lib/streaming/chat/_completions.py` passes. +- `git diff --check` passes. + +Note: `.codex-nightly-prompt.md` and `uv.lock` were already untracked in the checkout; I left them alone. \ No newline at end of file diff --git a/src/openai/lib/streaming/chat/_completions.py b/src/openai/lib/streaming/chat/_completions.py index 5f072cafbd..c1de80ca4b 100644 --- a/src/openai/lib/streaming/chat/_completions.py +++ b/src/openai/lib/streaming/chat/_completions.py @@ -39,7 +39,7 @@ from ....types.chat import ChatCompletionChunk, ParsedChatCompletion, ChatCompletionToolUnionParam from ...._exceptions import LengthFinishReasonError, ContentFilterFinishReasonError from ....types.chat.chat_completion import ChoiceLogprobs -from ....types.chat.chat_completion_chunk import Choice as ChoiceChunk +from ....types.chat.chat_completion_chunk import Choice as ChoiceChunk, ChoiceDeltaToolCall from ....types.chat.completion_create_params import ResponseFormat as ResponseFormatParam @@ -393,7 +393,7 @@ def _accumulate_chunk(self, chunk: ChatCompletionChunk) -> ParsedChatCompletionS ), ), ), - cast("dict[object, object]", choice.delta.to_dict()), + _choice_delta_to_dict(choice), ), ), ) @@ -415,7 +415,7 @@ def _accumulate_chunk(self, chunk: ChatCompletionChunk) -> ParsedChatCompletionS type_=ParsedChoiceSnapshot, value={ **choice.model_dump(exclude_unset=True, exclude={"delta"}), - "message": choice.delta.to_dict(), + "message": _choice_delta_to_dict(choice), }, ), ) @@ -445,8 +445,9 @@ def _accumulate_chunk(self, chunk: ChatCompletionChunk) -> ParsedChatCompletionS partial_mode=True, ) - for tool_call_chunk in choice.delta.tool_calls or []: - tool_call_snapshot = (choice_snapshot.message.tool_calls or [])[tool_call_chunk.index] + for tool_call_chunk_position, tool_call_chunk in enumerate(choice.delta.tool_calls or []): + tool_call_index = _tool_call_delta_index(tool_call_chunk, fallback_index=tool_call_chunk_position) + tool_call_snapshot = (choice_snapshot.message.tool_calls or [])[tool_call_index] if tool_call_snapshot.type == "function": input_tool = get_input_tool_by_name( @@ -531,8 +532,9 @@ def _build_events( tool_calls = choice_snapshot.message.tool_calls assert tool_calls is not None - for tool_call_delta in choice.delta.tool_calls: - tool_call = tool_calls[tool_call_delta.index] + for tool_call_delta_position, tool_call_delta in enumerate(choice.delta.tool_calls): + tool_call_index = _tool_call_delta_index(tool_call_delta, fallback_index=tool_call_delta_position) + tool_call = tool_calls[tool_call_index] if tool_call.type == "function": assert tool_call_delta.function is not None @@ -541,7 +543,7 @@ def _build_events( FunctionToolCallArgumentsDeltaEvent, type="tool_calls.function.arguments.delta", name=tool_call.function.name, - index=tool_call_delta.index, + index=tool_call_index, arguments=tool_call.function.arguments, parsed_arguments=tool_call.function.parsed_arguments, arguments_delta=tool_call_delta.function.arguments or "", @@ -617,8 +619,10 @@ def get_done_events( tool_index=self.__current_tool_call_index, ) - for tool_call in choice_chunk.delta.tool_calls or []: - if self.__current_tool_call_index != tool_call.index: + for tool_call_position, tool_call in enumerate(choice_chunk.delta.tool_calls or []): + tool_call_index = _tool_call_delta_index(tool_call, fallback_index=tool_call_position) + + if self.__current_tool_call_index != tool_call_index: events_to_fire.extend( self._content_done_events(choice_snapshot=choice_snapshot, response_format=response_format) ) @@ -630,7 +634,7 @@ def get_done_events( tool_index=self.__current_tool_call_index, ) - self.__current_tool_call_index = tool_call.index + self.__current_tool_call_index = tool_call_index return events_to_fire @@ -744,7 +748,7 @@ def _convert_initial_chunk_into_snapshot(chunk: ChatCompletionChunk) -> ParsedCh for choice in chunk.choices: choices[choice.index] = { **choice.model_dump(exclude_unset=True, exclude={"delta"}), - "message": choice.delta.to_dict(), + "message": _choice_delta_to_dict(choice), } return cast( @@ -760,6 +764,29 @@ def _convert_initial_chunk_into_snapshot(chunk: ChatCompletionChunk) -> ParsedCh ) +def _choice_delta_to_dict(choice: ChoiceChunk) -> dict[object, object]: + delta = cast("dict[object, object]", choice.delta.to_dict()) + tool_call_deltas = choice.delta.tool_calls + if not tool_call_deltas: + return delta + + tool_call_dicts = cast("list[dict[object, object]]", delta.get("tool_calls")) + for tool_call_position, tool_call_delta in enumerate(tool_call_deltas): + tool_call_dicts[tool_call_position]["index"] = _tool_call_delta_index( + tool_call_delta, fallback_index=tool_call_position + ) + + return delta + + +def _tool_call_delta_index(tool_call_delta: ChoiceDeltaToolCall, *, fallback_index: int) -> int: + index = cast("int | None", tool_call_delta.index) + if index is None: + return fallback_index + + return index + + def _is_valid_chat_completion_chunk_weak(sse_event: ChatCompletionChunk) -> bool: # Although the _raw_stream is always supposed to contain only objects adhering to ChatCompletionChunk schema, # this is broken by the Azure OpenAI in case of Asynchronous Filter enabled. diff --git a/tests/lib/chat/test_completions_streaming.py b/tests/lib/chat/test_completions_streaming.py index eb3a0973ac..5491562a89 100644 --- a/tests/lib/chat/test_completions_streaming.py +++ b/tests/lib/chat/test_completions_streaming.py @@ -20,6 +20,7 @@ from openai import OpenAI, AsyncOpenAI from openai._utils import consume_sync_iterator, assert_signatures_in_sync from openai._compat import model_copy +from openai._models import construct_type from openai.types.chat import ChatCompletionChunk from openai.lib.streaming.chat import ( ContentDoneEvent, @@ -1068,6 +1069,99 @@ def streamer(client: OpenAI) -> Iterator[ChatCompletionChunk]: ) +def test_chat_completion_state_handles_tool_call_delta_without_index(monkeypatch: pytest.MonkeyPatch) -> None: + state = ChatCompletionStreamState() + events: list[ChatCompletionStreamEvent[object]] = [] + + events.extend( + state.handle_chunk( + _make_chat_completion_chunk( + { + "role": "assistant", + "tool_calls": [ + { + "index": 0, + "id": "call_123", + "type": "function", + "function": {"name": "get_weather", "arguments": ""}, + } + ], + } + ) + ) + ) + events.extend( + state.handle_chunk( + _make_chat_completion_chunk({"tool_calls": [{"index": None, "function": {"arguments": '{"city"'}}]}) + ) + ) + events.extend( + state.handle_chunk( + _make_chat_completion_chunk({"tool_calls": [{"index": None, "function": {"arguments": ':"Chicago"}'}}]}) + ) + ) + events.extend(state.handle_chunk(_make_chat_completion_chunk({}, finish_reason="tool_calls"))) + + assert print_obj(events[-1], monkeypatch) == snapshot("""\ +FunctionToolCallArgumentsDoneEvent( + arguments='{"city":"Chicago"}', + index=0, + name='get_weather', + parsed_arguments=None, + type='tool_calls.function.arguments.done' +) +""") + + assert print_obj(state.get_final_completion().choices, monkeypatch) == snapshot( + """\ +[ + ParsedChoice( + finish_reason='tool_calls', + index=0, + logprobs=None, + message=ParsedChatCompletionMessage( + annotations=None, + audio=None, + content=None, + function_call=None, + parsed=None, + refusal=None, + role='assistant', + tool_calls=[ + ParsedFunctionToolCall( + function=ParsedFunction(arguments='{"city":"Chicago"}', name='get_weather', parsed_arguments=None), + id='call_123', + index=0, + type='function' + ) + ] + ) + ) +] +""" + ) + + +def _make_chat_completion_chunk( + delta: dict[str, object], + *, + finish_reason: Literal["stop", "length", "tool_calls", "content_filter", "function_call"] | None = None, +) -> ChatCompletionChunk: + return cast( + ChatCompletionChunk, + construct_type( + type_=ChatCompletionChunk, + value={ + "id": "chatcmpl_123", + "choices": [{"delta": delta, "finish_reason": finish_reason, "index": 0}], + "created": 1721075651, + "model": "gpt-4o-2024-08-06", + "object": "chat.completion.chunk", + }, + ), + ) + + @pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"]) def test_stream_method_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None: checking_client: OpenAI | AsyncOpenAI = client if sync else async_client