fix(types): make ImageGenerationCall result and status optional - #3558
fix(types): make ImageGenerationCall result and status optional#3558okxint wants to merge 2 commits into
Conversation
Filters out content items where `text` is None before joining, preventing a TypeError crash when the API returns null for a text block (issue openai#3063).
result (base64 image string) and status are output-only fields populated by the API response. Marking them Required on an input TypedDict forces callers to provide values they don't have and wouldn't supply when referencing an existing image generation call by id and type. Fixes openai#2648
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91f9172b37
ℹ️ 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".
| result: Optional[str] | ||
| """The generated image encoded in base64.""" | ||
|
|
||
| status: Required[Literal["in_progress", "completed", "generating", "failed"]] | ||
| status: Literal["in_progress", "completed", "generating", "failed"] |
There was a problem hiding this comment.
Mirror optional image fields in ResponseInputParam
This only fixes the standalone ResponseInputItemParam; the primary responses.create(..., input=...) overload imports ResponseInputParam from response_input_param.py, which defines a separate ImageGenerationCall union member that still has result: Required[Optional[str]] and status: Required[...]. In the usual responses.create(input=[{"type": "image_generation_call", "id": ...}]) path, type checkers will still require those output-only fields, so the typing bug remains for the main Responses API unless the mirrored type in response_input_param.py is updated too.
Useful? React with 👍 / 👎.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head 91f9172b377cf1fe48ef70a11acf6271647001af. Two blockers:
-
[P2] Apply the type fix to the primary Responses input alias. This changes
ImageGenerationCallonly inresponse_input_item_param.py, butclient.responses.create(),.parse(), and.stream()use the duplicate definition inresponse_input_param.py, whereresultandstatusare stillRequired. A Pyright repro of the documented id-only input still fails forresponses.create()while the changedinput_tokens.count()path accepts it. Please update the source spec/codegen so both public aliases stay consistent and add a type-checked regression for the mainResponseInputParampath. -
[P2] Drop or complete the unrelated null-output-text commit. Commit
9018ec5eis outside this PR's stated scope and only guardsResponse.output_textafter a model exists. Its test usesmodel_construct()to bypass validation, whileResponseOutputText.textremains non-nullable and structured parsing still callsparse_text()with the null value. Strict validation andresponses.parse(..., text_format=...)therefore remain broken for the issue this commit claims to address. Please rebase it out into a focused PR, or make the schema/parser fix complete with API-boundary and structured-parse tests.
Validation notes: git diff --check passes, but ruff check currently reports an unsorted import block in tests/lib/responses/test_responses.py, ruff format --check would reformat that file, hosted checks are absent, and the PR is currently merge-conflicted.
result(base64 image string) andstatusare output-only fields populated by the API. Marking themRequiredon an inputTypedDictforces callers to supply values they don't have and wouldn't provide when referencing an existing image generation call byidandtype.Fixes #2648