-
Notifications
You must be signed in to change notification settings - Fork 6.1k
fix image drag drop #5794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix image drag drop #5794
Conversation
|
@codex pls review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| pub fn handle_paste(&mut self, pasted: String) -> bool { | ||
| tracing::info!("handle_paste: {pasted}"); | ||
| let char_count = pasted.chars().count(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid logging full paste contents at info level
The new tracing::info!("handle_paste: {pasted}") call writes every pasted string—including arbitrary user messages, file contents, and potentially secrets—to the standard info log. Unlike the existing debug tooling, this is always executed and prints the entire payload, so a normal production build will emit user data to logs. Besides privacy concerns, large pastes will also generate very large log entries. Consider removing this log or gating/truncating it behind a debug flag.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
|
@genki i wonder if you'd mind taking a quick look at this one when you have a chance. No problem if you can't. I suspect IME will continue to work fine on this and I'll test to the best of my ability, but you likely know the edge cases better than I do on IME, and would notice problems straight away. |
fixing drag/drop photos bug in codex
state of the world before:
sometimes, when you drag screenshots into codex, the image does not properly render into context. instead, the file name is shown in quotation marks.
before.2.mov
the screenshot is not actually included in agent context. the agent needs to manually call the view_image tool to see the screenshot. this can be unreliable especially if the image is part of a longer prompt and is dependent on the agent going out of its way to view the image.
state of the world after:
after.mov
now, images will always be directly embedded into chat context
Technical Details
handle_pastecalls (quoted prefix +PM.png'). The split string could not be normalized to a real path, so we showed the quoted filename instead of embedding the image.