clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-02-01 22:47:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restore image attachments/text elements when recalling input history (Up/Down) (#9628)

Browse Source 
**Summary**
- Up/Down input history now restores image attachments and text elements
for local entries.
- Composer history stores rich local entries (text + text elements +
local image paths) while persistent history remains text-only.
- Added tests to verify history recall rehydrates image placeholders and
attachments in both `tui` and `tui2`.

**Changes**
- `tui/src/bottom_pane/chat_composer_history.rs`: store `HistoryEntry`
(text + elements + image paths) for local history; adapt navigation +
tests.
- `tui2/src/bottom_pane/chat_composer_history.rs`: same as above.
- `tui/src/bottom_pane/chat_composer.rs`: record rich history entries
and restore them on Up/Down; update Ctrl+C history and tests.
- `tui2/src/bottom_pane/chat_composer.rs`: same as above.
This commit is contained in:
Charley Cunningham
2026-01-27 18:39:59 -08:00
committed by GitHub
parent 1609f6aa81
commit add648df82
4 changed files with 171 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
codex-rs/tui/src/bottom_pane/chat_composer.rs
View File

@@ -15,6 +15,16 @@
//! [`ChatComposer::handle_key_event_without_popup`]. After every handled key, we call
//! [`ChatComposer::sync_popups`] so UI state follows the latest buffer/cursor.
//!
//! # History Navigation (↑/↓)
//!
//! The Up/Down history path is managed by [`ChatComposerHistory`]. It merges:
//!
//! - Persistent cross-session history (text-only; no element ranges or attachments).
//! - Local in-session history (full text + text elements + local image paths).
//!
//! When recalling a local entry, the composer rehydrates text elements and image attachments.
//! When recalling a persistent entry, only the text is restored.
//!
//! # Submission and Prompt Expansion
//!
//! On submit/queue paths, the composer:
@@ -89,6 +99,7 @@ use ratatui::widgets::StatefulWidgetRef;
use ratatui::widgets::WidgetRef;
use super::chat_composer_history::ChatComposerHistory;
use super::chat_composer_history::HistoryEntry;
use super::command_popup::CommandItem;
use super::command_popup::CommandPopup;
use super::command_popup::CommandPopupFlags;
@@ -460,11 +471,12 @@ impl ChatComposer {
offset: usize,
entry: Option<String>,
) -> bool {
let Some(text) = self.history.on_entry_response(log_id, offset, entry) else {
let Some(entry) = self.history.on_entry_response(log_id, offset, entry) else {
return false;
};
// Composer history (↑/↓) stores plain text only; no UI element ranges/attachments to restore here.
self.set_text_content(text, Vec::new(), Vec::new());
// Persistent ↑/↓ history is text-only (backwards-compatible and avoids persisting
// attachments), but local in-session ↑/↓ history can rehydrate elements and image paths.
self.set_text_content(entry.text, entry.text_elements, entry.local_image_paths);
true
}
@@ -707,9 +719,19 @@ impl ChatComposer {
return None;
}
let previous = self.current_text();
let text_elements = self.textarea.text_elements();
let local_image_paths = self
.attached_images
.iter()
.map(|img| img.path.clone())
.collect();
self.set_text_content(String::new(), Vec::new(), Vec::new());
self.history.reset_navigation();
self.history.record_local_submission(&previous);
self.history.record_local_submission(HistoryEntry {
text: previous.clone(),
text_elements,
local_image_paths,
});
Some(previous)
}
@@ -1792,8 +1814,17 @@ impl ChatComposer {
if text.is_empty() && self.attached_images.is_empty() {
return None;
}
if !text.is_empty() {
self.history.record_local_submission(&text);
if !text.is_empty() || !self.attached_images.is_empty() {
let local_image_paths = self
.attached_images
.iter()
.map(|img| img.path.clone())
.collect();
self.history.record_local_submission(HistoryEntry {
text: text.clone(),
text_elements: text_elements.clone(),
local_image_paths,
});
}
self.pending_pastes.clear();
Some((text, text_elements))
@@ -1989,15 +2020,19 @@ impl ChatComposer {
.history
.should_handle_navigation(self.textarea.text(), self.textarea.cursor())
{
let replace_text = match key_event.code {
let replace_entry = match key_event.code {
KeyCode::Up => self.history.navigate_up(&self.app_event_tx),
KeyCode::Down => self.history.navigate_down(&self.app_event_tx),
KeyCode::Char('p') => self.history.navigate_up(&self.app_event_tx),
KeyCode::Char('n') => self.history.navigate_down(&self.app_event_tx),
_ => unreachable!(),
};
if let Some(text) = replace_text {
self.set_text_content(text, Vec::new(), Vec::new());
if let Some(entry) = replace_entry {
self.set_text_content(
entry.text,
entry.text_elements,
entry.local_image_paths,
);
return (InputResult::None, true);
}
}
@@ -3268,7 +3303,7 @@ mod tests {
assert_eq!(
composer.history.navigate_up(&composer.app_event_tx),
Some("draft text".to_string())
Some(HistoryEntry::from_text("draft text".to_string()))
);
}
@@ -4721,6 +4756,37 @@ mod tests {
assert_eq!(vec![path], imgs);
}
#[test]
fn history_navigation_restores_image_attachments() {
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,
);
composer.set_steer_enabled(true);
let path = PathBuf::from("/tmp/image1.png");
composer.attach_image(path.clone());
let (result, _needs_redraw) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
assert!(matches!(result, InputResult::Submitted { .. }));
composer.set_text_content(String::new(), Vec::new(), Vec::new());
let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
let text = composer.current_text();
assert_eq!(text, "[Image #1]");
let text_elements = composer.text_elements();
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(composer.local_image_paths(), vec![path]);
}
#[test]
fn set_text_content_reattaches_images_without_placeholder_metadata() {
let (tx, _rx) = unbounded_channel::<AppEvent>();

114
codex-rs/tui/src/bottom_pane/chat_composer_history.rs
View File

@@ -1,8 +1,35 @@
use std::collections::HashMap;
use std::path::PathBuf;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use codex_core::protocol::Op;
use codex_protocol::user_input::TextElement;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct HistoryEntry {
pub(crate) text: String,
pub(crate) text_elements: Vec<TextElement>,
pub(crate) local_image_paths: Vec<PathBuf>,
}
impl HistoryEntry {
fn empty() -> Self {
Self {
text: String::new(),
text_elements: Vec::new(),
local_image_paths: Vec::new(),
}
}
pub(crate) fn from_text(text: String) -> Self {
Self {
text,
text_elements: Vec::new(),
local_image_paths: Vec::new(),
}
}
}
/// State machine that manages shell-style history navigation (Up/Down) inside
/// the chat composer. This struct is intentionally decoupled from the
@@ -15,10 +42,10 @@ pub(crate) struct ChatComposerHistory {
history_entry_count: usize,
/// Messages submitted by the user *during this UI session* (newest at END).
local_history: Vec<String>,
local_history: Vec<HistoryEntry>,
/// Cache of persistent history entries fetched on-demand.
fetched_history: HashMap<usize, String>,
fetched_history: HashMap<usize, HistoryEntry>,
/// Current cursor within the combined (persistent + local) history. `None`
/// indicates the user is *not* currently browsing history.
@@ -54,8 +81,8 @@ impl ChatComposerHistory {
/// Record a message submitted by the user in the current session so it can
/// be recalled later.
pub fn record_local_submission(&mut self, text: &str) {
if text.is_empty() {
pub fn record_local_submission(&mut self, entry: HistoryEntry) {
if entry.text.is_empty() && entry.local_image_paths.is_empty() {
return;
}
@@ -63,11 +90,11 @@ impl ChatComposerHistory {
self.last_history_text = None;
// Avoid inserting a duplicate if identical to the previous entry.
if self.local_history.last().is_some_and(|prev| prev == text) {
if self.local_history.last().is_some_and(|prev| prev == &entry) {
return;
}
self.local_history.push(text.to_string());
self.local_history.push(entry);
}
/// Reset navigation tracking so the next Up key resumes from the latest entry.
@@ -99,7 +126,7 @@ impl ChatComposerHistory {
/// Handle <Up>. Returns true when the key was consumed and the caller
/// should request a redraw.
pub fn navigate_up(&mut self, app_event_tx: &AppEventSender) -> Option<String> {
pub fn navigate_up(&mut self, app_event_tx: &AppEventSender) -> Option<HistoryEntry> {
let total_entries = self.history_entry_count + self.local_history.len();
if total_entries == 0 {
return None;
@@ -116,7 +143,7 @@ impl ChatComposerHistory {
}
/// Handle <Down>.
pub fn navigate_down(&mut self, app_event_tx: &AppEventSender) -> Option<String> {
pub fn navigate_down(&mut self, app_event_tx: &AppEventSender) -> Option<HistoryEntry> {
let total_entries = self.history_entry_count + self.local_history.len();
if total_entries == 0 {
return None;
@@ -137,7 +164,7 @@ impl ChatComposerHistory {
// Past newest – clear and exit browsing mode.
self.history_cursor = None;
self.last_history_text = None;
Some(String::new())
Some(HistoryEntry::empty())
}
}
}
@@ -148,16 +175,17 @@ impl ChatComposerHistory {
log_id: u64,
offset: usize,
entry: Option<String>,
) -> Option<String> {
) -> Option<HistoryEntry> {
if self.history_log_id != Some(log_id) {
return None;
}
let text = entry?;
self.fetched_history.insert(offset, text.clone());
let entry = HistoryEntry::from_text(text);
self.fetched_history.insert(offset, entry.clone());
if self.history_cursor == Some(offset as isize) {
self.last_history_text = Some(text.clone());
return Some(text);
self.last_history_text = Some(entry.text.clone());
return Some(entry);
}
None
}
@@ -170,19 +198,20 @@ impl ChatComposerHistory {
&mut self,
global_idx: usize,
app_event_tx: &AppEventSender,
) -> Option<String> {
) -> Option<HistoryEntry> {
if global_idx >= self.history_entry_count {
// Local entry.
if let Some(text) = self
if let Some(entry) = self
.local_history
.get(global_idx - self.history_entry_count)
.cloned()
{
self.last_history_text = Some(text.clone());
return Some(text.clone());
self.last_history_text = Some(entry.text.clone());
return Some(entry);
}
} else if let Some(text) = self.fetched_history.get(&global_idx) {
self.last_history_text = Some(text.clone());
return Some(text.clone());
} else if let Some(entry) = self.fetched_history.get(&global_idx).cloned() {
self.last_history_text = Some(entry.text.clone());
return Some(entry);
} else if let Some(log_id) = self.history_log_id {
let op = Op::GetHistoryEntryRequest {
offset: global_idx,
@@ -206,22 +235,28 @@ mod tests {
let mut history = ChatComposerHistory::new();
// Empty submissions are ignored.
history.record_local_submission("");
history.record_local_submission(HistoryEntry::from_text(String::new()));
assert_eq!(history.local_history.len(), 0);
// First entry is recorded.
history.record_local_submission("hello");
history.record_local_submission(HistoryEntry::from_text("hello".to_string()));
assert_eq!(history.local_history.len(), 1);
assert_eq!(history.local_history.last().unwrap(), "hello");
assert_eq!(
history.local_history.last().unwrap(),
&HistoryEntry::from_text("hello".to_string())
);
// Identical consecutive entry is skipped.
history.record_local_submission("hello");
history.record_local_submission(HistoryEntry::from_text("hello".to_string()));
assert_eq!(history.local_history.len(), 1);
// Different entry is recorded.
history.record_local_submission("world");
history.record_local_submission(HistoryEntry::from_text("world".to_string()));
assert_eq!(history.local_history.len(), 2);
assert_eq!(history.local_history.last().unwrap(), "world");
assert_eq!(
history.local_history.last().unwrap(),
&HistoryEntry::from_text("world".to_string())
);
}
#[test]
@@ -252,7 +287,7 @@ mod tests {
// Inject the async response.
assert_eq!(
Some("latest".into()),
Some(HistoryEntry::from_text("latest".to_string())),
history.on_entry_response(1, 2, Some("latest".into()))
);
@@ -273,7 +308,7 @@ mod tests {
);
assert_eq!(
Some("older".into()),
Some(HistoryEntry::from_text("older".to_string())),
history.on_entry_response(1, 1, Some("older".into()))
);
}
@@ -285,16 +320,29 @@ mod tests {
let mut history = ChatComposerHistory::new();
history.set_metadata(1, 3);
history.fetched_history.insert(1, "command2".into());
history.fetched_history.insert(2, "command3".into());
history
.fetched_history
.insert(1, HistoryEntry::from_text("command2".to_string()));
history
.fetched_history
.insert(2, HistoryEntry::from_text("command3".to_string()));
assert_eq!(Some("command3".into()), history.navigate_up(&tx));
assert_eq!(Some("command2".into()), history.navigate_up(&tx));
assert_eq!(
Some(HistoryEntry::from_text("command3".to_string())),
history.navigate_up(&tx)
);
assert_eq!(
Some(HistoryEntry::from_text("command2".to_string())),
history.navigate_up(&tx)
);
history.reset_navigation();
assert!(history.history_cursor.is_none());
assert!(history.last_history_text.is_none());
assert_eq!(Some("command3".into()), history.navigate_up(&tx));
assert_eq!(
Some(HistoryEntry::from_text("command3".to_string())),
history.navigate_up(&tx)
);
}
}

5
codex-rs/tui/src/chatwidget/tests.rs
View File

@@ -1749,10 +1749,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
let images = chat.bottom_pane.take_recent_submission_images();
assert!(
images.is_empty(),
"attachments are not preserved in history recall"
);
assert_eq!(vec![PathBuf::from("/tmp/preview.png")], images);
}
#[tokio::test]

13
docs/tui-chat-composer.md
View File

@@ -49,6 +49,19 @@ The solution is to detect paste-like _bursts_ and buffer them into a single expl
- After handling the key, `sync_popups()` runs so popup visibility/filters stay consistent with the
latest text + cursor.
### History navigation (↑/↓)
Up/Down recall is handled by `ChatComposerHistory` and merges two sources:
- **Persistent history** (cross-session, fetched from `~/.codex/history.jsonl`): text-only. It
does **not** carry text element ranges or local image attachments, so recalling one of these
entries only restores the text.
- **Local history** (current session): stores the full submission payload, including text
elements and local image paths. Recalling a local entry rehydrates placeholders and attachments.
This distinction keeps the on-disk history backward compatible and avoids persisting attachments,
while still providing a richer recall experience for in-session edits.
## Config gating for reuse
`ChatComposer` now supports feature gating via `ChatComposerConfig`