-
Notifications
You must be signed in to change notification settings - Fork 7.1k
tui: expand image placeholders to full paths for models (keep basenames for users) #8737
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
base: main
Are you sure you want to change the base?
Conversation
When users paste a local image file path (or paste an image from the clipboard), Codex inserts a placeholder in the composed prompt. Previously the placeholder only included the basename; this makes it hard for the model to refer back to the actual local file path. Include the full path instead. Tests: cargo test -p codex-tui; cargo test -p codex-tui2
|
All contributors have signed the CLA ✍️ ✅ |
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.
Pull request overview
This PR updates the TUI attachment placeholder format to use full file paths instead of basenames, enabling models to reference the actual local file paths in responses.
Key Changes:
- Modified
attach_image()in both TUI implementations to usepath.display()instead of extracting the filename - Updated all test assertions to expect full paths in placeholders (e.g.,
[/tmp/image.png 24x42]instead of[image.png 24x42])
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| codex-rs/tui2/src/bottom_pane/chat_composer.rs | Changed placeholder generation to use full path via path.display() instead of file_name() |
| codex-rs/tui2/src/chatwidget/tests.rs | Updated test assertion to expect full path in placeholder |
| codex-rs/tui/src/bottom_pane/chat_composer.rs | Changed placeholder generation to use full path via path.display() instead of file_name() |
| codex-rs/tui/src/chatwidget/tests.rs | Updated test assertion to expect full path in placeholder |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I have read the CLA Document and I hereby sign the CLA |
What
placeholder into the composed prompt. This placeholder now includes the full path (e.g.
/tmp/image.pnginstead of only the basename (e.g.image.png).image.pngwhile the model sees/tmp/image.png.Why
the model to use the path in commands / docs / patches rather than “just see the image”).
Testing done