Add saved image path hint to standalone image generation#25947
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d1ed8cefc
ℹ️ 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".
dbbe6f8 to
5b371f3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffdbcb79cc
ℹ️ 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".
| codex-model-provider-info = { workspace = true } | ||
| codex-protocol = { workspace = true } | ||
| codex-tools = { workspace = true } | ||
| codex-utils-absolute-path = { workspace = true } |
There was a problem hiding this comment.
Refresh Bazel lockfile for the new dependency
Adding this workspace dependency changes the Rust dependency graph, but the commit does not include the corresponding MODULE.bazel.lock update required by the root AGENTS.md dependency-change rule. I checked this commit locally with just bazel-lock-check; it exits with MODULE.bazel.lock is out of date. Run 'just bazel-lock-update' and commit the updated lockfile., so Bazel lock validation will fail until the lockfile update is committed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
already ran this and no change
| let output_hint = | ||
| extension_image_generation_output_hint(output_dir.display(), output_path.display()); |
There was a problem hiding this comment.
Only advertise the saved path after persistence succeeds
When the image bytes are valid but core fails to write the artifact (for example, codex_home is unwritable or the disk is full), persist_image_generation_item leaves saved_path as None, but this code still appends a model-visible hint saying the image is saved at the computed path. That makes follow-up model work reference a file that does not exist; the hint should be emitted only after the core finalization path confirms the image was actually persisted, matching the hosted image-generation behavior.
Useful? React with 👍 / 👎.
Why
Standalone image generation returns image bytes to the model, but the model also needs the host artifact path to reference the generated file in follow-up work.
What changed
Validation
just fmtjust bazel-lock-checkjust test -p codex-app-server standalone_image_generation_returns_saved_path_hint_to_model