remove flag for image preparation#29429
Conversation
rka-oai
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: ca0cdd724f
ℹ️ 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".
| .into_data_url() | ||
| }; | ||
| // The history insertion path owns image decoding and resizing. | ||
| let image_url = data_url_from_bytes("application/octet-stream", &file_bytes); |
There was a problem hiding this comment.
When a PostToolUse hook matches view_image or rollout tracing is enabled, those payloads are built from ViewImageOutput::to_response_item() before record_conversation_items() calls prepare_response_items, so this deferred URL is sent to those external surfaces as full unvalidated data:application/octet-stream bytes. That means a malformed/non-image file now appears as a successful tool output to hooks/traces, and large images bypass the bounded prepared/placeholder value that the model later sees. Please prepare before constructing non-history payloads or override those surfaces to emit a bounded placeholder/processed value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
not a serious issue
| ResponseItem::from(ResponseInputItem::from_user_input( | ||
| input, | ||
| local_image_preparation, | ||
| LocalImagePreparation::Defer, |
There was a problem hiding this comment.
When a user accidentally attaches a large non-image or oversized local file, this now always takes the Defer path, whose from_user_input branch wraps the entire file in a base64 data URL before prepare_response_items can validate or reject it. The previous default Process path could fail during image loading without creating that extra 4/3-sized string and then decoding it again, so a bad local attachment can now cause large transient memory spikes before it becomes a placeholder. Please reject/cap the byte buffer before converting it to a data URL, or prepare directly from bytes.
Useful? React with 👍 / 👎.
| { | ||
| prepare_response_items(items.to_mut()); | ||
| } | ||
| prepare_response_items(items.to_mut()); |
There was a problem hiding this comment.
When code mode emits a data-URL image with detail: "low" (which the code-mode parser still accepts and maps to ImageDetail::Low), this unconditional preparation path now runs before the tool output reaches the next model request. prepare_image treats ImageDetail::Low as unsupported and replaces the image with a text placeholder, so previously valid low-detail code-mode image outputs silently stop being images. Please either stop accepting low in code mode or normalize it to a supported detail before calling prepare_response_items.
Useful? React with 👍 / 👎.
2 participants
What
view_imageoutputs always defer decoding and resizing to that pathresize_all_imagesas an ignored, removed compatibility key for released clientsWhy
Centralized preparation is now the intended image path. Keeping the runtime feature checks also kept two image-processing implementations alive and allowed client config to select the legacy behavior.
This is a clean replacement for #28975, rebuilt from the latest
main.How
prepare_response_itemsnow runs whenever items enter history and whenever persisted history is reconstructed. Producers emit deferred image data, so malformed images become the existing model-visible placeholder instead of failing the session at the producer.Test plan
just fmtjust fix -p codex-core -p codex-featuresjust test -p codex-features— 52 passedcodex-coreset — 20 passedjust test -p codex-core handle_accepts_explicit_high_detail— 1 passedjust test -p codex-coreattempt — 2,723 passed; 88 unrelated environment failures from read-only~/.codexSQLite state and unavailable integration helper binaries