Guard response parsing against null output#3381
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates response parsing to tolerate null/None response outputs and adds a regression test to ensure parsing returns an empty list in that case.
Changes:
- Update
parse_responseto iterate safely whenresponse.outputisNone. - Add a unit test asserting
parse_responseconvertsoutput: nullintoparsed.output == [].
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/lib/responses/test_responses.py | Adds coverage for output=None parsing behavior. |
| src/openai/lib/_parsing/_responses.py | Makes parsing resilient to None response outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| output_list: List[ParsedResponseOutputItem[TextFormatT]] = [] | ||
|
|
||
| for output in response.output: | ||
| for output in response.output or []: |
| def test_parse_response_allows_null_output() -> None: | ||
| response = construct_type_unchecked( | ||
| type_=Response, | ||
| value={ | ||
| "id": "resp_689a0b2545288193953c892439b42e2800b2e36c65a1fd4b", | ||
| "object": "response", | ||
| "created_at": 1754925861, | ||
| "status": "completed", | ||
| "background": False, | ||
| "error": None, | ||
| "incomplete_details": None, | ||
| "instructions": None, | ||
| "max_output_tokens": None, | ||
| "max_tool_calls": None, | ||
| "model": "gpt-4o-mini-2024-07-18", | ||
| "output": None, | ||
| "parallel_tool_calls": True, | ||
| "previous_response_id": None, | ||
| "prompt_cache_key": None, | ||
| "reasoning": {"effort": None, "summary": None}, | ||
| "safety_identifier": None, | ||
| "service_tier": "default", | ||
| "store": True, | ||
| "temperature": 1.0, | ||
| "text": {"format": {"type": "text"}, "verbosity": "medium"}, | ||
| "tool_choice": "auto", | ||
| "tools": [], | ||
| "top_logprobs": 0, | ||
| "top_p": 1.0, | ||
| "truncation": "disabled", | ||
| "usage": { | ||
| "input_tokens": 14, | ||
| "input_tokens_details": {"cached_tokens": 0}, | ||
| "output_tokens": 50, | ||
| "output_tokens_details": {"reasoning_tokens": 0}, | ||
| "total_tokens": 64, | ||
| }, | ||
| "user": None, | ||
| "metadata": {}, | ||
| }, | ||
| ) |
Muhtasim-Munif-Fahim
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf38cd09d5
ℹ️ 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".
| output_list: List[ParsedResponseOutputItem[TextFormatT]] = [] | ||
|
|
||
| for output in response.output: | ||
| for output in response.output or []: |
There was a problem hiding this comment.
When a streamed response has valid response.output_item.* events but its final response.completed payload contains output: null, this fallback silently returns an empty output list. ResponseStreamState has already accumulated those items in its snapshot, but it calls parse_response() with event.response at src/openai/lib/streaming/responses/_responses.py:359-364, so the accumulated output is discarded. This avoids the prior exception but causes get_final_response() and the emitted completion event to lose all streamed content; the streaming completion path should fall back to the snapshot's output rather than converting null to an empty result.
Useful? React with 👍 / 👎.
Summary
Guard
parse_response()againstresponse.output == nullinresponse.completedpayloads and add a regression test for the null-output case.Root Cause
src/openai/lib/_parsing/_responses.pyassumesresponse.outputis always iterable:When a backend sends
response.completedwithoutput: null, this raisesTypeErrorduring final response parsing even if earlier streamed output items were valid.Changes
response.outputasresponse.output or []inparse_response()Responseobject withoutput=NoneValidation
python -m pytest tests/lib/responses/test_responses.py -p no:xdist -o addopts=""ruff check src/openai/lib/_parsing/_responses.py tests/lib/responses/test_responses.pyImpact
This makes response parsing defensive against null
outputpayloads while preserving existing behavior for normal responses with populated output lists.