Let models opt into original image detail#14175
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/core/src/codex.rs
Line 790 in 080c890
get_user_instructions now depends on model_info, but when a turn switches models this path reuses self.user_instructions verbatim. That leaves JS REPL instructions stale for the previous model, so prompts may still advertise (or omit) detail: "original" guidance that no longer matches current model capabilities.
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0f925657a
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cda854ed0e
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83f88cfb1a
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cabee9e5c
ℹ️ 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 review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83ba4a0d8c
ℹ️ 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 review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a4bf2bfeb
ℹ️ 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".
| struct ViewImageArgs { | ||
| path: String, | ||
| #[serde(flatten)] | ||
| extra: serde_json::Map<String, Value>, |
There was a problem hiding this comment.
we can define the field in the model unconditionally just not tell model about it or use it's value when the feature is off
There was a problem hiding this comment.
yup, it detail: original is just dropped if it's not available.
| } | ||
|
|
||
| #[tokio::test(flavor = "multi_thread", worker_threads = 2)] | ||
| async fn view_image_tool_does_not_force_original_resolution_with_capability_feature_only() |
There was a problem hiding this comment.
is this a useful test? Isn't this the default behavior that's already covered?
There was a problem hiding this comment.
The difference between this one and view_image_tool_attaches_local_image is that this specifically enables the image_detail_original feature. Once the image_detail_original is always on, then yes it will be redundant. Would you prefer that we keep just view_image_tool_attaches_local_image and turn on image_detail_original in that test?
pakrym-oai
left a comment
There was a problem hiding this comment.
Mostly nits except the change to instruction generation in the main loop.
3a4bf2b to
311cdaa
Compare
311cdaa to
27ddd16
Compare
27ddd16 to
46d9b42
Compare
46d9b42 to
34149b2
Compare
Summary
This PR narrows original image detail handling to a single opt-in feature:
image_detail_originallets the model requestdetail: "original"on supported modelsdetailpreserves the default resized behaviorThe model only sees
detail: "original"guidance when the active model supports it:view_imageonly exposes adetailparameter when the feature and model can use itThe image detail API is intentionally narrow and consistent across both paths:
view_image.detailsupports only"original"; otherwise omit the fieldcodex.emitImage(..., detail)supports only"original"; otherwise omit the fielddetail: "original"requests fall back to normal behavior when the feature is disabled or the model does not support original detail