diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index d6d01045b4b..265eef4961f 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,17 +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) { - 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); + 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 { @@ -414,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()) } @@ -480,6 +506,35 @@ impl ChatComposer { } } + fn display_label_for_image_placeholder(path: &Path) -> String { + // Keep the UI placeholder compact: show only the basename (pre-existing behavior). + // The model can still receive the full path via model_placeholder expansion. + path.file_name() + .and_then(|name| name.to_str()) + .map(str::to_string) + .unwrap_or_else(|| "image".to_string()) + } + + 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(); @@ -1438,15 +1493,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; @@ -1470,7 +1525,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; } @@ -1500,7 +1555,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())) @@ -1519,7 +1574,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; } @@ -1547,7 +1602,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())); @@ -3296,11 +3351,11 @@ mod tests { assert!(text.contains("[image_dup.png 10x5]")); assert!(text.contains("[image_dup.png 10x5 #2]")); assert_eq!( - composer.attached_images[0].placeholder, + composer.attached_images[0].display_placeholder, "[image_dup.png 10x5]" ); assert_eq!( - composer.attached_images[1].placeholder, + composer.attached_images[1].display_placeholder, "[image_dup.png 10x5 #2]" ); } @@ -3318,7 +3373,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); @@ -3329,7 +3384,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); @@ -3397,8 +3452,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(); @@ -3421,7 +3476,8 @@ mod tests { assert_eq!( vec![AttachedImage { path: path2, - placeholder: "[image_dup2.png 10x5]".to_string() + display_placeholder: "[image_dup2.png 10x5]".to_string(), + model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string() }], composer.attached_images, "one image mapping remains" @@ -3448,12 +3504,9 @@ 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] ") - ); + 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(); assert_eq!(imgs, vec![tmp_path]); @@ -4170,7 +4223,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 @@ -4185,7 +4239,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()); } @@ -4204,7 +4258,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"), }); @@ -4254,7 +4309,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 7634f699aa0..e9a15dda51b 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 f4418ceadb2..a25dd1187d9 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 a0ff8d42e9d..13a0cfd7034 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 we can verify display vs model text differ. + let image_path = "/var/tmp/some/dir/image.png"; + + chat.bottom_pane.insert_str("see "); + chat.bottom_pane + .attach_image(PathBuf::from(image_path), 24, 42, "png"); + + let display_text = chat.bottom_pane.composer_text(); + assert!( + display_text.contains("[image.png 24x42]"), + "expected basename 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("image.png 24x42"), + "expected transcript to contain basename placeholder, got {rendered:?}" + ); + assert!( + !rendered.contains(image_path), + "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(image_path), + "expected model text to contain full path, got {model_text:?}" + ); + assert!( + !model_text.contains("[image.png 24x42]"), + "expected model text not to contain display 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 0073173fdc7..5ba79ac8c72 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,15 @@ impl ChatComposer { self.skills = skills; } + fn display_label_for_image_placeholder(path: &Path) -> String { + // Keep the UI placeholder compact: show only the basename (pre-existing behavior). + // The model can still receive the full path via model_placeholder expansion. + path.file_name() + .and_then(|name| name.to_str()) + .map(str::to_string) + .unwrap_or_else(|| "image".to_string()) + } + fn layout_areas(&self, area: Rect) -> [Rect; 3] { let footer_props = self.footer_props(); let footer_hint_height = self @@ -330,16 +340,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) { - 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}]"); + 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 { @@ -347,6 +359,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()) } @@ -1355,15 +1377,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; @@ -1387,7 +1409,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; } @@ -1417,7 +1439,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())) @@ -1436,7 +1458,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; } @@ -1464,7 +1486,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())); @@ -3212,7 +3234,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); @@ -3223,7 +3245,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); @@ -3291,8 +3313,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(); @@ -3315,7 +3337,8 @@ mod tests { assert_eq!( vec![AttachedImage { path: path2, - placeholder: "[image_dup2.png 10x5]".to_string() + display_placeholder: "[image_dup2.png 10x5]".to_string(), + model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string() }], composer.attached_images, "one image mapping remains" @@ -3342,12 +3365,9 @@ 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] ") - ); + 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(); assert_eq!(imgs, vec![tmp_path]); diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index 2ebd0715e7d..98fab17ade4 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 d92cf602140..5178ec16503 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,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 { @@ -1751,7 +1767,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 +1783,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 +2052,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 8b216812dfd..da4c1a0d89d 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 we can verify display vs model text differ. + let image_path = "/var/tmp/some/dir/image.png"; + + chat.bottom_pane.insert_str("see "); + chat.bottom_pane + .attach_image(PathBuf::from(image_path), 24, 42, "png"); + + let display_text = chat.bottom_pane.composer_text(); + assert!( + display_text.contains("[image.png 24x42]"), + "expected basename 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("image.png 24x42"), + "expected transcript to contain basename placeholder, got {rendered:?}" + ); + assert!( + !rendered.contains(image_path), + "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(image_path), + "expected model text to contain full path, got {model_text:?}" + ); + assert!( + !model_text.contains("[image.png 24x42]"), + "expected model text not to contain display 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;