diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 1faf8bacf1..8140d413f7 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -78,7 +78,8 @@ pub enum InputResult { #[derive(Clone, Debug, PartialEq)] struct AttachedImage { - placeholder: String, + display_placeholder: String, + model_placeholder: String, path: PathBuf, } @@ -291,7 +292,11 @@ impl ChatComposer { // Count placeholder occurrences in the new text. let mut placeholder_counts: HashMap = HashMap::new(); - for placeholder in self.attached_images.iter().map(|img| &img.placeholder) { + for placeholder in self + .attached_images + .iter() + .map(|img| &img.display_placeholder) + { if placeholder_counts.contains_key(placeholder) { continue; } @@ -304,7 +309,7 @@ impl ChatComposer { // Keep attachments only while we have matching occurrences left. let mut kept_images = Vec::new(); for img in self.attached_images.drain(..) { - if let Some(count) = placeholder_counts.get_mut(&img.placeholder) + if let Some(count) = placeholder_counts.get_mut(&img.display_placeholder) && *count > 0 { *count -= 1; @@ -317,7 +322,9 @@ impl ChatComposer { self.textarea.set_text(""); let mut remaining: HashMap<&str, usize> = HashMap::new(); for img in &self.attached_images { - *remaining.entry(img.placeholder.as_str()).or_insert(0) += 1; + *remaining + .entry(img.display_placeholder.as_str()) + .or_insert(0) += 1; } let mut occurrences: Vec<(usize, &str)> = Vec::new(); @@ -396,15 +403,26 @@ 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) { - // Include the full path in the placeholder so the model can refer back to the local file. - let file_label = path.display().to_string(); - let base_placeholder = format!("{file_label} {width}x{height}"); - let placeholder = self.next_image_placeholder(&base_placeholder); + let display_label = Self::display_label_for_image_placeholder(&path); + let full_label = path.display().to_string(); + + let display_base = format!("{display_label} {width}x{height}"); + let model_base = format!("{full_label} {width}x{height}"); + + let display_placeholder = self.next_image_placeholder(&display_base); + let model_placeholder = Self::model_placeholder_from_display_placeholder( + &display_placeholder, + &display_base, + &model_base, + ); // Insert as an element to match large paste placeholder behavior: // styled distinctly and treated atomically for cursor/mutations. - self.textarea.insert_element(&placeholder); - self.attached_images - .push(AttachedImage { placeholder, path }); + self.textarea.insert_element(&display_placeholder); + self.attached_images.push(AttachedImage { + display_placeholder, + model_placeholder, + path, + }); } pub fn take_recent_submission_images(&mut self) -> Vec { @@ -412,6 +430,16 @@ impl ChatComposer { images.into_iter().map(|img| img.path).collect() } + pub fn expand_attached_image_placeholders_for_model(&self, display_text: &str) -> String { + let mut text = display_text.to_string(); + for img in &self.attached_images { + if text.contains(&img.display_placeholder) { + text = text.replace(&img.display_placeholder, &img.model_placeholder); + } + } + text + } + pub(crate) fn flush_paste_burst_if_due(&mut self) -> bool { self.handle_paste_burst_flush(Instant::now()) } @@ -478,6 +506,56 @@ impl ChatComposer { } } + fn display_label_for_image_placeholder(path: &Path) -> String { + let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else { + return "image".to_string(); + }; + + // Avoid dumping very long absolute paths into the composer UI (common when attaching screenshots). + if !path.is_absolute() { + return path.display().to_string(); + } + + // Count normal path segments (including the file name). + let mut segments: Vec = Vec::new(); + for c in path.components() { + if let std::path::Component::Normal(seg) = c { + segments.push(seg.to_string_lossy().into_owned()); + } + } + + // Keep short absolute paths readable. + if segments.len() <= 2 { + return path.display().to_string(); + } + + let top = segments + .first() + .cloned() + .unwrap_or_else(|| "...".to_string()); + format!("/{top}/.../{file_name}") + } + + fn model_placeholder_from_display_placeholder( + display_placeholder: &str, + display_base: &str, + model_base: &str, + ) -> String { + let Some(inner) = display_placeholder + .strip_prefix('[') + .and_then(|s| s.strip_suffix(']')) + else { + return format!("[{model_base}]"); + }; + + if let Some(tail) = inner.strip_prefix(display_base) { + format!("[{model_base}{tail}]") + } else { + // Fallback: preserve the placeholder bracket shape even if we can't recover the suffix. + format!("[{model_base}]") + } + } + pub(crate) fn insert_str(&mut self, text: &str) { self.textarea.insert_str(text); self.sync_popups(); @@ -1436,15 +1514,15 @@ impl ChatComposer { let mut needed: HashMap = HashMap::new(); for img in &self.attached_images { needed - .entry(img.placeholder.clone()) - .or_insert_with(|| text_after.matches(&img.placeholder).count()); + .entry(img.display_placeholder.clone()) + .or_insert_with(|| text_after.matches(&img.display_placeholder).count()); } let mut used: HashMap = HashMap::new(); let mut kept: Vec = 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); + let total_needed = *needed.get(&img.display_placeholder).unwrap_or(&0); + let used_count = used.entry(img.display_placeholder.clone()).or_insert(0); if *used_count < total_needed { kept.push(img); *used_count += 1; @@ -1468,7 +1546,7 @@ impl ChatComposer { // 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; + let ph = &img.display_placeholder; if p < ph.len() { continue; } @@ -1498,7 +1576,7 @@ impl ChatComposer { .attached_images .iter() .enumerate() - .filter(|(_, img2)| img2.placeholder == *ph) + .filter(|(_, img2)| img2.display_placeholder == *ph) .nth(occ_before) { Some((remove_idx, ph.clone())) @@ -1517,7 +1595,7 @@ impl ChatComposer { // let result = 'out: { let out: Option<(usize, String)> = 'out: { for (i, img) in self.attached_images.iter().enumerate() { - let ph = &img.placeholder; + let ph = &img.display_placeholder; if p + ph.len() > text.len() { continue; } @@ -1545,7 +1623,7 @@ impl ChatComposer { .attached_images .iter() .enumerate() - .filter(|(_, img2)| img2.placeholder == *ph) + .filter(|(_, img2)| img2.display_placeholder == *ph) .nth(occ_before) { break 'out Some((remove_idx, ph.clone())); @@ -3294,11 +3372,11 @@ mod tests { assert!(text.contains("[/tmp/image_dup.png 10x5]")); assert!(text.contains("[/tmp/image_dup.png 10x5 #2]")); assert_eq!( - composer.attached_images[0].placeholder, + composer.attached_images[0].display_placeholder, "[/tmp/image_dup.png 10x5]" ); assert_eq!( - composer.attached_images[1].placeholder, + composer.attached_images[1].display_placeholder, "[/tmp/image_dup.png 10x5 #2]" ); } @@ -3316,7 +3394,7 @@ mod tests { ); let path = PathBuf::from("/tmp/image3.png"); composer.attach_image(path.clone(), 20, 10, "PNG"); - let placeholder = composer.attached_images[0].placeholder.clone(); + let placeholder = composer.attached_images[0].display_placeholder.clone(); // Case 1: backspace at end composer.textarea.move_cursor_to_end_of_line(false); @@ -3327,7 +3405,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"); - let placeholder2 = composer.attached_images[0].placeholder.clone(); + let placeholder2 = composer.attached_images[0].display_placeholder.clone(); // Move cursor to roughly middle of placeholder if let Some(start_pos) = composer.textarea.text().find(&placeholder2) { let mid_pos = start_pos + (placeholder2.len() / 2); @@ -3399,8 +3477,8 @@ mod tests { composer.handle_paste(" ".into()); composer.attach_image(path2.clone(), 10, 5, "PNG"); - let placeholder1 = composer.attached_images[0].placeholder.clone(); - let placeholder2 = composer.attached_images[1].placeholder.clone(); + let placeholder1 = composer.attached_images[0].display_placeholder.clone(); + let placeholder2 = composer.attached_images[1].display_placeholder.clone(); let text = composer.textarea.text().to_string(); let start1 = text.find(&placeholder1).expect("first placeholder present"); let end1 = start1 + placeholder1.len(); @@ -3423,7 +3501,8 @@ mod tests { assert_eq!( vec![AttachedImage { path: path2, - placeholder: "[/tmp/image_dup2.png 10x5]".to_string() + display_placeholder: "[/tmp/image_dup2.png 10x5]".to_string(), + model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string() }], composer.attached_images, "one image mapping remains" @@ -3450,7 +3529,8 @@ mod tests { let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string()); assert!(needs_redraw); - let expected_prefix = format!("[{} 3x2] ", tmp_path.display()); + let display_label = ChatComposer::display_label_for_image_placeholder(&tmp_path); + let expected_prefix = format!("[{display_label} 3x2] "); assert!(composer.textarea.text().starts_with(&expected_prefix)); let imgs = composer.take_recent_submission_images(); @@ -4168,7 +4248,8 @@ mod tests { let placeholder = "[image 10x10]".to_string(); composer.textarea.insert_element(&placeholder); composer.attached_images.push(AttachedImage { - placeholder: placeholder.clone(), + display_placeholder: placeholder.clone(), + model_placeholder: placeholder.clone(), path: PathBuf::from("img.png"), }); composer @@ -4183,7 +4264,7 @@ mod tests { ); assert!(composer.pending_pastes.is_empty()); assert_eq!(composer.attached_images.len(), 1); - assert_eq!(composer.attached_images[0].placeholder, placeholder); + assert_eq!(composer.attached_images[0].display_placeholder, placeholder); assert_eq!(composer.textarea.cursor(), composer.current_text().len()); } @@ -4202,7 +4283,8 @@ mod tests { let placeholder = "[image 10x10]".to_string(); composer.textarea.insert_element(&placeholder); composer.attached_images.push(AttachedImage { - placeholder: placeholder.clone(), + display_placeholder: placeholder.clone(), + model_placeholder: placeholder.clone(), path: PathBuf::from("img.png"), }); @@ -4252,7 +4334,8 @@ mod tests { let placeholder = "[image 10x10]".to_string(); composer.textarea.insert_element(&placeholder); composer.attached_images.push(AttachedImage { - placeholder: placeholder.clone(), + display_placeholder: placeholder.clone(), + model_placeholder: placeholder.clone(), path: PathBuf::from("img.png"), }); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 7634f699aa..e9a15dda51 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -549,6 +549,14 @@ impl BottomPane { self.composer.take_recent_submission_images() } + pub(crate) fn expand_attached_image_placeholders_for_model( + &self, + display_text: &str, + ) -> String { + self.composer + .expand_attached_image_placeholders_for_model(display_text) + } + fn as_renderable(&'_ self) -> RenderableItem<'_> { if let Some(view) = self.active_view() { RenderableItem::Borrowed(view) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f4418ceadb..a25dd1187d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -371,14 +371,16 @@ pub(crate) struct ChatWidget { } struct UserMessage { - text: String, + display_text: String, + model_text: String, image_paths: Vec, } impl From for UserMessage { fn from(text: String) -> Self { Self { - text, + display_text: text.clone(), + model_text: text, image_paths: Vec::new(), } } @@ -387,7 +389,8 @@ impl From for UserMessage { impl From<&str> for UserMessage { fn from(text: &str) -> Self { Self { - text: text.to_string(), + display_text: text.to_string(), + model_text: text.to_string(), image_paths: Vec::new(), } } @@ -397,7 +400,11 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio if text.is_empty() && image_paths.is_empty() { None } else { - Some(UserMessage { text, image_paths }) + Some(UserMessage { + display_text: text.clone(), + model_text: text, + image_paths, + }) } } @@ -813,7 +820,7 @@ impl ChatWidget { let queued_text = self .queued_user_messages .iter() - .map(|m| m.text.clone()) + .map(|m| m.display_text.clone()) .collect::>() .join("\n"); let existing_text = self.bottom_pane.composer_text(); @@ -1621,7 +1628,8 @@ impl ChatWidget { } if !self.queued_user_messages.is_empty() => { // Prefer the most recently queued item. if let Some(user_message) = self.queued_user_messages.pop_back() { - self.bottom_pane.set_composer_text(user_message.text); + self.bottom_pane + .set_composer_text(user_message.display_text); self.refresh_queued_user_messages(); self.request_redraw(); } @@ -1630,8 +1638,12 @@ impl ChatWidget { match self.bottom_pane.handle_key_event(key_event) { InputResult::Submitted(text) => { // If a task is running, queue the user input to be sent after the turn completes. + let model_text = self + .bottom_pane + .expand_attached_image_placeholders_for_model(&text); let user_message = UserMessage { - text, + display_text: text, + model_text, image_paths: self.bottom_pane.take_recent_submission_images(), }; self.queue_user_message(user_message); @@ -1909,15 +1921,19 @@ impl ChatWidget { } fn submit_user_message(&mut self, user_message: UserMessage) { - let UserMessage { text, image_paths } = user_message; - if text.is_empty() && image_paths.is_empty() { + let UserMessage { + display_text, + model_text, + image_paths, + } = user_message; + if display_text.is_empty() && image_paths.is_empty() { return; } let mut items: Vec = Vec::new(); // Special-case: "!cmd" executes a local shell command instead of sending to the model. - if let Some(stripped) = text.strip_prefix('!') { + if let Some(stripped) = display_text.strip_prefix('!') { let cmd = stripped.trim(); if cmd.is_empty() { self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( @@ -1934,8 +1950,8 @@ impl ChatWidget { return; } - if !text.is_empty() { - items.push(UserInput::Text { text: text.clone() }); + if !model_text.is_empty() { + items.push(UserInput::Text { text: model_text }); } for path in image_paths { @@ -1943,7 +1959,7 @@ impl ChatWidget { } if let Some(skills) = self.bottom_pane.skills() { - let skill_mentions = find_skill_mentions(&text, skills); + let skill_mentions = find_skill_mentions(&display_text, skills); for skill in skill_mentions { items.push(UserInput::Skill { name: skill.name.clone(), @@ -1959,17 +1975,19 @@ impl ChatWidget { }); // Persist the text to cross-session message history. - if !text.is_empty() { + if !display_text.is_empty() { self.codex_op_tx - .send(Op::AddToHistory { text: text.clone() }) + .send(Op::AddToHistory { + text: display_text.clone(), + }) .unwrap_or_else(|e| { tracing::error!("failed to send AddHistory op: {e}"); }); } // Only show the text portion in conversation history. - if !text.is_empty() { - self.add_to_history(history_cell::new_user_prompt(text)); + if !display_text.is_empty() { + self.add_to_history(history_cell::new_user_prompt(display_text)); } self.needs_final_message_separator = false; } @@ -2226,7 +2244,7 @@ impl ChatWidget { let messages: Vec = self .queued_user_messages .iter() - .map(|m| m.text.clone()) + .map(|m| m.display_text.clone()) .collect(); self.bottom_pane.set_queued_user_messages(messages); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 31b7c926cd..d380e12d42 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1010,7 +1010,7 @@ async fn alt_up_edits_most_recent_queued_message() { // And the queue should now contain only the remaining (older) item. assert_eq!(chat.queued_user_messages.len(), 1); assert_eq!( - chat.queued_user_messages.front().unwrap().text, + chat.queued_user_messages.front().unwrap().display_text, "first queued" ); } @@ -1040,7 +1040,7 @@ async fn enqueueing_history_prompt_multiple_times_is_stable() { assert_eq!(chat.queued_user_messages.len(), 3); for message in chat.queued_user_messages.iter() { - assert_eq!(message.text, "repeat me"); + assert_eq!(message.display_text, "repeat me"); } } @@ -1061,7 +1061,7 @@ async fn streaming_final_answer_keeps_task_running_state() { assert_eq!(chat.queued_user_messages.len(), 1); assert_eq!( - chat.queued_user_messages.front().unwrap().text, + chat.queued_user_messages.front().unwrap().display_text, "queued submission" ); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); @@ -1120,6 +1120,67 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { ); } +#[tokio::test] +async fn submitting_image_shows_short_path_but_sends_full_path_to_model() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + // Use a long absolute path so the display placeholder is shortened. + let image_path = PathBuf::from("/var/tmp/some/dir/image.png"); + + chat.bottom_pane.insert_str("see "); + chat.bottom_pane + .attach_image(image_path.clone(), 24, 42, "png"); + + let display_text = chat.bottom_pane.composer_text(); + assert!( + display_text.contains("[/var/.../image.png 24x42]"), + "expected shortened display placeholder, got {display_text:?}" + ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + // The user-visible transcript should keep the short placeholder. + let cells = drain_insert_history(&mut rx); + assert!( + !cells.is_empty(), + "expected a user history cell to be inserted" + ); + let rendered = lines_to_single_string(&cells[0]); + assert!( + rendered.contains("/var/.../image.png"), + "expected transcript to contain shortened placeholder, got {rendered:?}" + ); + assert!( + !rendered.contains("/var/tmp/some/dir/image.png"), + "expected transcript not to contain full path, got {rendered:?}" + ); + + // The model-facing text should expand the placeholder to the full path. + let mut model_text: Option = None; + while let Ok(op) = op_rx.try_recv() { + if let Op::UserInput { items } = op { + for item in items { + if let codex_protocol::user_input::UserInput::Text { text } = item { + model_text = Some(text); + break; + } + } + } + if model_text.is_some() { + break; + } + } + let model_text = model_text.expect("expected Op::UserInput with a Text item"); + assert!( + model_text.contains("/var/tmp/some/dir/image.png"), + "expected model text to contain full path, got {model_text:?}" + ); + assert!( + !model_text.contains("/var/.../image.png"), + "expected model text not to contain shortened placeholder, got {model_text:?}" + ); +} + #[tokio::test] async fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index d3741fcd07..67ab352def 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -81,7 +81,8 @@ pub enum InputResult { #[derive(Clone, Debug, PartialEq)] struct AttachedImage { - placeholder: String, + display_placeholder: String, + model_placeholder: String, path: PathBuf, } @@ -191,6 +192,36 @@ impl ChatComposer { self.skills = skills; } + fn display_label_for_image_placeholder(path: &Path) -> String { + let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else { + return "image".to_string(); + }; + + // Avoid dumping very long absolute paths into the composer UI (common when attaching screenshots). + if !path.is_absolute() { + return path.display().to_string(); + } + + // Count normal path segments (including the file name). + let mut segments: Vec = Vec::new(); + for c in path.components() { + if let std::path::Component::Normal(seg) = c { + segments.push(seg.to_string_lossy().into_owned()); + } + } + + // Keep short absolute paths readable. + if segments.len() <= 2 { + return path.display().to_string(); + } + + let top = segments + .first() + .cloned() + .unwrap_or_else(|| "...".to_string()); + format!("/{top}/.../{file_name}") + } + fn layout_areas(&self, area: Rect) -> [Rect; 3] { let footer_props = self.footer_props(); let footer_hint_height = self @@ -330,14 +361,18 @@ 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) { - // Include the full path in the placeholder so the model can refer back to the local file. - let file_label = path.display().to_string(); - let placeholder = format!("[{file_label} {width}x{height}]"); + let display_label = Self::display_label_for_image_placeholder(&path); + let full_label = path.display().to_string(); + let display_placeholder = format!("[{display_label} {width}x{height}]"); + let model_placeholder = format!("[{full_label} {width}x{height}]"); // Insert as an element to match large paste placeholder behavior: // styled distinctly and treated atomically for cursor/mutations. - self.textarea.insert_element(&placeholder); - self.attached_images - .push(AttachedImage { placeholder, path }); + self.textarea.insert_element(&display_placeholder); + self.attached_images.push(AttachedImage { + display_placeholder, + model_placeholder, + path, + }); } pub fn take_recent_submission_images(&mut self) -> Vec { @@ -345,6 +380,16 @@ impl ChatComposer { images.into_iter().map(|img| img.path).collect() } + pub fn expand_attached_image_placeholders_for_model(&self, display_text: &str) -> String { + let mut text = display_text.to_string(); + for img in &self.attached_images { + if text.contains(&img.display_placeholder) { + text = text.replace(&img.display_placeholder, &img.model_placeholder); + } + } + text + } + pub(crate) fn flush_paste_burst_if_due(&mut self) -> bool { self.handle_paste_burst_flush(Instant::now()) } @@ -1353,15 +1398,15 @@ impl ChatComposer { let mut needed: HashMap = HashMap::new(); for img in &self.attached_images { needed - .entry(img.placeholder.clone()) - .or_insert_with(|| text_after.matches(&img.placeholder).count()); + .entry(img.display_placeholder.clone()) + .or_insert_with(|| text_after.matches(&img.display_placeholder).count()); } let mut used: HashMap = HashMap::new(); let mut kept: Vec = 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); + let total_needed = *needed.get(&img.display_placeholder).unwrap_or(&0); + let used_count = used.entry(img.display_placeholder.clone()).or_insert(0); if *used_count < total_needed { kept.push(img); *used_count += 1; @@ -1385,7 +1430,7 @@ impl ChatComposer { // 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; + let ph = &img.display_placeholder; if p < ph.len() { continue; } @@ -1415,7 +1460,7 @@ impl ChatComposer { .attached_images .iter() .enumerate() - .filter(|(_, img2)| img2.placeholder == *ph) + .filter(|(_, img2)| img2.display_placeholder == *ph) .nth(occ_before) { Some((remove_idx, ph.clone())) @@ -1434,7 +1479,7 @@ impl ChatComposer { // let result = 'out: { let out: Option<(usize, String)> = 'out: { for (i, img) in self.attached_images.iter().enumerate() { - let ph = &img.placeholder; + let ph = &img.display_placeholder; if p + ph.len() > text.len() { continue; } @@ -1462,7 +1507,7 @@ impl ChatComposer { .attached_images .iter() .enumerate() - .filter(|(_, img2)| img2.placeholder == *ph) + .filter(|(_, img2)| img2.display_placeholder == *ph) .nth(occ_before) { break 'out Some((remove_idx, ph.clone())); @@ -3210,7 +3255,7 @@ mod tests { ); let path = PathBuf::from("/tmp/image3.png"); composer.attach_image(path.clone(), 20, 10, "PNG"); - let placeholder = composer.attached_images[0].placeholder.clone(); + let placeholder = composer.attached_images[0].display_placeholder.clone(); // Case 1: backspace at end composer.textarea.move_cursor_to_end_of_line(false); @@ -3221,7 +3266,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"); - let placeholder2 = composer.attached_images[0].placeholder.clone(); + let placeholder2 = composer.attached_images[0].display_placeholder.clone(); // Move cursor to roughly middle of placeholder if let Some(start_pos) = composer.textarea.text().find(&placeholder2) { let mid_pos = start_pos + (placeholder2.len() / 2); @@ -3293,8 +3338,8 @@ mod tests { composer.handle_paste(" ".into()); composer.attach_image(path2.clone(), 10, 5, "PNG"); - let placeholder1 = composer.attached_images[0].placeholder.clone(); - let placeholder2 = composer.attached_images[1].placeholder.clone(); + let placeholder1 = composer.attached_images[0].display_placeholder.clone(); + let placeholder2 = composer.attached_images[1].display_placeholder.clone(); let text = composer.textarea.text().to_string(); let start1 = text.find(&placeholder1).expect("first placeholder present"); let end1 = start1 + placeholder1.len(); @@ -3317,7 +3362,8 @@ mod tests { assert_eq!( vec![AttachedImage { path: path2, - placeholder: "[/tmp/image_dup2.png 10x5]".to_string() + display_placeholder: "[/tmp/image_dup2.png 10x5]".to_string(), + model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string() }], composer.attached_images, "one image mapping remains" @@ -3344,7 +3390,8 @@ mod tests { let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string()); assert!(needs_redraw); - let expected_prefix = format!("[{} 3x2] ", tmp_path.display()); + let display_label = ChatComposer::display_label_for_image_placeholder(&tmp_path); + let expected_prefix = format!("[{display_label} 3x2] "); assert!(composer.textarea.text().starts_with(&expected_prefix)); let imgs = composer.take_recent_submission_images(); diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index 2ebd0715e7..98fab17ade 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -536,6 +536,14 @@ impl BottomPane { self.composer.take_recent_submission_images() } + pub(crate) fn expand_attached_image_placeholders_for_model( + &self, + display_text: &str, + ) -> String { + self.composer + .expand_attached_image_placeholders_for_model(display_text) + } + fn as_renderable(&'_ self) -> RenderableItem<'_> { if let Some(view) = self.active_view() { RenderableItem::Borrowed(view) diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index d92cf60214..1cab44db54 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -337,14 +337,16 @@ pub(crate) struct ChatWidget { } struct UserMessage { - text: String, + display_text: String, + model_text: String, image_paths: Vec, } impl From for UserMessage { fn from(text: String) -> Self { Self { - text, + display_text: text.clone(), + model_text: text, image_paths: Vec::new(), } } @@ -353,7 +355,8 @@ impl From for UserMessage { impl From<&str> for UserMessage { fn from(text: &str) -> Self { Self { - text: text.to_string(), + display_text: text.to_string(), + model_text: text.to_string(), image_paths: Vec::new(), } } @@ -363,7 +366,11 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio if text.is_empty() && image_paths.is_empty() { None } else { - Some(UserMessage { text, image_paths }) + Some(UserMessage { + display_text: text.clone(), + model_text: text, + image_paths, + }) } } @@ -778,7 +785,7 @@ impl ChatWidget { let queued_text = self .queued_user_messages .iter() - .map(|m| m.text.clone()) + .map(|m| m.display_text.clone()) .collect::>() .join("\n"); let existing_text = self.bottom_pane.composer_text(); @@ -1482,7 +1489,8 @@ impl ChatWidget { } if !self.queued_user_messages.is_empty() => { // Prefer the most recently queued item. if let Some(user_message) = self.queued_user_messages.pop_back() { - self.bottom_pane.set_composer_text(user_message.text); + self.bottom_pane + .set_composer_text(user_message.display_text); self.refresh_queued_user_messages(); self.request_redraw(); } @@ -1491,8 +1499,12 @@ impl ChatWidget { match self.bottom_pane.handle_key_event(key_event) { InputResult::Submitted(text) => { // If a task is running, queue the user input to be sent after the turn completes. + let model_text = self + .bottom_pane + .expand_attached_image_placeholders_for_model(&text); let user_message = UserMessage { - text, + display_text: text, + model_text, image_paths: self.bottom_pane.take_recent_submission_images(), }; self.queue_user_message(user_message); @@ -1717,15 +1729,19 @@ impl ChatWidget { } fn submit_user_message(&mut self, user_message: UserMessage) { - let UserMessage { text, image_paths } = user_message; - if text.is_empty() && image_paths.is_empty() { + let UserMessage { + display_text, + model_text, + image_paths, + } = user_message; + if display_text.is_empty() && image_paths.is_empty() { return; } let mut items: Vec = Vec::new(); // Special-case: "!cmd" executes a local shell command instead of sending to the model. - if let Some(stripped) = text.strip_prefix('!') { + if let Some(stripped) = display_text.strip_prefix('!') { let cmd = stripped.trim(); if cmd.is_empty() { self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( @@ -1742,8 +1758,10 @@ impl ChatWidget { return; } - if !text.is_empty() { - items.push(UserInput::Text { text: text.clone() }); + if !model_text.is_empty() { + items.push(UserInput::Text { + text: model_text.clone(), + }); } for path in image_paths { @@ -1751,7 +1769,7 @@ impl ChatWidget { } if let Some(skills) = self.bottom_pane.skills() { - let skill_mentions = find_skill_mentions(&text, skills); + let skill_mentions = find_skill_mentions(&display_text, skills); for skill in skill_mentions { items.push(UserInput::Skill { name: skill.name.clone(), @@ -1767,17 +1785,19 @@ impl ChatWidget { }); // Persist the text to cross-session message history. - if !text.is_empty() { + if !display_text.is_empty() { self.codex_op_tx - .send(Op::AddToHistory { text: text.clone() }) + .send(Op::AddToHistory { + text: display_text.clone(), + }) .unwrap_or_else(|e| { tracing::error!("failed to send AddHistory op: {e}"); }); } // Only show the text portion in conversation history. - if !text.is_empty() { - self.add_to_history(history_cell::new_user_prompt(text)); + if !display_text.is_empty() { + self.add_to_history(history_cell::new_user_prompt(display_text)); } self.needs_final_message_separator = false; } @@ -2034,7 +2054,7 @@ impl ChatWidget { let messages: Vec = self .queued_user_messages .iter() - .map(|m| m.text.clone()) + .map(|m| m.display_text.clone()) .collect(); self.bottom_pane.set_queued_user_messages(messages); } diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index f904fc25ca..4fde367287 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -970,7 +970,7 @@ async fn alt_up_edits_most_recent_queued_message() { // And the queue should now contain only the remaining (older) item. assert_eq!(chat.queued_user_messages.len(), 1); assert_eq!( - chat.queued_user_messages.front().unwrap().text, + chat.queued_user_messages.front().unwrap().display_text, "first queued" ); } @@ -1000,7 +1000,7 @@ async fn enqueueing_history_prompt_multiple_times_is_stable() { assert_eq!(chat.queued_user_messages.len(), 3); for message in chat.queued_user_messages.iter() { - assert_eq!(message.text, "repeat me"); + assert_eq!(message.display_text, "repeat me"); } } @@ -1021,7 +1021,7 @@ async fn streaming_final_answer_keeps_task_running_state() { assert_eq!(chat.queued_user_messages.len(), 1); assert_eq!( - chat.queued_user_messages.front().unwrap().text, + chat.queued_user_messages.front().unwrap().display_text, "queued submission" ); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); @@ -1080,6 +1080,67 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { ); } +#[tokio::test] +async fn submitting_image_shows_short_path_but_sends_full_path_to_model() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + // Use a long absolute path so the display placeholder is shortened. + let image_path = PathBuf::from("/var/tmp/some/dir/image.png"); + + chat.bottom_pane.insert_str("see "); + chat.bottom_pane + .attach_image(image_path.clone(), 24, 42, "png"); + + let display_text = chat.bottom_pane.composer_text(); + assert!( + display_text.contains("[/var/.../image.png 24x42]"), + "expected shortened display placeholder, got {display_text:?}" + ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + // The user-visible transcript should keep the short placeholder. + let cells = drain_insert_history(&mut rx); + assert!( + !cells.is_empty(), + "expected a user history cell to be inserted" + ); + let rendered = lines_to_single_string(&cells[0]); + assert!( + rendered.contains("/var/.../image.png"), + "expected transcript to contain shortened placeholder, got {rendered:?}" + ); + assert!( + !rendered.contains("/var/tmp/some/dir/image.png"), + "expected transcript not to contain full path, got {rendered:?}" + ); + + // The model-facing text should expand the placeholder to the full path. + let mut model_text: Option = None; + while let Ok(op) = op_rx.try_recv() { + if let Op::UserInput { items } = op { + for item in items { + if let codex_protocol::user_input::UserInput::Text { text } = item { + model_text = Some(text); + break; + } + } + } + if model_text.is_some() { + break; + } + } + let model_text = model_text.expect("expected Op::UserInput with a Text item"); + assert!( + model_text.contains("/var/tmp/some/dir/image.png"), + "expected model text to contain full path, got {model_text:?}" + ); + assert!( + !model_text.contains("/var/.../image.png"), + "expected model text not to contain shortened placeholder, got {model_text:?}" + ); +} + #[tokio::test] async fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;