agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Label attached images so agent can understand in-message labels (#8950)

Browse Source 
Agent wouldn't "see" attached images and would instead try to use the
view_file tool:
<img width="1516" height="504" alt="image"
src="https://github.com/user-attachments/assets/68a705bb-f962-4fc1-9087-e932a6859b12"
/>

In this PR, we wrap image content items in XML tags with the name of
each image (now just a numbered name like `[Image #1]`), so that the
model can understand inline image references (based on name). We also
put the image content items above the user message which the model seems
to prefer (maybe it's more used to definitions being before references).

We also tweak the view_file tool description which seemed to help a bit

Results on a simple eval set of images:

Before
<img width="980" height="310" alt="image"
src="https://github.com/user-attachments/assets/ba838651-2565-4684-a12e-81a36641bf86"
/>

After
<img width="918" height="322" alt="image"
src="https://github.com/user-attachments/assets/10a81951-7ee6-415e-a27e-e7a3fd0aee6f"
/>

```json
[
  {
    "id": "single_describe",
    "prompt": "Describe the attached image in one sentence.",
    "images": ["image_a.png"]
  },
  {
    "id": "single_color",
    "prompt": "What is the dominant color in the image? Answer with a single color word.",
    "images": ["image_b.png"]
  },
  {
    "id": "orientation_check",
    "prompt": "Is the image portrait or landscape? Answer in one sentence.",
    "images": ["image_c.png"]
  },
  {
    "id": "detail_request",
    "prompt": "Look closely at the image and call out any small details you notice.",
    "images": ["image_d.png"]
  },
  {
    "id": "two_images_compare",
    "prompt": "I attached two images. Are they the same or different? Briefly explain.",
    "images": ["image_a.png", "image_b.png"]
  },
  {
    "id": "two_images_captions",
    "prompt": "Provide a short caption for each image (Image 1, Image 2).",
    "images": ["image_c.png", "image_d.png"]
  },
  {
    "id": "multi_image_rank",
    "prompt": "Rank the attached images from most colorful to least colorful.",
    "images": ["image_a.png", "image_b.png", "image_c.png"]
  },
  {
    "id": "multi_image_choice",
    "prompt": "Which image looks more vibrant? Answer with 'Image 1' or 'Image 2'.",
    "images": ["image_b.png", "image_d.png"]
  }
]
```
This commit is contained in:
charley-oai
2026-01-09 21:33:45 -08:00
committed by GitHub
Unverified
parent cf515142b0
commit 6709ad8975
16 changed files with 800 additions and 608 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+18
View File
@@ -1540,6 +1540,24 @@ impl Session {
}
}
/// Returns the input if there was no task running to inject into
pub async fn inject_response_items(
&self,
input: Vec<ResponseInputItem>,
) -> Result<(), Vec<ResponseInputItem>> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
for item in input {
ts.push_pending_input(item);
}
Ok(())
}
None => Err(input),
}
}
pub async fn get_pending_input(&self) -> Vec<ResponseInputItem> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
codex-rs/core/src/event_mapping.rs
+87 -1
View File
@@ -9,6 +9,10 @@ use codex_protocol::models::ReasoningItemContent;
use codex_protocol::models::ReasoningItemReasoningSummary;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::WebSearchAction;
use codex_protocol::models::is_image_close_tag_text;
use codex_protocol::models::is_image_open_tag_text;
use codex_protocol::models::is_local_image_close_tag_text;
use codex_protocol::models::is_local_image_open_tag_text;
use codex_protocol::user_input::UserInput;
use tracing::warn;
use uuid::Uuid;
@@ -32,9 +36,17 @@ fn parse_user_message(message: &[ContentItem]) -> Option<UserMessageItem> {
let mut content: Vec<UserInput> = Vec::new();
for content_item in message.iter() {
for (idx, content_item) in message.iter().enumerate() {
match content_item {
ContentItem::InputText { text } => {
if (is_local_image_open_tag_text(text) || is_image_open_tag_text(text))
&& (matches!(message.get(idx + 1), Some(ContentItem::InputImage { .. })))
|| (idx > 0
&& (is_local_image_close_tag_text(text) || is_image_close_tag_text(text))
&& matches!(message.get(idx - 1), Some(ContentItem::InputImage { .. })))
{
continue;
}
if is_session_prefix(text) || is_user_shell_command_text(text) {
return None;
}
@@ -177,6 +189,80 @@ mod tests {
}
}
#[test]
fn skips_local_image_label_text() {
let image_url = "data:image/png;base64,abc".to_string();
let label = codex_protocol::models::local_image_open_tag_text(1);
let user_text = "Please review this image.".to_string();
let item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![
ContentItem::InputText { text: label },
ContentItem::InputImage {
image_url: image_url.clone(),
},
ContentItem::InputText {
text: "</image>".to_string(),
},
ContentItem::InputText {
text: user_text.clone(),
},
],
};
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
match turn_item {
TurnItem::UserMessage(user) => {
let expected_content = vec![
UserInput::Image { image_url },
UserInput::Text { text: user_text },
];
assert_eq!(user.content, expected_content);
}
other => panic!("expected TurnItem::UserMessage, got {other:?}"),
}
}
#[test]
fn skips_unnamed_image_label_text() {
let image_url = "data:image/png;base64,abc".to_string();
let label = codex_protocol::models::image_open_tag_text();
let user_text = "Please review this image.".to_string();
let item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![
ContentItem::InputText { text: label },
ContentItem::InputImage {
image_url: image_url.clone(),
},
ContentItem::InputText {
text: codex_protocol::models::image_close_tag_text(),
},
ContentItem::InputText {
text: user_text.clone(),
},
],
};
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
match turn_item {
TurnItem::UserMessage(user) => {
let expected_content = vec![
UserInput::Image { image_url },
UserInput::Text { text: user_text },
];
assert_eq!(user.content, expected_content);
}
other => panic!("expected TurnItem::UserMessage, got {other:?}"),
}
}
#[test]
fn skips_user_instructions_and_env() {
let items = vec![
codex-rs/core/src/tools/handlers/view_image.rs
+11 -2
View File
@@ -11,7 +11,9 @@ use crate::tools::context::ToolPayload;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use codex_protocol::user_input::UserInput;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::local_image_content_items_with_label_number;
pub struct ViewImageHandler;
@@ -63,8 +65,15 @@ impl ToolHandler for ViewImageHandler {
}
let event_path = abs_path.clone();
let content: Vec<ContentItem> =
local_image_content_items_with_label_number(&abs_path, None);
let input = ResponseInputItem::Message {
role: "user".to_string(),
content,
};
session
.inject_input(vec![UserInput::LocalImage { path: abs_path }])
.inject_response_items(vec![input])
.await
.map_err(|_| {
FunctionCallError::RespondToModel(
codex-rs/core/src/tools/spec.rs
+4 -4
View File
@@ -8,6 +8,7 @@ use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS;
use crate::tools::handlers::collab::MAX_WAIT_TIMEOUT_MS;
use crate::tools::registry::ToolRegistryBuilder;
use codex_protocol::models::VIEW_IMAGE_TOOL_NAME;
use codex_protocol::openai_models::ApplyPatchToolType;
use codex_protocol::openai_models::ConfigShellToolType;
use codex_protocol::openai_models::ModelInfo;
@@ -412,10 +413,9 @@ fn create_view_image_tool() -> ToolSpec {
)]);
ToolSpec::Function(ResponsesApiTool {
name: "view_image".to_string(),
description:
"Attach a local image (by filesystem path) to the thread context for this turn."
.to_string(),
name: VIEW_IMAGE_TOOL_NAME.to_string(),
description: "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within <image ...> tags)."
.to_string(),
strict: false,
parameters: JsonSchema::Object {
properties,
codex-rs/core/tests/suite/view_image.rs
+14
View File
@@ -216,6 +216,20 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> {
let image_message =
find_image_message(&body).expect("pending input image message not included in request");
let content_items = image_message
.get("content")
.and_then(Value::as_array)
.expect("image message has content array");
assert_eq!(
content_items.len(),
1,
"view_image should inject only the image content item (no tag/label text)"
);
assert_eq!(
content_items[0].get("type").and_then(Value::as_str),
Some("input_image"),
"view_image should inject only an input_image content item"
);
let image_url = image_message
.get("content")
.and_then(Value::as_array)
codex-rs/protocol/src/models.rs
+133 -31
View File
@@ -180,6 +180,48 @@ fn local_image_error_placeholder(
}
}
pub const VIEW_IMAGE_TOOL_NAME: &str = "view_image";
const IMAGE_OPEN_TAG: &str = "<image>";
const IMAGE_CLOSE_TAG: &str = "</image>";
const LOCAL_IMAGE_OPEN_TAG_PREFIX: &str = "<image name=";
const LOCAL_IMAGE_OPEN_TAG_SUFFIX: &str = ">";
const LOCAL_IMAGE_CLOSE_TAG: &str = IMAGE_CLOSE_TAG;
pub fn image_open_tag_text() -> String {
IMAGE_OPEN_TAG.to_string()
}
pub fn image_close_tag_text() -> String {
IMAGE_CLOSE_TAG.to_string()
}
pub fn local_image_label_text(label_number: usize) -> String {
format!("[Image #{label_number}]")
}
pub fn local_image_open_tag_text(label_number: usize) -> String {
let label = local_image_label_text(label_number);
format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label}{LOCAL_IMAGE_OPEN_TAG_SUFFIX}")
}
pub fn is_local_image_open_tag_text(text: &str) -> bool {
text.strip_prefix(LOCAL_IMAGE_OPEN_TAG_PREFIX)
.is_some_and(|rest| rest.ends_with(LOCAL_IMAGE_OPEN_TAG_SUFFIX))
}
pub fn is_local_image_close_tag_text(text: &str) -> bool {
is_image_close_tag_text(text)
}
pub fn is_image_open_tag_text(text: &str) -> bool {
text == IMAGE_OPEN_TAG
}
pub fn is_image_close_tag_text(text: &str) -> bool {
text == IMAGE_CLOSE_TAG
}
fn invalid_image_error_placeholder(
path: &std::path::Path,
error: impl std::fmt::Display,
@@ -203,6 +245,53 @@ fn unsupported_image_error_placeholder(path: &std::path::Path, mime: &str) -> Co
}
}
pub fn local_image_content_items_with_label_number(
path: &std::path::Path,
label_number: Option<usize>,
) -> Vec<ContentItem> {
match load_and_resize_to_fit(path) {
Ok(image) => {
let mut items = Vec::with_capacity(3);
if let Some(label_number) = label_number {
items.push(ContentItem::InputText {
text: local_image_open_tag_text(label_number),
});
}
items.push(ContentItem::InputImage {
image_url: image.into_data_url(),
});
if label_number.is_some() {
items.push(ContentItem::InputText {
text: LOCAL_IMAGE_CLOSE_TAG.to_string(),
});
}
items
}
Err(err) => {
if matches!(&err, ImageProcessingError::Read { .. }) {
vec![local_image_error_placeholder(path, &err)]
} else if err.is_invalid_image() {
vec![invalid_image_error_placeholder(path, &err)]
} else {
let Some(mime_guess) = mime_guess::from_path(path).first() else {
return vec![local_image_error_placeholder(
path,
"unsupported MIME type (unknown)",
)];
};
let mime = mime_guess.essence_str().to_owned();
if !mime.starts_with("image/") {
return vec![local_image_error_placeholder(
path,
format!("unsupported MIME type `{mime}`"),
)];
}
vec![unsupported_image_error_placeholder(path, &mime)]
}
}
}
}
impl From<ResponseInputItem> for ResponseItem {
fn from(item: ResponseInputItem) -> Self {
match item {
@@ -296,41 +385,27 @@ pub enum ReasoningItemContent {
impl From<Vec<UserInput>> for ResponseInputItem {
fn from(items: Vec<UserInput>) -> Self {
let mut image_index = 0;
Self::Message {
role: "user".to_string(),
content: items
.into_iter()
.filter_map(|c| match c {
UserInput::Text { text } => Some(ContentItem::InputText { text }),
UserInput::Image { image_url } => Some(ContentItem::InputImage { image_url }),
UserInput::LocalImage { path } => match load_and_resize_to_fit(&path) {
Ok(image) => Some(ContentItem::InputImage {
image_url: image.into_data_url(),
}),
Err(err) => {
if matches!(&err, ImageProcessingError::Read { .. }) {
Some(local_image_error_placeholder(&path, &err))
} else if err.is_invalid_image() {
Some(invalid_image_error_placeholder(&path, &err))
} else {
let Some(mime_guess) = mime_guess::from_path(&path).first() else {
return Some(local_image_error_placeholder(
&path,
"unsupported MIME type (unknown)",
));
};
let mime = mime_guess.essence_str().to_owned();
if !mime.starts_with("image/") {
return Some(local_image_error_placeholder(
&path,
format!("unsupported MIME type `{mime}`"),
));
}
Some(unsupported_image_error_placeholder(&path, &mime))
}
}
},
UserInput::Skill { .. } => None, // Skill bodies are injected later in core
.flat_map(|c| match c {
UserInput::Text { text } => vec![ContentItem::InputText { text }],
UserInput::Image { image_url } => vec![
ContentItem::InputText {
text: image_open_tag_text(),
},
ContentItem::InputImage { image_url },
ContentItem::InputText {
text: image_close_tag_text(),
},
],
UserInput::LocalImage { path } => {
image_index += 1;
local_image_content_items_with_label_number(&path, Some(image_index))
}
UserInput::Skill { .. } => Vec::new(), // Skill bodies are injected later in core
})
.collect::<Vec<ContentItem>>(),
}
@@ -770,6 +845,33 @@ mod tests {
Ok(())
}
#[test]
fn wraps_image_user_input_with_tags() -> Result<()> {
let image_url = "data:image/png;base64,abc".to_string();
let item = ResponseInputItem::from(vec![UserInput::Image {
image_url: image_url.clone(),
}]);
match item {
ResponseInputItem::Message { content, .. } => {
let expected = vec![
ContentItem::InputText {
text: image_open_tag_text(),
},
ContentItem::InputImage { image_url },
ContentItem::InputText {
text: image_close_tag_text(),
},
];
assert_eq!(content, expected);
}
other => panic!("expected message response but got {other:?}"),
}
Ok(())
}
#[test]
fn local_image_read_error_adds_placeholder() -> Result<()> {
let dir = tempdir()?;
codex-rs/tui/src/bottom_pane/chat_composer.rs
+176 -270
View File
@@ -46,6 +46,7 @@ use crate::style::user_message_style;
use codex_common::fuzzy_match::fuzzy_match;
use codex_protocol::custom_prompts::CustomPrompt;
use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX;
use codex_protocol::models::local_image_label_text;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
@@ -59,7 +60,7 @@ use codex_core::skills::model::SkillMetadata;
use codex_file_search::FileMatch;
use std::cell::RefCell;
use std::collections::HashMap;
use std::path::Path;
use std::collections::HashSet;
use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
@@ -274,10 +275,12 @@ impl ChatComposer {
// normalize_pasted_path already handles Windows → WSL path conversion,
// so we can directly try to read the image dimensions.
match image::image_dimensions(&path_buf) {
Ok((w, h)) => {
Ok((width, height)) => {
tracing::info!("OK: {pasted}");
let format_label = pasted_image_format(&path_buf).label();
self.attach_image(path_buf, w, h, format_label);
tracing::debug!("image dimensions={}x{}", width, height);
let format = pasted_image_format(&path_buf);
tracing::debug!("attached image format={}", format.label());
self.attach_image(path_buf);
true
}
Err(err) => {
@@ -408,13 +411,9 @@ impl ChatComposer {
}
/// Attempt to start a burst by retro-capturing recent chars before the cursor.
pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, _format_label: &str) {
let file_label = path
.file_name()
.map(|name| name.to_string_lossy().into_owned())
.unwrap_or_else(|| "image".to_string());
let base_placeholder = format!("{file_label} {width}x{height}");
let placeholder = self.next_image_placeholder(&base_placeholder);
pub fn attach_image(&mut self, path: PathBuf) {
let image_number = self.attached_images.len() + 1;
let placeholder = local_image_label_text(image_number);
// Insert as an element to match large paste placeholder behavior:
// styled distinctly and treated atomically for cursor/mutations.
self.textarea.insert_element(&placeholder);
@@ -477,22 +476,6 @@ impl ChatComposer {
}
}
fn next_image_placeholder(&mut self, base: &str) -> String {
let text = self.textarea.text();
let mut suffix = 1;
loop {
let placeholder = if suffix == 1 {
format!("[{base}]")
} else {
format!("[{base} #{suffix}]")
};
if !text.contains(&placeholder) {
return placeholder;
}
suffix += 1;
}
}
pub(crate) fn insert_str(&mut self, text: &str) {
self.textarea.insert_str(text);
self.sync_popups();
@@ -818,47 +801,43 @@ impl ChatComposer {
if is_image {
// Determine dimensions; if that fails fall back to normal path insertion.
let path_buf = PathBuf::from(&sel_path);
if let Ok((w, h)) = image::image_dimensions(&path_buf) {
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
match image::image_dimensions(&path_buf) {
Ok((width, height)) => {
tracing::debug!("selected image dimensions={}x{}", width, height);
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
let format_label = match Path::new(&sel_path)
.extension()
.and_then(|e| e.to_str())
.map(str::to_ascii_lowercase)
{
Some(ext) if ext == "png" => "PNG",
Some(ext) if ext == "jpg" || ext == "jpeg" => "JPEG",
_ => "IMG",
};
self.attach_image(path_buf, w, h, format_label);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
} else {
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
self.attach_image(path_buf);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
}
Err(err) => {
tracing::trace!("image dimensions lookup failed: {err}");
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
}
}
} else {
// Non-image: inserting file path.
@@ -1464,20 +1443,29 @@ impl ChatComposer {
}
}
// For non-char inputs (or after flushing), handle normally.
// Special handling for backspace on placeholders
if let KeyEvent {
code: KeyCode::Backspace,
..
} = input
&& self.try_remove_any_placeholder_at_cursor()
// Backspace at the start of an image placeholder should delete that placeholder (rather
// than deleting content before it). Do this without scanning the full text by consulting
// the textarea's element list.
if matches!(input.code, KeyCode::Backspace)
&& self.try_remove_image_element_at_cursor_start()
{
return (InputResult::None, true);
}
// Normal input handling
// For non-char inputs (or after flushing), handle normally.
// Track element removals so we can drop any corresponding placeholders without scanning
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
let elements_before = if self.pending_pastes.is_empty() && self.attached_images.is_empty() {
None
} else {
Some(self.textarea.element_payloads())
};
self.textarea.input(input);
let text_after = self.textarea.text();
if let Some(elements_before) = elements_before {
self.reconcile_deleted_elements(elements_before);
}
// Update paste-burst heuristic for plain Char (no Ctrl/Alt) events.
let crossterm::event::KeyEvent {
@@ -1499,176 +1487,69 @@ impl ChatComposer {
}
}
// Check if any placeholders were removed and remove their corresponding pending pastes
self.pending_pastes
.retain(|(placeholder, _)| text_after.contains(placeholder));
// Keep attached images in proportion to how many matching placeholders exist in the text.
// This handles duplicate placeholders that share the same visible label.
if !self.attached_images.is_empty() {
let mut needed: HashMap<String, usize> = HashMap::new();
for img in &self.attached_images {
needed
.entry(img.placeholder.clone())
.or_insert_with(|| text_after.matches(&img.placeholder).count());
}
let mut used: HashMap<String, usize> = HashMap::new();
let mut kept: Vec<AttachedImage> = Vec::with_capacity(self.attached_images.len());
for img in self.attached_images.drain(..) {
let total_needed = *needed.get(&img.placeholder).unwrap_or(&0);
let used_count = used.entry(img.placeholder.clone()).or_insert(0);
if *used_count < total_needed {
kept.push(img);
*used_count += 1;
}
}
self.attached_images = kept;
}
(InputResult::None, true)
}
/// Attempts to remove an image or paste placeholder if the cursor is at the end of one.
/// Returns true if a placeholder was removed.
fn try_remove_any_placeholder_at_cursor(&mut self) -> bool {
// Clamp the cursor to a valid char boundary to avoid panics when slicing.
let text = self.textarea.text();
let p = Self::clamp_to_char_boundary(text, self.textarea.cursor());
// Try image placeholders first
let mut out: Option<(usize, String)> = None;
// Detect if the cursor is at the end of any image placeholder.
// If duplicates exist, remove the specific occurrence's mapping.
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p < ph.len() {
continue;
}
let start = p - ph.len();
if text.get(start..p) != Some(ph.as_str()) {
continue;
}
// Count the number of occurrences of `ph` before `start`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < start {
let segment = match text.get(search_pos..start) {
Some(s) => s,
None => break,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break;
}
}
// Remove the occ_before-th attached image that shares this placeholder label.
out = if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
Some((remove_idx, ph.clone()))
} else {
Some((i, ph.clone()))
};
break;
}
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.attached_images.remove(idx);
return true;
fn try_remove_image_element_at_cursor_start(&mut self) -> bool {
if self.attached_images.is_empty() {
return false;
}
// Also handle when the cursor is at the START of an image placeholder.
// let result = 'out: {
let out: Option<(usize, String)> = 'out: {
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p + ph.len() > text.len() {
continue;
}
if text.get(p..p + ph.len()) != Some(ph.as_str()) {
continue;
}
// Count occurrences of `ph` before `p`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < p {
let segment = match text.get(search_pos..p) {
Some(s) => s,
None => break 'out None,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break 'out None;
}
}
if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
break 'out Some((remove_idx, ph.clone()));
} else {
break 'out Some((i, ph.clone()));
}
}
None
let p = self.textarea.cursor();
let Some(payload) = self.textarea.element_payload_starting_at(p) else {
return false;
};
let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == payload)
else {
return false;
};
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.attached_images.remove(idx);
return true;
self.textarea.replace_range(p..p + payload.len(), "");
self.attached_images.remove(idx);
self.relabel_attached_images_and_update_placeholders();
true
}
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
let elements_after: HashSet<String> =
self.textarea.element_payloads().into_iter().collect();
let mut removed_any_image = false;
for removed in elements_before
.into_iter()
.filter(|payload| !elements_after.contains(payload))
{
self.pending_pastes.retain(|(ph, _)| ph != &removed);
if let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == removed)
{
self.attached_images.remove(idx);
removed_any_image = true;
}
}
// Then try pasted-content placeholders
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p < ph.len() {
return None;
}
let start = p - ph.len();
if text.get(start..p) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
if removed_any_image {
self.relabel_attached_images_and_update_placeholders();
}
}
// Also handle when the cursor is at the START of a pasted-content placeholder.
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p + ph.len() > text.len() {
return None;
fn relabel_attached_images_and_update_placeholders(&mut self) {
for idx in 0..self.attached_images.len() {
let expected = local_image_label_text(idx + 1);
let current = self.attached_images[idx].placeholder.clone();
if current == expected {
continue;
}
if text.get(p..p + ph.len()) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
self.attached_images[idx].placeholder = expected.clone();
let _renamed = self.textarea.replace_element_payload(&current, &expected);
}
false
}
fn handle_shortcut_overlay_key(&mut self, key_event: &KeyEvent) -> bool {
@@ -3477,12 +3358,12 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image1.png");
composer.attach_image(path.clone(), 32, 16, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" hi".into());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1] hi"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3501,11 +3382,11 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image2.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1]"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3526,21 +3407,15 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image_dup.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" ".into());
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
let text = composer.textarea.text().to_string();
assert!(text.contains("[image_dup.png 10x5]"));
assert!(text.contains("[image_dup.png 10x5 #2]"));
assert_eq!(
composer.attached_images[0].placeholder,
"[image_dup.png 10x5]"
);
assert_eq!(
composer.attached_images[1].placeholder,
"[image_dup.png 10x5 #2]"
);
assert!(text.contains("[Image #1]"));
assert!(text.contains("[Image #2]"));
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.attached_images[1].placeholder, "[Image #2]");
}
#[test]
@@ -3555,7 +3430,7 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image3.png");
composer.attach_image(path.clone(), 20, 10, "PNG");
composer.attach_image(path.clone());
let placeholder = composer.attached_images[0].placeholder.clone();
// Case 1: backspace at end
@@ -3566,7 +3441,7 @@ mod tests {
// Re-add and test backspace in middle: should break the placeholder string
// and drop the image mapping (same as text placeholder behavior).
composer.attach_image(path, 20, 10, "PNG");
composer.attach_image(path);
let placeholder2 = composer.attached_images[0].placeholder.clone();
// Move cursor to roughly middle of placeholder
if let Some(start_pos) = composer.textarea.text().find(&placeholder2) {
@@ -3598,7 +3473,7 @@ mod tests {
// Insert an image placeholder at the start
let path = PathBuf::from("/tmp/image_multibyte.png");
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
// Add multibyte text after the placeholder
composer.textarea.insert_str("日本語");
@@ -3607,16 +3482,11 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.attached_images.len(), 1);
assert!(
composer
.textarea
.text()
.starts_with("[image_multibyte.png 10x5]")
);
assert!(composer.textarea.text().starts_with("[Image #1]"));
}
#[test]
fn deleting_one_of_duplicate_image_placeholders_removes_matching_entry() {
fn deleting_one_of_duplicate_image_placeholders_removes_one_entry() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
@@ -3630,10 +3500,10 @@ mod tests {
let path1 = PathBuf::from("/tmp/image_dup1.png");
let path2 = PathBuf::from("/tmp/image_dup2.png");
composer.attach_image(path1, 10, 5, "PNG");
composer.attach_image(path1);
// separate placeholders with a space for clarity
composer.handle_paste(" ".into());
composer.attach_image(path2.clone(), 10, 5, "PNG");
composer.attach_image(path2.clone());
let placeholder1 = composer.attached_images[0].placeholder.clone();
let placeholder2 = composer.attached_images[1].placeholder.clone();
@@ -3647,25 +3517,66 @@ mod tests {
let new_text = composer.textarea.text().to_string();
assert_eq!(
0,
1,
new_text.matches(&placeholder1).count(),
"first placeholder removed"
"one placeholder remains after deletion"
);
assert_eq!(
0,
new_text.matches(&placeholder2).count(),
"second placeholder was relabeled"
);
assert_eq!(
1,
new_text.matches(&placeholder2).count(),
"second placeholder remains"
new_text.matches("[Image #1]").count(),
"remaining placeholder relabeled to #1"
);
assert_eq!(
vec![AttachedImage {
path: path2,
placeholder: "[image_dup2.png 10x5]".to_string()
placeholder: "[Image #1]".to_string()
}],
composer.attached_images,
"one image mapping remains"
);
}
#[test]
fn deleting_first_text_element_renumbers_following_text_element() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let path1 = PathBuf::from("/tmp/image_first.png");
let path2 = PathBuf::from("/tmp/image_second.png");
// Insert two adjacent atomic elements.
composer.attach_image(path1);
composer.attach_image(path2.clone());
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
assert_eq!(composer.attached_images.len(), 2);
// Delete the first element using normal textarea editing (Delete at cursor start).
composer.textarea.set_cursor(0);
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
// Remaining image should be renumbered and the textarea element updated.
assert_eq!(composer.attached_images.len(), 1);
assert_eq!(composer.attached_images[0].path, path2);
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.textarea.text(), "[Image #1]");
}
#[test]
fn pasting_filepath_attaches_image() {
let tmp = tempdir().expect("create TempDir");
@@ -3686,12 +3597,7 @@ mod tests {
let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string());
assert!(needs_redraw);
assert!(
composer
.textarea
.text()
.starts_with("[codex_tui_test_paste_image.png 3x2] ")
);
assert!(composer.textarea.text().starts_with("[Image #1] "));
let imgs = composer.take_recent_submission_images();
assert_eq!(imgs, vec![tmp_path]);
codex-rs/tui/src/bottom_pane/mod.rs
+2 -9
View File
@@ -542,16 +542,9 @@ impl BottomPane {
self.request_redraw();
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
pub(crate) fn attach_image(&mut self, path: PathBuf) {
if self.view_stack.is_empty() {
self.composer
.attach_image(path, width, height, format_label);
self.composer.attach_image(path);
self.request_redraw();
}
}
codex-rs/tui/src/bottom_pane/textarea.rs
+82
View File
@@ -715,6 +715,88 @@ impl TextArea {
// ===== Text elements support =====
pub fn element_payloads(&self) -> Vec<String> {
self.elements
.iter()
.filter_map(|e| self.text.get(e.range.clone()).map(str::to_string))
.collect()
}
pub fn element_payload_starting_at(&self, pos: usize) -> Option<String> {
let pos = pos.min(self.text.len());
let elem = self.elements.iter().find(|e| e.range.start == pos)?;
self.text.get(elem.range.clone()).map(str::to_string)
}
/// Renames a single text element in-place, keeping it atomic.
///
/// This is intended for cases where the element payload is an identifier (e.g. a placeholder)
/// that must be updated without converting the element back into normal text.
pub fn replace_element_payload(&mut self, old: &str, new: &str) -> bool {
let Some(idx) = self
.elements
.iter()
.position(|e| self.text.get(e.range.clone()) == Some(old))
else {
return false;
};
let range = self.elements[idx].range.clone();
let start = range.start;
let end = range.end;
if start > end || end > self.text.len() {
return false;
}
let removed_len = end - start;
let inserted_len = new.len();
let diff = inserted_len as isize - removed_len as isize;
self.text.replace_range(range, new);
self.wrap_cache.replace(None);
self.preferred_col = None;
// Update the modified element's range.
self.elements[idx].range = start..(start + inserted_len);
// Shift element ranges that occur after the replaced element.
if diff != 0 {
for (j, e) in self.elements.iter_mut().enumerate() {
if j == idx {
continue;
}
if e.range.end <= start {
continue;
}
if e.range.start >= end {
e.range.start = ((e.range.start as isize) + diff) as usize;
e.range.end = ((e.range.end as isize) + diff) as usize;
continue;
}
// Elements should not partially overlap each other; degrade gracefully by
// snapping anything intersecting the replaced range to the new bounds.
e.range.start = start.min(e.range.start);
e.range.end = (start + inserted_len).max(e.range.end.saturating_add_signed(diff));
}
}
// Update the cursor position to account for the edit.
self.cursor_pos = if self.cursor_pos < start {
self.cursor_pos
} else if self.cursor_pos <= end {
start + inserted_len
} else {
((self.cursor_pos as isize) + diff) as usize
};
self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos);
// Keep element ordering deterministic.
self.elements.sort_by_key(|e| e.range.start);
true
}
pub fn insert_element(&mut self, text: &str) {
let start = self.clamp_pos_for_insertion(self.cursor_pos);
self.insert_str_at(start, text);
codex-rs/tui/src/chatwidget.rs
+11 -19
View File
@@ -1593,12 +1593,13 @@ impl ChatWidget {
{
match paste_image_to_temp_png() {
Ok((path, info)) => {
self.attach_image(
path,
tracing::debug!(
"pasted image size={}x{} format={}",
info.width,
info.height,
info.encoded_format.label(),
info.encoded_format.label()
);
self.attach_image(path);
}
Err(err) => {
tracing::warn!("failed to paste image: {err}");
@@ -1651,18 +1652,9 @@ impl ChatWidget {
}
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
tracing::info!(
"attach_image path={path:?} width={width} height={height} format={format_label}",
);
self.bottom_pane
.attach_image(path, width, height, format_label);
pub(crate) fn attach_image(&mut self, path: PathBuf) {
tracing::info!("attach_image path={path:?}");
self.bottom_pane.attach_image(path);
self.request_redraw();
}
@@ -2012,14 +2004,14 @@ impl ChatWidget {
return;
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
for path in image_paths {
items.push(UserInput::LocalImage { path });
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
if let Some(skills) = self.bottom_pane.skills() {
let skill_mentions = find_skill_mentions(&text, skills);
for skill in skill_mentions {
codex-rs/tui/src/chatwidget/tests.rs
+2 -2
View File
@@ -1114,8 +1114,8 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
chat.bottom_pane.insert_str("draft message ");
chat.bottom_pane
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
let placeholder = "[preview.png 24x42]";
.attach_image(PathBuf::from("/tmp/preview.png"));
let placeholder = "[Image #1]";
assert!(
chat.bottom_pane.composer_text().ends_with(placeholder),
"expected placeholder {placeholder:?} in composer text"
codex-rs/tui2/src/bottom_pane/chat_composer.rs
+163 -240
View File
@@ -49,6 +49,7 @@ use crate::style::user_message_style;
use codex_common::fuzzy_match::fuzzy_match;
use codex_protocol::custom_prompts::CustomPrompt;
use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX;
use codex_protocol::models::local_image_label_text;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
@@ -62,7 +63,7 @@ use codex_core::skills::model::SkillMetadata;
use codex_file_search::FileMatch;
use std::cell::RefCell;
use std::collections::HashMap;
use std::path::Path;
use std::collections::HashSet;
use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
@@ -287,10 +288,12 @@ impl ChatComposer {
// normalize_pasted_path already handles Windows → WSL path conversion,
// so we can directly try to read the image dimensions.
match image::image_dimensions(&path_buf) {
Ok((w, h)) => {
Ok((width, height)) => {
tracing::info!("OK: {pasted}");
let format_label = pasted_image_format(&path_buf).label();
self.attach_image(path_buf, w, h, format_label);
tracing::debug!("image dimensions={}x{}", width, height);
let format = pasted_image_format(&path_buf);
tracing::debug!("attached image format={}", format.label());
self.attach_image(path_buf);
true
}
Err(err) => {
@@ -342,12 +345,9 @@ impl ChatComposer {
}
/// Attempt to start a burst by retro-capturing recent chars before the cursor.
pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, _format_label: &str) {
let file_label = path
.file_name()
.map(|name| name.to_string_lossy().into_owned())
.unwrap_or_else(|| "image".to_string());
let placeholder = format!("[{file_label} {width}x{height}]");
pub fn attach_image(&mut self, path: PathBuf) {
let image_number = self.attached_images.len() + 1;
let placeholder = local_image_label_text(image_number);
// Insert as an element to match large paste placeholder behavior:
// styled distinctly and treated atomically for cursor/mutations.
self.textarea.insert_element(&placeholder);
@@ -735,47 +735,43 @@ impl ChatComposer {
if is_image {
// Determine dimensions; if that fails fall back to normal path insertion.
let path_buf = PathBuf::from(&sel_path);
if let Ok((w, h)) = image::image_dimensions(&path_buf) {
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
match image::image_dimensions(&path_buf) {
Ok((width, height)) => {
tracing::debug!("selected image dimensions={}x{}", width, height);
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
let format_label = match Path::new(&sel_path)
.extension()
.and_then(|e| e.to_str())
.map(str::to_ascii_lowercase)
{
Some(ext) if ext == "png" => "PNG",
Some(ext) if ext == "jpg" || ext == "jpeg" => "JPEG",
_ => "IMG",
};
self.attach_image(path_buf, w, h, format_label);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
} else {
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
self.attach_image(path_buf);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
}
Err(err) => {
tracing::trace!("image dimensions lookup failed: {err}");
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
}
}
} else {
// Non-image: inserting file path.
@@ -1381,20 +1377,28 @@ impl ChatComposer {
}
}
// For non-char inputs (or after flushing), handle normally.
// Special handling for backspace on placeholders
if let KeyEvent {
code: KeyCode::Backspace,
..
} = input
&& self.try_remove_any_placeholder_at_cursor()
// Backspace at the start of an image placeholder should delete that placeholder (rather
// than deleting content before it). Do this without scanning the full text by consulting
// the textarea's element list.
if matches!(input.code, KeyCode::Backspace)
&& self.try_remove_image_element_at_cursor_start()
{
return (InputResult::None, true);
}
// Normal input handling
// Track element removals so we can drop any corresponding placeholders without scanning
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
let elements_before = if self.pending_pastes.is_empty() && self.attached_images.is_empty() {
None
} else {
Some(self.textarea.element_payloads())
};
self.textarea.input(input);
let text_after = self.textarea.text();
if let Some(elements_before) = elements_before {
self.reconcile_deleted_elements(elements_before);
}
// Update paste-burst heuristic for plain Char (no Ctrl/Alt) events.
let crossterm::event::KeyEvent {
@@ -1416,176 +1420,69 @@ impl ChatComposer {
}
}
// Check if any placeholders were removed and remove their corresponding pending pastes
self.pending_pastes
.retain(|(placeholder, _)| text_after.contains(placeholder));
// Keep attached images in proportion to how many matching placeholders exist in the text.
// This handles duplicate placeholders that share the same visible label.
if !self.attached_images.is_empty() {
let mut needed: HashMap<String, usize> = HashMap::new();
for img in &self.attached_images {
needed
.entry(img.placeholder.clone())
.or_insert_with(|| text_after.matches(&img.placeholder).count());
}
let mut used: HashMap<String, usize> = HashMap::new();
let mut kept: Vec<AttachedImage> = Vec::with_capacity(self.attached_images.len());
for img in self.attached_images.drain(..) {
let total_needed = *needed.get(&img.placeholder).unwrap_or(&0);
let used_count = used.entry(img.placeholder.clone()).or_insert(0);
if *used_count < total_needed {
kept.push(img);
*used_count += 1;
}
}
self.attached_images = kept;
}
(InputResult::None, true)
}
/// Attempts to remove an image or paste placeholder if the cursor is at the end of one.
/// Returns true if a placeholder was removed.
fn try_remove_any_placeholder_at_cursor(&mut self) -> bool {
// Clamp the cursor to a valid char boundary to avoid panics when slicing.
let text = self.textarea.text();
let p = Self::clamp_to_char_boundary(text, self.textarea.cursor());
// Try image placeholders first
let mut out: Option<(usize, String)> = None;
// Detect if the cursor is at the end of any image placeholder.
// If duplicates exist, remove the specific occurrence's mapping.
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p < ph.len() {
continue;
}
let start = p - ph.len();
if text.get(start..p) != Some(ph.as_str()) {
continue;
}
// Count the number of occurrences of `ph` before `start`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < start {
let segment = match text.get(search_pos..start) {
Some(s) => s,
None => break,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break;
}
}
// Remove the occ_before-th attached image that shares this placeholder label.
out = if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
Some((remove_idx, ph.clone()))
} else {
Some((i, ph.clone()))
};
break;
}
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.attached_images.remove(idx);
return true;
fn try_remove_image_element_at_cursor_start(&mut self) -> bool {
if self.attached_images.is_empty() {
return false;
}
// Also handle when the cursor is at the START of an image placeholder.
// let result = 'out: {
let out: Option<(usize, String)> = 'out: {
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p + ph.len() > text.len() {
continue;
}
if text.get(p..p + ph.len()) != Some(ph.as_str()) {
continue;
}
// Count occurrences of `ph` before `p`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < p {
let segment = match text.get(search_pos..p) {
Some(s) => s,
None => break 'out None,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break 'out None;
}
}
if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
break 'out Some((remove_idx, ph.clone()));
} else {
break 'out Some((i, ph.clone()));
}
}
None
let p = self.textarea.cursor();
let Some(payload) = self.textarea.element_payload_starting_at(p) else {
return false;
};
let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == payload)
else {
return false;
};
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.attached_images.remove(idx);
return true;
self.textarea.replace_range(p..p + payload.len(), "");
self.attached_images.remove(idx);
self.relabel_attached_images_and_update_placeholders();
true
}
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
let elements_after: HashSet<String> =
self.textarea.element_payloads().into_iter().collect();
let mut removed_any_image = false;
for removed in elements_before
.into_iter()
.filter(|payload| !elements_after.contains(payload))
{
self.pending_pastes.retain(|(ph, _)| ph != &removed);
if let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == removed)
{
self.attached_images.remove(idx);
removed_any_image = true;
}
}
// Then try pasted-content placeholders
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p < ph.len() {
return None;
}
let start = p - ph.len();
if text.get(start..p) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
if removed_any_image {
self.relabel_attached_images_and_update_placeholders();
}
}
// Also handle when the cursor is at the START of a pasted-content placeholder.
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p + ph.len() > text.len() {
return None;
fn relabel_attached_images_and_update_placeholders(&mut self) {
for idx in 0..self.attached_images.len() {
let expected = local_image_label_text(idx + 1);
let current = self.attached_images[idx].placeholder.clone();
if current == expected {
continue;
}
if text.get(p..p + ph.len()) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
self.attached_images[idx].placeholder = expected.clone();
let _renamed = self.textarea.replace_element_payload(&current, &expected);
}
false
}
fn handle_shortcut_overlay_key(&mut self, key_event: &KeyEvent) -> bool {
@@ -3408,12 +3305,12 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image1.png");
composer.attach_image(path.clone(), 32, 16, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" hi".into());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1] hi"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3432,11 +3329,11 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image2.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1]"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3457,7 +3354,7 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image3.png");
composer.attach_image(path.clone(), 20, 10, "PNG");
composer.attach_image(path.clone());
let placeholder = composer.attached_images[0].placeholder.clone();
// Case 1: backspace at end
@@ -3468,7 +3365,7 @@ mod tests {
// Re-add and test backspace in middle: should break the placeholder string
// and drop the image mapping (same as text placeholder behavior).
composer.attach_image(path, 20, 10, "PNG");
composer.attach_image(path);
let placeholder2 = composer.attached_images[0].placeholder.clone();
// Move cursor to roughly middle of placeholder
if let Some(start_pos) = composer.textarea.text().find(&placeholder2) {
@@ -3500,7 +3397,7 @@ mod tests {
// Insert an image placeholder at the start
let path = PathBuf::from("/tmp/image_multibyte.png");
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
// Add multibyte text after the placeholder
composer.textarea.insert_str("日本語");
@@ -3509,12 +3406,7 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.attached_images.len(), 1);
assert!(
composer
.textarea
.text()
.starts_with("[image_multibyte.png 10x5]")
);
assert!(composer.textarea.text().starts_with("[Image #1]"));
}
#[test]
@@ -3532,10 +3424,10 @@ mod tests {
let path1 = PathBuf::from("/tmp/image_dup1.png");
let path2 = PathBuf::from("/tmp/image_dup2.png");
composer.attach_image(path1, 10, 5, "PNG");
composer.attach_image(path1);
// separate placeholders with a space for clarity
composer.handle_paste(" ".into());
composer.attach_image(path2.clone(), 10, 5, "PNG");
composer.attach_image(path2.clone());
let placeholder1 = composer.attached_images[0].placeholder.clone();
let placeholder2 = composer.attached_images[1].placeholder.clone();
@@ -3550,24 +3442,60 @@ mod tests {
let new_text = composer.textarea.text().to_string();
assert_eq!(
0,
new_text.matches(&placeholder1).count(),
"first placeholder removed"
new_text.matches(&placeholder2).count(),
"second placeholder was relabeled"
);
assert_eq!(
1,
new_text.matches(&placeholder2).count(),
"second placeholder remains"
new_text.matches("[Image #1]").count(),
"remaining placeholder relabeled to #1"
);
assert_eq!(
vec![AttachedImage {
path: path2,
placeholder: "[image_dup2.png 10x5]".to_string()
placeholder: "[Image #1]".to_string()
}],
composer.attached_images,
"one image mapping remains"
);
}
#[test]
fn deleting_first_text_element_renumbers_following_text_element() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let path1 = PathBuf::from("/tmp/image_first.png");
let path2 = PathBuf::from("/tmp/image_second.png");
// Insert two adjacent atomic elements.
composer.attach_image(path1);
composer.attach_image(path2.clone());
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
assert_eq!(composer.attached_images.len(), 2);
// Delete the first element using normal textarea editing (Delete at cursor start).
composer.textarea.set_cursor(0);
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
// Remaining image should be renumbered and the textarea element updated.
assert_eq!(composer.attached_images.len(), 1);
assert_eq!(composer.attached_images[0].path, path2);
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.textarea.text(), "[Image #1]");
}
#[test]
fn pasting_filepath_attaches_image() {
let tmp = tempdir().expect("create TempDir");
@@ -3588,12 +3516,7 @@ mod tests {
let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string());
assert!(needs_redraw);
assert!(
composer
.textarea
.text()
.starts_with("[codex_tui_test_paste_image.png 3x2] ")
);
assert!(composer.textarea.text().starts_with("[Image #1] "));
let imgs = composer.take_recent_submission_images();
assert_eq!(imgs, vec![tmp_path]);
codex-rs/tui2/src/bottom_pane/mod.rs
+2 -9
View File
@@ -529,16 +529,9 @@ impl BottomPane {
self.request_redraw();
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
pub(crate) fn attach_image(&mut self, path: PathBuf) {
if self.view_stack.is_empty() {
self.composer
.attach_image(path, width, height, format_label);
self.composer.attach_image(path);
self.request_redraw();
}
}
codex-rs/tui2/src/bottom_pane/textarea.rs
+82
View File
@@ -715,6 +715,88 @@ impl TextArea {
// ===== Text elements support =====
pub fn element_payloads(&self) -> Vec<String> {
self.elements
.iter()
.filter_map(|e| self.text.get(e.range.clone()).map(str::to_string))
.collect()
}
pub fn element_payload_starting_at(&self, pos: usize) -> Option<String> {
let pos = pos.min(self.text.len());
let elem = self.elements.iter().find(|e| e.range.start == pos)?;
self.text.get(elem.range.clone()).map(str::to_string)
}
/// Renames a single text element in-place, keeping it atomic.
///
/// This is intended for cases where the element payload is an identifier (e.g. a placeholder)
/// that must be updated without converting the element back into normal text.
pub fn replace_element_payload(&mut self, old: &str, new: &str) -> bool {
let Some(idx) = self
.elements
.iter()
.position(|e| self.text.get(e.range.clone()) == Some(old))
else {
return false;
};
let range = self.elements[idx].range.clone();
let start = range.start;
let end = range.end;
if start > end || end > self.text.len() {
return false;
}
let removed_len = end - start;
let inserted_len = new.len();
let diff = inserted_len as isize - removed_len as isize;
self.text.replace_range(range, new);
self.wrap_cache.replace(None);
self.preferred_col = None;
// Update the modified element's range.
self.elements[idx].range = start..(start + inserted_len);
// Shift element ranges that occur after the replaced element.
if diff != 0 {
for (j, e) in self.elements.iter_mut().enumerate() {
if j == idx {
continue;
}
if e.range.end <= start {
continue;
}
if e.range.start >= end {
e.range.start = ((e.range.start as isize) + diff) as usize;
e.range.end = ((e.range.end as isize) + diff) as usize;
continue;
}
// Elements should not partially overlap each other; degrade gracefully by
// snapping anything intersecting the replaced range to the new bounds.
e.range.start = start.min(e.range.start);
e.range.end = (start + inserted_len).max(e.range.end.saturating_add_signed(diff));
}
}
// Update the cursor position to account for the edit.
self.cursor_pos = if self.cursor_pos < start {
self.cursor_pos
} else if self.cursor_pos <= end {
start + inserted_len
} else {
((self.cursor_pos as isize) + diff) as usize
};
self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos);
// Keep element ordering deterministic.
self.elements.sort_by_key(|e| e.range.start);
true
}
pub fn insert_element(&mut self, text: &str) {
let start = self.clamp_pos_for_insertion(self.cursor_pos);
self.insert_str_at(start, text);
codex-rs/tui2/src/chatwidget.rs
+11 -19
View File
@@ -1452,12 +1452,13 @@ impl ChatWidget {
{
match paste_image_to_temp_png() {
Ok((path, info)) => {
self.attach_image(
path,
tracing::debug!(
"pasted image size={}x{} format={}",
info.width,
info.height,
info.encoded_format.label(),
info.encoded_format.label()
);
self.attach_image(path);
}
Err(err) => {
tracing::warn!("failed to paste image: {err}");
@@ -1510,18 +1511,9 @@ impl ChatWidget {
}
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
tracing::info!(
"attach_image path={path:?} width={width} height={height} format={format_label}",
);
self.bottom_pane
.attach_image(path, width, height, format_label);
pub(crate) fn attach_image(&mut self, path: PathBuf) {
tracing::info!("attach_image path={path:?}");
self.bottom_pane.attach_image(path);
self.request_redraw();
}
@@ -1818,14 +1810,14 @@ impl ChatWidget {
return;
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
for path in image_paths {
items.push(UserInput::LocalImage { path });
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
if let Some(skills) = self.bottom_pane.skills() {
let skill_mentions = find_skill_mentions(&text, skills);
for skill in skill_mentions {
codex-rs/tui2/src/chatwidget/tests.rs
+2 -2
View File
@@ -1065,8 +1065,8 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
chat.bottom_pane.insert_str("draft message ");
chat.bottom_pane
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
let placeholder = "[preview.png 24x42]";
.attach_image(PathBuf::from("/tmp/preview.png"));
let placeholder = "[Image #1]";
assert!(
chat.bottom_pane.composer_text().ends_with(placeholder),
"expected placeholder {placeholder:?} in composer text"