mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Seed prompt history from resumed messages (#24298)
## Why When the TUI resumes a thread, transcript replay renders prior user messages but did not seed the composer history. That leaves the resumed session with empty in-memory prompt history, so pressing Up can fall through to persisted global history and surface a prompt from another thread. The expected behavior is that prompts from the resumed thread are recalled first, with global history only as a fallback. ## What changed - Record replayed user messages into the composer history during resume replay. - Preserve the existing persisted history format and avoid any startup history scan. - Add focused TUI coverage showing replayed prompts are recalled before persisted global history. ## Validation - Added `replayed_user_messages_seed_composer_history` in `codex-rs/tui/src/chatwidget/tests/history_replay.rs`. - `just test -p codex-tui replayed_user_messages_seed_composer_history` passed.
This commit is contained in:
committed by
GitHub
Unverified
parent
f0a839ea0c
commit
56958f2512
@@ -5,7 +5,11 @@ pub fn connector_display_label(connector: &AppInfo) -> String {
|
||||
}
|
||||
|
||||
pub fn connector_mention_slug(connector: &AppInfo) -> String {
|
||||
crate::connector_name_slug(&connector_display_label(connector))
|
||||
connector_mention_slug_from_name(&connector_display_label(connector))
|
||||
}
|
||||
|
||||
pub fn connector_mention_slug_from_name(name: &str) -> String {
|
||||
crate::connector_name_slug(name)
|
||||
}
|
||||
|
||||
pub fn connector_install_url(name: &str, connector_id: &str) -> String {
|
||||
|
||||
@@ -858,6 +858,10 @@ impl ChatComposer {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) {
|
||||
self.history.record_replayed_submission(entry);
|
||||
}
|
||||
|
||||
/// Integrate pasted text into the composer.
|
||||
///
|
||||
/// Acts as the only place where paste text is integrated, both for:
|
||||
|
||||
@@ -121,6 +121,8 @@ pub(crate) struct ChatComposerHistory {
|
||||
/// Messages submitted by the user *during this UI session* (newest at END).
|
||||
/// Local entries retain full draft state (text elements, image paths, pending pastes, remote image URLs).
|
||||
local_history: Vec<HistoryEntry>,
|
||||
/// Local entries seeded from resumed transcript replay.
|
||||
replay_seeded_history: Vec<HistoryEntry>,
|
||||
|
||||
/// Cache of persistent history entries fetched on-demand (text-only).
|
||||
fetched_history: HashMap<usize, HistoryEntry>,
|
||||
@@ -128,6 +130,7 @@ pub(crate) struct ChatComposerHistory {
|
||||
/// Current cursor within the combined (persistent + local) history. `None`
|
||||
/// indicates the user is *not* currently browsing history.
|
||||
history_cursor: Option<isize>,
|
||||
pending_navigation_direction: Option<HistorySearchDirection>,
|
||||
|
||||
/// The text that was last inserted into the composer as a result of
|
||||
/// history navigation. Used to decide if further Up/Down presses should be
|
||||
@@ -229,8 +232,10 @@ impl ChatComposerHistory {
|
||||
persistent_log_id: None,
|
||||
persistent_entry_count: 0,
|
||||
local_history: Vec::new(),
|
||||
replay_seeded_history: Vec::new(),
|
||||
fetched_history: HashMap::new(),
|
||||
history_cursor: None,
|
||||
pending_navigation_direction: None,
|
||||
last_history_text: None,
|
||||
search: None,
|
||||
at_mention_restore_enabled: false,
|
||||
@@ -259,7 +264,9 @@ impl ChatComposerHistory {
|
||||
self.persistent_entry_count = entry_count;
|
||||
self.fetched_history.clear();
|
||||
self.local_history.clear();
|
||||
self.replay_seeded_history.clear();
|
||||
self.history_cursor = None;
|
||||
self.pending_navigation_direction = None;
|
||||
self.last_history_text = None;
|
||||
self.search = None;
|
||||
}
|
||||
@@ -269,6 +276,16 @@ impl ChatComposerHistory {
|
||||
/// Empty submissions are ignored, adjacent duplicates are collapsed, and active navigation or
|
||||
/// search state is reset because a new newest entry changes the combined history offset space.
|
||||
pub fn record_local_submission(&mut self, entry: HistoryEntry) {
|
||||
self.record_local_submission_inner(entry);
|
||||
}
|
||||
|
||||
pub fn record_replayed_submission(&mut self, entry: HistoryEntry) {
|
||||
if self.record_local_submission_inner(entry.clone()) {
|
||||
self.replay_seeded_history.push(entry);
|
||||
}
|
||||
}
|
||||
|
||||
fn record_local_submission_inner(&mut self, entry: HistoryEntry) -> bool {
|
||||
if entry.text.is_empty()
|
||||
&& entry.text_elements.is_empty()
|
||||
&& entry.local_image_paths.is_empty()
|
||||
@@ -276,18 +293,20 @@ impl ChatComposerHistory {
|
||||
&& entry.mention_bindings.is_empty()
|
||||
&& entry.pending_pastes.is_empty()
|
||||
{
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
self.history_cursor = None;
|
||||
self.pending_navigation_direction = None;
|
||||
self.last_history_text = None;
|
||||
self.search = None;
|
||||
|
||||
// Avoid inserting a duplicate if identical to the previous entry.
|
||||
if self.local_history.last().is_some_and(|prev| prev == &entry) {
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
|
||||
self.local_history.push(entry);
|
||||
true
|
||||
}
|
||||
|
||||
/// Resets normal history navigation so the next Up key resumes from the newest entry.
|
||||
@@ -297,6 +316,7 @@ impl ChatComposerHistory {
|
||||
/// influence later Up/Down recall.
|
||||
pub fn reset_navigation(&mut self) {
|
||||
self.history_cursor = None;
|
||||
self.pending_navigation_direction = None;
|
||||
self.last_history_text = None;
|
||||
self.search = None;
|
||||
}
|
||||
@@ -359,7 +379,11 @@ impl ChatComposerHistory {
|
||||
};
|
||||
|
||||
self.history_cursor = Some(next_idx);
|
||||
self.populate_history_at_index(next_idx as usize, app_event_tx)
|
||||
self.populate_history_at_index(
|
||||
next_idx as usize,
|
||||
HistorySearchDirection::Older,
|
||||
app_event_tx,
|
||||
)
|
||||
}
|
||||
|
||||
/// Handles Down by moving toward newer entries or clearing the composer past the newest entry.
|
||||
@@ -383,11 +407,16 @@ impl ChatComposerHistory {
|
||||
match next_idx_opt {
|
||||
Some(idx) => {
|
||||
self.history_cursor = Some(idx);
|
||||
self.populate_history_at_index(idx as usize, app_event_tx)
|
||||
self.populate_history_at_index(
|
||||
idx as usize,
|
||||
HistorySearchDirection::Newer,
|
||||
app_event_tx,
|
||||
)
|
||||
}
|
||||
None => {
|
||||
// Past newest – clear and exit browsing mode.
|
||||
self.history_cursor = None;
|
||||
self.pending_navigation_direction = None;
|
||||
self.last_history_text = None;
|
||||
Some(HistoryEntry::new(String::new()))
|
||||
}
|
||||
@@ -449,9 +478,22 @@ impl ChatComposerHistory {
|
||||
}
|
||||
|
||||
if self.history_cursor == Some(offset as isize) {
|
||||
let direction = self.pending_navigation_direction.take();
|
||||
let Some(entry) = entry else {
|
||||
return HistoryEntryResponse::Ignored;
|
||||
};
|
||||
if self.persistent_entry_duplicates_local(&entry)
|
||||
&& let Some(direction) = direction
|
||||
{
|
||||
let Some(offset) = self.next_history_offset(offset, direction) else {
|
||||
return HistoryEntryResponse::Ignored;
|
||||
};
|
||||
self.history_cursor = Some(offset as isize);
|
||||
return self
|
||||
.populate_history_at_index(offset, direction, app_event_tx)
|
||||
.map(HistoryEntryResponse::Found)
|
||||
.unwrap_or(HistoryEntryResponse::Ignored);
|
||||
}
|
||||
self.last_history_text = Some(entry.text.clone());
|
||||
return HistoryEntryResponse::Found(entry);
|
||||
}
|
||||
@@ -729,29 +771,61 @@ impl ChatComposerHistory {
|
||||
fn populate_history_at_index(
|
||||
&mut self,
|
||||
global_idx: usize,
|
||||
direction: HistorySearchDirection,
|
||||
app_event_tx: &AppEventSender,
|
||||
) -> Option<HistoryEntry> {
|
||||
if global_idx >= self.persistent_entry_count {
|
||||
// Local entry.
|
||||
if let Some(entry) = self
|
||||
.local_history
|
||||
.get(global_idx - self.persistent_entry_count)
|
||||
.cloned()
|
||||
{
|
||||
let mut global_idx = global_idx;
|
||||
loop {
|
||||
if let Some(entry) = self.entry_at_cached_offset(global_idx) {
|
||||
if global_idx < self.persistent_entry_count
|
||||
&& self.persistent_entry_duplicates_local(&entry)
|
||||
{
|
||||
let Some(next_idx) = self.next_history_offset(global_idx, direction) else {
|
||||
self.pending_navigation_direction = None;
|
||||
return None;
|
||||
};
|
||||
self.history_cursor = Some(next_idx as isize);
|
||||
global_idx = next_idx;
|
||||
continue;
|
||||
}
|
||||
self.pending_navigation_direction = None;
|
||||
self.last_history_text = Some(entry.text.clone());
|
||||
return Some(entry);
|
||||
}
|
||||
} 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(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) {
|
||||
app_event_tx.send(AppEvent::LookupMessageHistoryEntry {
|
||||
thread_id,
|
||||
offset: global_idx,
|
||||
log_id,
|
||||
});
|
||||
|
||||
if global_idx >= self.persistent_entry_count {
|
||||
return None;
|
||||
}
|
||||
|
||||
if let (Some(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) {
|
||||
self.pending_navigation_direction = Some(direction);
|
||||
app_event_tx.send(AppEvent::LookupMessageHistoryEntry {
|
||||
thread_id,
|
||||
offset: global_idx,
|
||||
log_id,
|
||||
});
|
||||
}
|
||||
return None;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
fn next_history_offset(
|
||||
&self,
|
||||
offset: usize,
|
||||
direction: HistorySearchDirection,
|
||||
) -> Option<usize> {
|
||||
match direction {
|
||||
HistorySearchDirection::Older => offset.checked_sub(1),
|
||||
HistorySearchDirection::Newer => offset
|
||||
.checked_add(1)
|
||||
.filter(|next| *next < self.total_entries()),
|
||||
}
|
||||
}
|
||||
|
||||
fn persistent_entry_duplicates_local(&self, entry: &HistoryEntry) -> bool {
|
||||
self.replay_seeded_history.iter().any(|local_entry| {
|
||||
local_entry.text == entry.text && local_entry.mention_bindings == entry.mention_bindings
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -939,9 +1013,16 @@ mod tests {
|
||||
// Pretend there are 3 persistent entries.
|
||||
let thread_id = test_thread_id();
|
||||
history.set_metadata(thread_id, /*log_id*/ 1, /*entry_count*/ 3);
|
||||
history.record_local_submission(HistoryEntry::new("latest".to_string()));
|
||||
|
||||
// First Up should request offset 2 (latest) and await async data.
|
||||
// First Up should recall current-session local history.
|
||||
assert!(history.should_handle_navigation("", /*cursor*/ 0));
|
||||
assert_eq!(
|
||||
Some(HistoryEntry::new("latest".to_string())),
|
||||
history.navigate_up(&tx)
|
||||
);
|
||||
|
||||
// Next Up should request offset 2 and await async data.
|
||||
assert!(history.navigate_up(&tx).is_none()); // don't replace the text yet
|
||||
|
||||
// Verify that a history lookup request was sent.
|
||||
|
||||
@@ -184,6 +184,7 @@ pub(crate) use chat_composer::ChatComposer;
|
||||
pub(crate) use chat_composer::ChatComposerConfig;
|
||||
pub(crate) use chat_composer::InputResult;
|
||||
pub(crate) use chat_composer::QueuedInputAction;
|
||||
pub(crate) use chat_composer_history::HistoryEntry;
|
||||
|
||||
use crate::status_indicator_widget::StatusDetailsCapitalization;
|
||||
use crate::status_indicator_widget::StatusIndicatorWidget;
|
||||
@@ -1543,6 +1544,10 @@ impl BottomPane {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) {
|
||||
self.composer.record_replayed_user_message_history(entry);
|
||||
}
|
||||
|
||||
pub(crate) fn on_file_search_result(&mut self, query: String, matches: Vec<FileMatch>) {
|
||||
self.composer.on_file_search_result(query, matches);
|
||||
self.request_redraw();
|
||||
|
||||
@@ -169,6 +169,8 @@ use codex_terminal_detection::TerminalName;
|
||||
use codex_terminal_detection::terminal_info;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use codex_utils_plugins::mention_syntax::PLUGIN_TEXT_MENTION_SIGIL;
|
||||
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
@@ -274,6 +276,7 @@ use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED;
|
||||
use crate::bottom_pane::ExperimentalFeatureItem;
|
||||
use crate::bottom_pane::ExperimentalFeaturesView;
|
||||
use crate::bottom_pane::GoalStatusIndicator;
|
||||
use crate::bottom_pane::HistoryEntry;
|
||||
use crate::bottom_pane::InputResult;
|
||||
use crate::bottom_pane::LocalImageAttachment;
|
||||
use crate::bottom_pane::McpServerElicitationFormRequest;
|
||||
@@ -1223,9 +1226,75 @@ impl ChatWidget {
|
||||
fn on_committed_user_message(&mut self, items: &[UserInput], from_replay: bool) {
|
||||
let display = Self::user_message_display_from_inputs(items);
|
||||
if from_replay {
|
||||
if !self.review.is_review_mode {
|
||||
self.on_user_message_display(display);
|
||||
if self.review.is_review_mode {
|
||||
return;
|
||||
}
|
||||
let message = display.message.as_str();
|
||||
let mention_start = |sigil: char, mention: &str| {
|
||||
let token = format!("{sigil}{mention}");
|
||||
message.match_indices(&token).find_map(|(start, _)| {
|
||||
let end = start + token.len();
|
||||
message
|
||||
.as_bytes()
|
||||
.get(end)
|
||||
.is_none_or(|byte| {
|
||||
!byte.is_ascii_alphanumeric() && !matches!(byte, b'_' | b'-')
|
||||
})
|
||||
.then_some(start)
|
||||
})
|
||||
};
|
||||
let mut mention_bindings: Vec<MentionBinding> = items
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
UserInput::Skill { name, path } => Some(MentionBinding {
|
||||
sigil: TOOL_MENTION_SIGIL,
|
||||
mention: name.clone(),
|
||||
path: path.to_string_lossy().into_owned(),
|
||||
}),
|
||||
UserInput::Mention { name, path } => {
|
||||
let plugin_id = path.strip_prefix("plugin://");
|
||||
let mention = if let Some(plugin_id) = plugin_id {
|
||||
plugin_id
|
||||
.split_once('@')
|
||||
.map(|(plugin_name, _)| plugin_name)
|
||||
.unwrap_or(plugin_id)
|
||||
.to_string()
|
||||
} else if path.starts_with("app://") {
|
||||
codex_connectors::metadata::connector_mention_slug_from_name(name)
|
||||
} else {
|
||||
name.clone()
|
||||
};
|
||||
let sigil = if plugin_id.is_some()
|
||||
&& mention_start(PLUGIN_TEXT_MENTION_SIGIL, &mention).is_some()
|
||||
{
|
||||
PLUGIN_TEXT_MENTION_SIGIL
|
||||
} else {
|
||||
TOOL_MENTION_SIGIL
|
||||
};
|
||||
Some(MentionBinding {
|
||||
sigil,
|
||||
mention,
|
||||
path: path.clone(),
|
||||
})
|
||||
}
|
||||
UserInput::Text { .. }
|
||||
| UserInput::Image { .. }
|
||||
| UserInput::LocalImage { .. } => None,
|
||||
})
|
||||
.collect();
|
||||
mention_bindings.sort_by_key(|binding| {
|
||||
mention_start(binding.sigil, &binding.mention).unwrap_or(usize::MAX)
|
||||
});
|
||||
self.bottom_pane
|
||||
.record_replayed_user_message_history(HistoryEntry {
|
||||
text: display.message.clone(),
|
||||
text_elements: display.text_elements.clone(),
|
||||
local_image_paths: display.local_images.clone(),
|
||||
remote_image_urls: display.remote_image_urls.clone(),
|
||||
mention_bindings,
|
||||
pending_pastes: Vec::new(),
|
||||
});
|
||||
self.on_user_message_display(display);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::app_event::HistoryLookupResponse;
|
||||
use codex_app_server_protocol::NetworkAccess;
|
||||
use codex_app_server_protocol::SandboxPolicy;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
@@ -74,6 +75,120 @@ async fn resumed_initial_messages_render_history() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_user_messages_seed_composer_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.bottom_pane.set_history_metadata(
|
||||
ThreadId::new(),
|
||||
/*log_id*/ 1,
|
||||
/*entry_count*/ 3,
|
||||
);
|
||||
|
||||
let mut replay_mention = |id: &str, text: &str, name: &str, path: &str| {
|
||||
replay_user_message_inputs(
|
||||
&mut chat,
|
||||
id,
|
||||
vec![
|
||||
AppServerUserInput::Text {
|
||||
text: text.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
},
|
||||
AppServerUserInput::Mention {
|
||||
name: name.to_string(),
|
||||
path: path.to_string(),
|
||||
},
|
||||
],
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
};
|
||||
replay_mention(
|
||||
"user-1",
|
||||
"use $sample",
|
||||
"Sample Plugin",
|
||||
"plugin://sample@test",
|
||||
);
|
||||
replay_mention(
|
||||
"user-2",
|
||||
"use $google-calendar",
|
||||
"Google Calendar",
|
||||
"app://google_calendar",
|
||||
);
|
||||
drain_insert_history(&mut rx);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "use $google-calendar");
|
||||
assert_eq!(
|
||||
chat.bottom_pane.take_mention_bindings(),
|
||||
vec![MentionBinding {
|
||||
sigil: '$',
|
||||
mention: "google-calendar".to_string(),
|
||||
path: "app://google_calendar".to_string(),
|
||||
}]
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "use $sample");
|
||||
assert_eq!(
|
||||
chat.bottom_pane.take_mention_bindings(),
|
||||
vec![MentionBinding {
|
||||
sigil: '$',
|
||||
mention: "sample".to_string(),
|
||||
path: "plugin://sample@test".to_string(),
|
||||
}]
|
||||
);
|
||||
|
||||
let mut next_lookup_offset = || {
|
||||
let AppEvent::LookupMessageHistoryEntry { offset, .. } =
|
||||
rx.try_recv().expect("expected lookup")
|
||||
else {
|
||||
panic!("unexpected event variant");
|
||||
};
|
||||
offset
|
||||
};
|
||||
let response = |offset, entry: &str| HistoryLookupResponse {
|
||||
offset,
|
||||
log_id: 1,
|
||||
entry: Some(entry.to_string()),
|
||||
};
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
chat.handle_history_entry_response(response(
|
||||
next_lookup_offset(),
|
||||
"use [$google-calendar](app://google_calendar)",
|
||||
));
|
||||
|
||||
assert_eq!(next_lookup_offset(), 1);
|
||||
chat.handle_history_entry_response(response(1, "use [$sample](plugin://sample@test)"));
|
||||
|
||||
assert_eq!(next_lookup_offset(), 0);
|
||||
chat.handle_history_entry_response(response(0, "/rename smoke-1"));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "/rename smoke-1");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_review_prompt_does_not_seed_composer_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.replay_thread_item(
|
||||
AppServerThreadItem::EnteredReviewMode {
|
||||
id: "review-start".to_string(),
|
||||
review: "changes against main".to_string(),
|
||||
},
|
||||
"turn-1".to_string(),
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
replay_user_message_text(
|
||||
&mut chat,
|
||||
"review-prompt",
|
||||
"Review the code changes against the base branch 'main'.",
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
drain_insert_history(&mut rx);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_user_message_preserves_text_elements_and_local_images() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
Reference in New Issue
Block a user