mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Cancel pending transcript Home jumps on subsequent navigation (#46722)
## Why Pressing `Home` while older transcript history is loading leaves a pending jump to the beginning. Subsequent navigation should supersede that jump so arriving history does not override the user's new reading position. ## What changed Cancel the pending jump when scrolling, jumping to the latest entry, or navigating prompts in the read-only transcript viewer. Keep the in-flight history page by switching from `LoadingBeginning` to `LoadingOlder`. Handle navigation before the first draw as well as after rendering, while preserving pending jumps during passive highlight restoration. ## Testing Add regression coverage for navigation after `Home` before and after the first draw, checking that arriving older history preserves the expected viewport. Extend prompt backtracking tests to verify that navigation clears the pending jump. GitOrigin-RevId: 438c3289afee2719655b11dbc43a3ccb006a5fa3
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
//! Analytics also preserves the composer and stays separate from transcript backtracking.
|
||||
|
||||
use super::*;
|
||||
use crate::pager_overlay::TranscriptHistoryState;
|
||||
use crate::test_support::test_path_display;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
@@ -271,7 +272,22 @@ async fn transcript_flag_off_preserves_viewer_and_backtracking() -> Result<()> {
|
||||
(KeyCode::Right, 1),
|
||||
(KeyCode::Right, 1),
|
||||
] {
|
||||
if let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() {
|
||||
overlay.set_history_state(TranscriptHistoryState::LoadingBeginning);
|
||||
overlay.render(area, &mut buffer);
|
||||
assert_eq!(
|
||||
overlay.set_history_state(TranscriptHistoryState::LoadingBeginning),
|
||||
TranscriptHistoryState::LoadingBeginning,
|
||||
);
|
||||
}
|
||||
press_key(&mut app, &mut tui, &mut app_server, key).await?;
|
||||
let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() else {
|
||||
panic!("viewer closed")
|
||||
};
|
||||
assert_eq!(
|
||||
overlay.set_history_state(TranscriptHistoryState::Complete),
|
||||
TranscriptHistoryState::LoadingOlder,
|
||||
);
|
||||
assert_eq!(app.backtrack.nth_user_message, selected);
|
||||
}
|
||||
press_key(&mut app, &mut tui, &mut app_server, KeyCode::Enter).await?;
|
||||
|
||||
@@ -12,6 +12,15 @@ impl App {
|
||||
app_server: &mut AppServerSession,
|
||||
event: TuiEvent,
|
||||
) -> Result<bool> {
|
||||
if let TuiEvent::Key(key) = &event
|
||||
&& matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat)
|
||||
&& (key.code == KeyCode::Esc
|
||||
|| (self.backtrack.overlay_preview_active
|
||||
&& matches!(key.code, KeyCode::Left | KeyCode::Right)))
|
||||
&& let Some(Overlay::Transcript(overlay)) = self.overlay.as_mut()
|
||||
{
|
||||
overlay.cancel_pending_jump();
|
||||
}
|
||||
if let TuiEvent::Key(key_event) = &event
|
||||
&& let Some(Overlay::Transcript(overlay)) = self.overlay.as_mut()
|
||||
&& (overlay.should_load_older(*key_event)
|
||||
|
||||
@@ -228,6 +228,11 @@ impl TranscriptOverlay {
|
||||
self.view.sync_live_tail(width, key, compute_lines);
|
||||
}
|
||||
|
||||
/// Explicit prompt navigation supersedes Home; passive highlight restoration does not.
|
||||
pub(crate) fn cancel_pending_jump(&mut self) {
|
||||
self.view.cancel_beginning();
|
||||
}
|
||||
|
||||
pub(crate) fn set_highlight_cell(&mut self, cell: Option<usize>) {
|
||||
self.highlight_cell = cell.filter(|index| *index < self.cells.len());
|
||||
self.reveal_highlight = self.highlight_cell.is_some();
|
||||
@@ -270,6 +275,7 @@ impl TranscriptOverlay {
|
||||
let Some(delta) = delta else {
|
||||
return false;
|
||||
};
|
||||
self.view.cancel_beginning();
|
||||
if !self.content_area.is_empty() {
|
||||
self.scroll(delta);
|
||||
}
|
||||
|
||||
@@ -195,35 +195,51 @@ async fn loading_beginning_keeps_the_entry_until_history_is_complete() -> Result
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn navigation_before_the_first_draw_preserves_the_full_latest_viewport() -> Result<()> {
|
||||
#[test]
|
||||
fn navigation_supersedes_home_before_and_after_the_first_draw() {
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 10,
|
||||
);
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
for key in [
|
||||
KeyEvent::new(KeyCode::Up, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Down, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL),
|
||||
KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL),
|
||||
] {
|
||||
let cells = vec![Arc::new(TestCell {
|
||||
lines: (0..30)
|
||||
.map(|index| Line::from(format!("line {index}")))
|
||||
.collect(),
|
||||
}) as Arc<dyn HistoryCell>];
|
||||
let mut expected_overlay = transcript_overlay(cells.clone());
|
||||
let mut expected = Buffer::empty(area);
|
||||
expected_overlay.render(area, &mut expected);
|
||||
let mut overlay = transcript_overlay(cells);
|
||||
overlay.handle_event(&mut tui, TuiEvent::Key(key))?;
|
||||
let mut actual = Buffer::empty(area);
|
||||
overlay.render(area, &mut actual);
|
||||
assert_eq!(actual, expected);
|
||||
for rendered in [false, true] {
|
||||
for key in [
|
||||
KeyEvent::new(KeyCode::Up, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Down, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL),
|
||||
KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL),
|
||||
KeyEvent::new(KeyCode::End, KeyModifiers::NONE),
|
||||
] {
|
||||
let cells = vec![Arc::new(TestCell {
|
||||
lines: (0..30)
|
||||
.map(|index| Line::from(format!("line {index}")))
|
||||
.collect(),
|
||||
}) as Arc<dyn HistoryCell>];
|
||||
let mut expected = transcript_overlay(cells.clone());
|
||||
let mut actual = transcript_overlay(cells);
|
||||
actual.set_history_state(TranscriptHistoryState::Partial);
|
||||
if rendered {
|
||||
expected.render(area, &mut Buffer::empty(area));
|
||||
actual.render(area, &mut Buffer::empty(area));
|
||||
}
|
||||
expected.navigate(key);
|
||||
actual.navigate(KeyEvent::new(KeyCode::Home, KeyModifiers::NONE));
|
||||
actual.navigate(key);
|
||||
assert_eq!(actual.view.history, TranscriptHistoryState::LoadingOlder);
|
||||
actual.prepend(vec![Arc::new(TestCell {
|
||||
lines: vec!["received older history".into()],
|
||||
})]);
|
||||
actual.set_history_state(TranscriptHistoryState::Complete);
|
||||
let mut expected_buffer = Buffer::empty(area);
|
||||
let mut actual_buffer = Buffer::empty(area);
|
||||
expected.render(area, &mut expected_buffer);
|
||||
actual.render(area, &mut actual_buffer);
|
||||
assert_eq!(
|
||||
buffer_to_text(&actual_buffer, actual.content_area),
|
||||
buffer_to_text(&expected_buffer, expected.content_area)
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -242,6 +258,9 @@ fn transcript_overlay_snapshots_paginated_history_states() {
|
||||
("failed", TranscriptHistoryState::Failed),
|
||||
("complete", TranscriptHistoryState::Complete),
|
||||
] {
|
||||
overlay.set_history_state(TranscriptHistoryState::Partial);
|
||||
overlay.navigate(KeyEvent::new(KeyCode::Home, KeyModifiers::NONE));
|
||||
overlay.navigate(KeyEvent::new(KeyCode::End, KeyModifiers::NONE));
|
||||
overlay.set_history_state(state);
|
||||
let mut buf = Buffer::empty(area);
|
||||
overlay.render(area, &mut buf);
|
||||
|
||||
@@ -183,10 +183,14 @@ impl TranscriptView {
|
||||
}
|
||||
|
||||
pub(crate) fn jump_to_latest(&mut self) {
|
||||
self.cancel_beginning();
|
||||
self.position = Position::Latest;
|
||||
}
|
||||
|
||||
pub(crate) fn scroll(&mut self, cells: &[Arc<dyn HistoryCell>], rows: isize) {
|
||||
if rows != 0 {
|
||||
self.cancel_beginning();
|
||||
}
|
||||
let start = self.start(cells);
|
||||
let (index, row) = self.move_rows(cells, start.0, start.1, rows);
|
||||
if rows < 0
|
||||
@@ -220,6 +224,13 @@ impl TranscriptView {
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn cancel_beginning(&mut self) {
|
||||
if self.history == TranscriptHistoryState::LoadingBeginning {
|
||||
// Keep the in-flight page, but let the user's new navigation supersede the jump.
|
||||
self.history = TranscriptHistoryState::LoadingOlder;
|
||||
}
|
||||
}
|
||||
|
||||
/// Reveal an entire entry when prompt backtracking changes the highlight.
|
||||
pub(crate) fn ensure_entry_visible(&mut self, cells: &[Arc<dyn HistoryCell>], index: usize) {
|
||||
let key = cells.get(index).map_or(EntryKey::Live, EntryKey::cell);
|
||||
|
||||
Reference in New Issue
Block a user