fix(responses): normalize streamed function call names - #3605
fix(responses): normalize streamed function call names#3605scarab-systems wants to merge 2 commits into
Conversation
Handle Responses API streams that emit response.function_call_arguments.done without name by deriving the function name from the earlier response.output_item.added event. This keeps strict ResponseStreamEvent validation useful without hand-editing generated Castiron event models. Adds sync and async regression coverage for client.responses.create(..., stream=True). Verification: - uv run pytest tests/lib/responses/test_responses.py tests/test_streaming.py tests/test_httpx2.py -o addopts= - uv run nox -s test-pydantic-v1 -- tests/lib/responses/test_responses.py -k streamed_function_call_arguments_done_uses_output_item_name -o addopts= - uv run ruff check . - uv run ruff format --check src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.py - uv run pyright src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.py - uv run mypy src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py - uv run python -c 'import openai'
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d02cdca4
ℹ️ 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".
| name = self._function_call_names_by_item_id.get(item_id) | ||
| if name is None: | ||
| return data |
There was a problem hiding this comment.
Handle resumed streams without a cached function name
When a caller uses responses.retrieve(response_id, stream=True, starting_after=N) and N is at or after the corresponding response.output_item.added event, the resumed stream does not replay the event that populates this map. If the next response.function_call_arguments.done payload omits name, this branch returns it unchanged, so strict validation still raises because ResponseFunctionCallArgumentsDoneEvent.name is required. The normalization needs a fallback that does not depend exclusively on observing an earlier event in the same connection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I checked this path against the current stream helpers. Direct responses.retrieve(..., stream=True, starting_after=N) can omit the earlier event that carries the function name, so the SDK cannot reconstruct a truthful name from that partial stream alone. I left that out of this patch rather than synthesizing an unknown value; a broader fix would need API support, a schema/model decision, or an explicit prefetch/state design for resumed raw streams.
|
|
||
|
|
||
| def _make_stream_event_normalizer(cast_to: object) -> Callable[[object], object] | None: | ||
| from .lib.streaming.responses._stream_event_normalizer import maybe_response_stream_event_normalizer |
There was a problem hiding this comment.
Avoid importing Responses helpers for unrelated streams
On the first iteration of every Stream or AsyncStream, including Chat Completions and other non-Responses streams, this import initializes the openai.lib.streaming.responses package before the function can return None. That package's __init__.py eagerly imports _events and _responses, while this normalizer also imports the large regular and beta response-event unions, adding substantial one-time module loading, memory, and time-to-first-token overhead to unrelated streaming calls. Gate the Responses-specific import without initializing this package for other stream chunk types.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in aaa72174: _streaming.py now checks the stream-event alias before importing the Responses normalizer, and tests/test_streaming.py covers a ChatCompletionChunk stream that does not import openai.lib.streaming.responses._stream_event_normalizer.
Gate the Responses stream-event normalizer before importing Responses streaming helpers so Chat Completions and other stream chunk types avoid the extra package load. Add a streaming regression test that proves a ChatCompletionChunk stream does not import openai.lib.streaming.responses._stream_event_normalizer. Verification: - uv run pytest tests/test_streaming.py -k unrelated_stream_does_not_import_responses_normalizer -o addopts= - uv run pytest tests/lib/responses/test_responses.py -k streamed_function_call_arguments_done_uses_output_item_name -o addopts= - uv run pytest tests/lib/responses/test_responses.py tests/test_streaming.py tests/test_httpx2.py -o addopts= - uv run nox -s test-pydantic-v1 -- tests/lib/responses/test_responses.py -k streamed_function_call_arguments_done_uses_output_item_name -o addopts= - uv run ruff check . - uv run ruff format --check src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.py tests/test_streaming.py - uv run pyright src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.py tests/test_streaming.py - uv run mypy src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py - uv run python -c 'import openai'
Changes being requested
Fixes #3472.
This adds a small Responses stream-event normalizer in hand-written SDK code. For Responses stream events, it records the function name from
response.output_item.addedbyitem_id, then uses that mapping when a laterresponse.function_call_arguments.doneevent is missingname.That keeps the raw
client.responses.create(..., stream=True)path valid under strictResponseStreamEventvalidation, while avoiding hand edits to generated Castiron event models.Additional context & links
This follows the state-mapping approach discussed on #3472 and avoids the generated-model edit shape from #3523.
Disclosure: This PR was prepared with Codex 5.5 under my review. I reviewed the changes and take responsibility for the code.
Validation
uv run pytest tests/lib/responses/test_responses.py tests/test_streaming.py tests/test_httpx2.py -o addopts=uv run nox -s test-pydantic-v1 -- tests/lib/responses/test_responses.py -k streamed_function_call_arguments_done_uses_output_item_name -o addopts=uv run ruff check .uv run ruff format --check src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.pyuv run pyright src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.py tests/lib/responses/test_responses.pyuv run mypy src/openai/_streaming.py src/openai/lib/streaming/responses/_stream_event_normalizer.pyuv run python -c 'import openai'