fix(responses): handle null output in parse_response - #3586
Conversation
Some backends (e.g. the chatgpt.com Codex backend used by the Codex CLI) can send `output: null` on the `response.completed` event even though the schema declares `output` as a non-nullable list. This caused parse_response() to raise `TypeError: 'NoneType' object is not iterable`, killing the entire stream before consumers could read already-accumulated deltas. Coerce a null output to an empty list before iterating, matching the behavior implied by get_final_response() returning an otherwise valid Response/ParsedResponse. Fixes openai#3325
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b34be89cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| output_list: List[ParsedResponseOutputItem[TextFormatT]] = [] | ||
|
|
||
| for output in response.output: | ||
| for output in response.output or []: |
There was a problem hiding this comment.
Preserve accumulated stream output on null completion
When using responses.stream() against a backend that sends response.completed.response.output = null after prior response.output_item.added / text-delta events, this fallback makes parse_response() treat the completed response as having no output. ResponseStreamState has already accumulated those items in its snapshot, so get_final_response().output and output_text become empty even though the stream delivered content; the null-completion path should parse the accumulated snapshot rather than unconditionally replacing it with [].
Useful? React with 👍 / 👎.
response.completed events can carry a null/empty output even when prior response.output_item.added / delta events already populated the snapshot; prefer the accumulated snapshot in that case instead of discarding it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5171fb4c96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Some backends send `response.completed` with a null/empty `output` | ||
| # even though prior `response.output_item.added` / delta events already | ||
| # populated it on the snapshot; prefer the accumulated snapshot in that case. | ||
| response = response.model_copy(update={"output": snapshot.output}) |
There was a problem hiding this comment.
Avoid the Pydantic-v2-only copy method
When this null-output path is reached under supported Pydantic 1.x (pyproject.toml allows pydantic>=1.9.0, <3), response models provide .copy() but not Pydantic 2's .model_copy(). A stream with accumulated items therefore raises AttributeError here, so the original failure still prevents both the completed event and get_final_response() from succeeding; use a compatibility copy path that supports the update argument on both major versions.
Useful? React with 👍 / 👎.
| # Some backends send `response.completed` with a null/empty `output` | ||
| # even though prior `response.output_item.added` / delta events already | ||
| # populated it on the snapshot; prefer the accumulated snapshot in that case. | ||
| response = response.model_copy(update={"output": snapshot.output}) |
There was a problem hiding this comment.
Apply done-event payloads before reusing the snapshot
When the stream includes the normal response.output_item.done event before a null completion, accumulate_event() never applies that event's final item to the snapshot; it likewise ignores final content-part and annotation payloads. Copying this snapshot therefore returns items that can still have status="in_progress" and can omit final annotations or logprobs even though the corresponding done events supplied them. Update the accumulated entries from the done-event payloads before using them as the completed response output.
Useful? React with 👍 / 👎.
Summary
Some backends (e.g. the chatgpt.com Codex backend used by the Codex CLI) can send
output: nullon theresponse.completedevent, even though the schema declaresoutputas a non-nullable list. This crashesparse_response()insrc/openai/lib/_parsing/_responses.pywith:This kills the entire stream before the consumer can read any already-accumulated deltas, even when valid
response.output_item.doneevents were seen earlier in the same response.Fix
Coerce a null
response.outputto an empty list before iterating inparse_response(), so streams complete andget_final_response()returns aResponse/ParsedResponsewithoutput=[]instead of crashing. Consumers that already trackoutput_item.doneevents can backfill from their own collected items.Test plan
test_parse_response_with_null_outputintests/lib/responses/test_responses.py, which constructs aResponsewithoutput=Noneand assertsparse_response()returnsoutput=[]instead of raising.TypeError: 'NoneType' object is not iterableon the pre-fix code, and passes after the fix.tests/lib/responses/locally: all tests pass.ruff checkon the changed files: no issues.Fixes #3325