fix: use Annotation type for ResponseOutputTextAnnotationAddedEvent.annotation#3443
fix: use Annotation type for ResponseOutputTextAnnotationAddedEvent.annotation#3443C1-BA-B1-F3 wants to merge 4 commits into
Conversation
When a backend sends 'response.output: null' in a response.completed event (e.g., chatgpt.com Codex backend), parse_response would crash with TypeError: 'NoneType' object is not iterable. This one-line fix coerces None to an empty list, allowing the stream to complete gracefully. Consumers that track output_item.done events can still backfill from their collected items. Fixes openai#3325
After receiving the [DONE] SSE event, Stream.__stream__ and AsyncStream.__stream__ now drain remaining events from the iterator before calling response.close(). This ensures the HTTP/1.1 chunked transfer-encoding terminator (0\r\n\r\n) is consumed, allowing h11's their_state to advance to DONE so httpcore returns the connection to the pool gracefully instead of destroying it with TCP FIN. Regression from 6132922 which removed the drain. Fixes openai#3440
…nnotation The annotation field was typed as 'object' instead of the proper 'Annotation' union type (AnnotationFileCitation | AnnotationURLCitation | AnnotationContainerFileCitation | AnnotationFilePath). This forced consumers to use typing.cast to access typed fields. Fixes openai#3419 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e0b2a1df1
ℹ️ 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".
| for _ in iterator: | ||
| pass |
There was a problem hiding this comment.
Always close the response if draining fails
When the added drain loop reads the remaining stream, response.iter_bytes() can still raise (for example a read timeout or protocol error while waiting for the chunked terminator). In that case execution never reaches response.close(), so this new cleanup path can leak the response/connection and can also replace the original streaming exception with the drain failure; the async branch added below has the same ordering. Wrap the drain in its own try/finally so close/aclose always runs.
Useful? React with 👍 / 👎.
Wrap the drain loop in a nested try/finally in both Stream.__stream__ and AsyncStream.__stream__ so that response.close() / aclose() is always called, even if iter_bytes() raises during drain (e.g. read timeout or protocol error). Without this, a drain failure could leak the response/connection and replace the original streaming exception. Addresses Codex review P2 on openai#3443.
Description
Fixes #3419
The
annotationfield onResponseOutputTextAnnotationAddedEventwas typed asobjectinstead of the properAnnotationunion type. This forced consumers to usetyping.castto access typed fields.Changes
annotation: objecttoannotation: AnnotationinResponseOutputTextAnnotationAddedEventAnnotationfromresponse_output_text.pyType Definition
The
Annotationtype is already defined inresponse_output_text.py:This is consistent with how
ResponseOutputText.annotations: List[Annotation]is already typed.Testing
Added comprehensive tests covering all annotation variants:
test_annotation_url_citation- URL citation annotationstest_annotation_file_citation- File citation annotationstest_annotation_container_file_citation- Container file citation annotationstest_annotation_file_path- File path annotationstest_annotation_type_is_union- Verifies field is not typed asobjecttest_construct_type_with_annotation- Tests streaming event parsingBefore/After
Before:
After:
Ultraworked with Sisyphus