Handle standalone image generation failures as terminal items#27920
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ca854c673
ℹ️ 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".
| call.turn_item_emitter | ||
| .emit_completed(ExtensionTurnItem::ImageGeneration(ImageGenerationItem { | ||
| id: call.call_id.clone(), | ||
| status: "failed".to_string(), |
There was a problem hiding this comment.
Don’t report failed image generations as generated
When this branch emits a terminal item with status: "failed", the /agent preview still treats every ThreadItem::ImageGeneration as Generated an image (codex-rs/tui/src/app/agent_status_feed.rs:187). In sub-agent activity, a backend failure will therefore still be summarized as a successful generation, which is the client misrendering this change is trying to avoid; please key that summary off the status too.
Useful? React with 👍 / 👎.
fcoury-oai
left a comment
There was a problem hiding this comment.
From a TUI perspective, I think there is an opportunity to align this with the existing failure presentation while preserving the actual failure reason.
The current rendering introduces a bespoke ✗ Image generation failed cell, while MCP, exec, and hook failures generally use a red status marker followed by an indented error detail.
The failed terminal item does not include message and sets result to an empty string. Although the message is returned to the model, clients only receive the prompt and generic failure status not the backend error, which may be important for diagnosing the issue.
Could we propagate the error message on the terminal image-generation item and render it using the existing tool-error convention? For example:
• Image generation failed
└ Error: <backend error>
We should also apply the existing wrapping and truncation behavior so an unexpectedly large backend response cannot dominate the transcript.
The revised prompt can remain as secondary context if useful, but it should not replace the failure reason.
Why
Standalone image generation emitted a started item but no terminal item when the backend failed. Clients could leave the operation unresolved or render it as successful.
What changed
status: "failed"when generation or editing fails.Looks for TUI, App-Side change needed
Validation
just test -p codex-image-generation-extensionFollow-Up
UX Follow-Up incoming for the extension tool