chore: undo nits (#5631)

This commit is contained in:
jif-oai
2025-10-27 11:48:01 +00:00
committed by GitHub
parent 2338294b39
commit 5e8659dcbc
7 changed files with 66 additions and 18 deletions

View File

@@ -2,6 +2,7 @@ use std::sync::Arc;
use super::Session; use super::Session;
use super::TurnContext; use super::TurnContext;
use super::filter_model_visible_history;
use super::get_last_assistant_message_from_turn; use super::get_last_assistant_message_from_turn;
use crate::Prompt; use crate::Prompt;
use crate::client_common::ResponseEvent; use crate::client_common::ResponseEvent;
@@ -86,8 +87,9 @@ async fn run_compact_task_inner(
loop { loop {
let turn_input = history.get_history(); let turn_input = history.get_history();
let prompt_input = filter_model_visible_history(turn_input.clone());
let prompt = Prompt { let prompt = Prompt {
input: turn_input.clone(), input: prompt_input.clone(),
..Default::default() ..Default::default()
}; };
let attempt_result = drain_to_completed(&sess, turn_context.as_ref(), &prompt).await; let attempt_result = drain_to_completed(&sess, turn_context.as_ref(), &prompt).await;
@@ -109,7 +111,7 @@ async fn run_compact_task_inner(
return; return;
} }
Err(e @ CodexErr::ContextWindowExceeded) => { Err(e @ CodexErr::ContextWindowExceeded) => {
if turn_input.len() > 1 { if prompt_input.len() > 1 {
// Trim from the beginning to preserve cache (prefix-based) and keep recent messages intact. // Trim from the beginning to preserve cache (prefix-based) and keep recent messages intact.
error!( error!(
"Context window exceeded while compacting; removing oldest history item. Error: {e}" "Context window exceeded while compacting; removing oldest history item. Error: {e}"
@@ -152,7 +154,13 @@ async fn run_compact_task_inner(
let summary_text = get_last_assistant_message_from_turn(&history_snapshot).unwrap_or_default(); let summary_text = get_last_assistant_message_from_turn(&history_snapshot).unwrap_or_default();
let user_messages = collect_user_messages(&history_snapshot); let user_messages = collect_user_messages(&history_snapshot);
let initial_context = sess.build_initial_context(turn_context.as_ref()); let initial_context = sess.build_initial_context(turn_context.as_ref());
let new_history = build_compacted_history(initial_context, &user_messages, &summary_text); let mut new_history = build_compacted_history(initial_context, &user_messages, &summary_text);
let ghost_snapshots: Vec<ResponseItem> = history_snapshot
.iter()
.filter(|item| matches!(item, ResponseItem::GhostSnapshot { .. }))
.cloned()
.collect();
new_history.extend(ghost_snapshots);
sess.replace_history(new_history).await; sess.replace_history(new_history).await;
let rollout_item = RolloutItem::Compacted(CompactedItem { let rollout_item = RolloutItem::Compacted(CompactedItem {

View File

@@ -499,6 +499,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
| EventMsg::GetHistoryEntryResponse(_) | EventMsg::GetHistoryEntryResponse(_)
| EventMsg::McpListToolsResponse(_) | EventMsg::McpListToolsResponse(_)
| EventMsg::ListCustomPromptsResponse(_) | EventMsg::ListCustomPromptsResponse(_)
| EventMsg::RawResponseItem(_)
| EventMsg::UserMessage(_) | EventMsg::UserMessage(_)
| EventMsg::EnteredReviewMode(_) | EventMsg::EnteredReviewMode(_)
| EventMsg::ExitedReviewMode(_) | EventMsg::ExitedReviewMode(_)
@@ -507,7 +508,6 @@ impl EventProcessor for EventProcessorWithHumanOutput {
| EventMsg::AgentReasoningRawContentDelta(_) | EventMsg::AgentReasoningRawContentDelta(_)
| EventMsg::ItemStarted(_) | EventMsg::ItemStarted(_)
| EventMsg::ItemCompleted(_) | EventMsg::ItemCompleted(_)
| EventMsg::RawResponseItem(_)
| EventMsg::UndoCompleted(_) | EventMsg::UndoCompleted(_)
| EventMsg::UndoStarted(_) => {} | EventMsg::UndoStarted(_) => {}
} }

View File

@@ -159,13 +159,13 @@ pub fn restore_ghost_commit(repo_path: &Path, commit: &GhostCommit) -> Result<()
let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); let repo_prefix = repo_subdir(repo_root.as_path(), repo_path);
let current_untracked = let current_untracked =
capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?; capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?;
restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit.id())?;
remove_new_untracked( remove_new_untracked(
repo_root.as_path(), repo_root.as_path(),
commit.preexisting_untracked_files(), commit.preexisting_untracked_files(),
commit.preexisting_untracked_dirs(), commit.preexisting_untracked_dirs(),
current_untracked, current_untracked,
)?; )
restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit.id())
} }
/// Restore the working tree to match the given commit ID. /// Restore the working tree to match the given commit ID.

View File

@@ -348,6 +348,7 @@ impl BottomPane {
)); ));
} }
if let Some(status) = self.status.as_mut() { if let Some(status) = self.status.as_mut() {
status.set_interrupt_hint_visible(true);
status.set_queued_messages(self.queued_user_messages.clone()); status.set_queued_messages(self.queued_user_messages.clone());
} }
self.request_redraw(); self.request_redraw();
@@ -374,6 +375,13 @@ impl BottomPane {
} }
} }
pub(crate) fn set_interrupt_hint_visible(&mut self, visible: bool) {
if let Some(status) = self.status.as_mut() {
status.set_interrupt_hint_visible(visible);
self.request_redraw();
}
}
pub(crate) fn set_context_window_percent(&mut self, percent: Option<i64>) { pub(crate) fn set_context_window_percent(&mut self, percent: Option<i64>) {
if self.context_window_percent == percent { if self.context_window_percent == percent {
return; return;

View File

@@ -304,9 +304,6 @@ impl ChatWidget {
} }
fn set_status_header(&mut self, header: String) { fn set_status_header(&mut self, header: String) {
if self.current_status_header == header {
return;
}
self.current_status_header = header.clone(); self.current_status_header = header.clone();
self.bottom_pane.update_status_header(header); self.bottom_pane.update_status_header(header);
} }
@@ -429,6 +426,7 @@ impl ChatWidget {
self.bottom_pane.clear_ctrl_c_quit_hint(); self.bottom_pane.clear_ctrl_c_quit_hint();
self.bottom_pane.set_task_running(true); self.bottom_pane.set_task_running(true);
self.retry_status_header = None; self.retry_status_header = None;
self.bottom_pane.set_interrupt_hint_visible(true);
self.set_status_header(String::from("Working")); self.set_status_header(String::from("Working"));
self.full_reasoning_buffer.clear(); self.full_reasoning_buffer.clear();
self.reasoning_buffer.clear(); self.reasoning_buffer.clear();
@@ -666,6 +664,7 @@ impl ChatWidget {
fn on_undo_started(&mut self, event: UndoStartedEvent) { fn on_undo_started(&mut self, event: UndoStartedEvent) {
self.bottom_pane.ensure_status_indicator(); self.bottom_pane.ensure_status_indicator();
self.bottom_pane.set_interrupt_hint_visible(false);
let message = event let message = event
.message .message
.unwrap_or_else(|| "Undo in progress...".to_string()); .unwrap_or_else(|| "Undo in progress...".to_string());

View File

@@ -933,6 +933,25 @@ fn undo_failure_events_render_error_message() {
); );
} }
#[test]
fn undo_started_hides_interrupt_hint() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
chat.handle_codex_event(Event {
id: "turn-hint".to_string(),
msg: EventMsg::UndoStarted(UndoStartedEvent { message: None }),
});
let status = chat
.bottom_pane
.status_widget()
.expect("status indicator should be active");
assert!(
!status.interrupt_hint_visible(),
"undo should hide the interrupt hint because the operation cannot be cancelled"
);
}
/// The commit picker shows only commit subjects (no timestamps). /// The commit picker shows only commit subjects (no timestamps).
#[test] #[test]
fn review_commit_picker_shows_subjects_without_timestamps() { fn review_commit_picker_shows_subjects_without_timestamps() {

View File

@@ -25,6 +25,8 @@ pub(crate) struct StatusIndicatorWidget {
header: String, header: String,
/// Queued user messages to display under the status line. /// Queued user messages to display under the status line.
queued_messages: Vec<String>, queued_messages: Vec<String>,
/// Whether to show the interrupt hint (Esc).
show_interrupt_hint: bool,
elapsed_running: Duration, elapsed_running: Duration,
last_resume_at: Instant, last_resume_at: Instant,
@@ -55,6 +57,7 @@ impl StatusIndicatorWidget {
Self { Self {
header: String::from("Working"), header: String::from("Working"),
queued_messages: Vec::new(), queued_messages: Vec::new(),
show_interrupt_hint: true,
elapsed_running: Duration::ZERO, elapsed_running: Duration::ZERO,
last_resume_at: Instant::now(), last_resume_at: Instant::now(),
is_paused: false, is_paused: false,
@@ -98,9 +101,11 @@ impl StatusIndicatorWidget {
/// Update the animated header label (left of the brackets). /// Update the animated header label (left of the brackets).
pub(crate) fn update_header(&mut self, header: String) { pub(crate) fn update_header(&mut self, header: String) {
if self.header != header { self.header = header;
self.header = header; }
}
pub(crate) fn set_interrupt_hint_visible(&mut self, visible: bool) {
self.show_interrupt_hint = visible;
} }
#[cfg(test)] #[cfg(test)]
@@ -108,6 +113,11 @@ impl StatusIndicatorWidget {
&self.header &self.header
} }
#[cfg(test)]
pub(crate) fn interrupt_hint_visible(&self) -> bool {
self.show_interrupt_hint
}
/// Replace the queued messages displayed beneath the header. /// Replace the queued messages displayed beneath the header.
pub(crate) fn set_queued_messages(&mut self, queued: Vec<String>) { pub(crate) fn set_queued_messages(&mut self, queued: Vec<String>) {
self.queued_messages = queued; self.queued_messages = queued;
@@ -175,12 +185,16 @@ impl WidgetRef for StatusIndicatorWidget {
spans.push(spinner(Some(self.last_resume_at))); spans.push(spinner(Some(self.last_resume_at)));
spans.push(" ".into()); spans.push(" ".into());
spans.extend(shimmer_spans(&self.header)); spans.extend(shimmer_spans(&self.header));
spans.extend(vec![ spans.push(" ".into());
" ".into(), if self.show_interrupt_hint {
format!("({pretty_elapsed}").dim(), spans.extend(vec![
key_hint::plain(KeyCode::Esc).into(), format!("({pretty_elapsed}").dim(),
" to interrupt)".dim(), key_hint::plain(KeyCode::Esc).into(),
]); " to interrupt)".dim(),
]);
} else {
spans.push(format!("({pretty_elapsed})").dim());
}
// Build lines: status, then queued messages, then spacer. // Build lines: status, then queued messages, then spacer.
let mut lines: Vec<Line<'static>> = Vec::new(); let mut lines: Vec<Line<'static>> = Vec::new();