From c5a3700dd7ee73a07b30ce1516dae51a264bb8cf Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Mon, 31 Aug 2026 23:28:34 +0000 Subject: [PATCH] Preserve transcript layout caches during backtrack selection (#41940) ## Why Moving between prompts in backtrack mode rebuilt every transcript renderable and discarded their cached heights, causing the entire transcript to be laid out again for each selection change. ## What changed Rerender only the previously highlighted cell and the newly highlighted cell. Keep all other renderables, cached heights, and the live tail intact while continuing to scroll the selected prompt into view. ## Testing Add regression tests that verify selection changes preserve unrelated height caches and produce the same viewport and scroll position as a full rebuild, including with a live tail and terminal width changes. GitOrigin-RevId: 685a8ce65ecadef20521ce2ded7b516fdf8499c9 --- codex-rs/tui/src/pager_overlay.rs | 21 ++- .../tui/src/pager_overlay/highlight_tests.rs | 144 ++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 codex-rs/tui/src/pager_overlay/highlight_tests.rs diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 8bb78b0253..3a52f951d4 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -17,6 +17,10 @@ mod scrolling; +#[cfg(test)] +#[path = "pager_overlay/highlight_tests.rs"] +mod highlight_tests; + use std::io::Result; use std::sync::Arc; @@ -798,9 +802,22 @@ impl TranscriptOverlay { } pub(crate) fn set_highlight_cell(&mut self, cell: Option) { - let live_tail = self.take_live_tail_renderable(); + let previous = self.highlight_cell; self.highlight_cell = cell; - self.rebuild_renderables(live_tail); + // Highlighting changes only these cells' styling. Keep the other renderables and their + // cached heights so moving between prompts does not lay out the entire transcript again. + if previous != cell { + for index in [previous, cell].into_iter().flatten() { + if let Some(history_cell) = self.cells.get(index) { + self.view.renderables[index] = Self::render_cell( + history_cell, + index, + self.highlight_cell, + self.history_state, + ); + } + } + } if let Some(idx) = self.highlight_cell { self.view.scroll_chunk_into_view(idx); } diff --git a/codex-rs/tui/src/pager_overlay/highlight_tests.rs b/codex-rs/tui/src/pager_overlay/highlight_tests.rs new file mode 100644 index 0000000000..a3661013f5 --- /dev/null +++ b/codex-rs/tui/src/pager_overlay/highlight_tests.rs @@ -0,0 +1,144 @@ +//! Backtrack selection preserves unrelated layout caches and the complete rendered viewport. + +use super::*; +use crate::history_cell::UserHistoryCell; +use crate::keymap::RuntimeKeymap; +use pretty_assertions::assert_eq; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; + +#[derive(Debug)] +struct MeasuredCell { + measurements: AtomicUsize, +} + +impl HistoryCell for MeasuredCell { + fn display_lines(&self, _width: u16) -> Vec> { + vec!["history".into()] + } + + fn raw_lines(&self) -> Vec> { + vec!["history".into()] + } + + fn desired_transcript_height(&self, _width: u16) -> u16 { + self.measurements.fetch_add(1, Ordering::Relaxed); + 1 + } +} + +#[test] +fn moving_highlight_preserves_unaffected_height_caches() { + let cells: Vec<_> = (0..32) + .map(|_| { + Arc::new(MeasuredCell { + measurements: AtomicUsize::new(0), + }) + }) + .collect(); + let mut overlay = TranscriptOverlay::new( + cells + .iter() + .map(|cell| cell.clone() as Arc) + .collect(), + RuntimeKeymap::defaults().pager, + ); + let mut area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 12, + ); + overlay.render(area, &mut Buffer::empty(area)); + + for selection in [Some(30), Some(28), Some(28), None] { + overlay.set_highlight_cell(selection); + overlay.render(area, &mut Buffer::empty(area)); + } + + let measurements = || { + cells + .iter() + .map(|cell| cell.measurements.load(Ordering::Relaxed)) + .collect::>() + }; + let mut expected = vec![1; cells.len()]; + expected[28] = 3; + expected[30] = 3; + assert_eq!(measurements(), expected); + + // Width changes still invalidate every cached height. + area.width = 24; + overlay.render(area, &mut Buffer::empty(area)); + for count in &mut expected { + *count += 1; + } + assert_eq!(measurements(), expected); +} + +#[test] +fn moving_highlight_matches_full_rebuild_with_live_tail() { + let cells: Vec> = ["first prompt", "second prompt"] + .into_iter() + .map(|message| { + Arc::new(UserHistoryCell { + message: message.to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }) as Arc + }) + .collect(); + let mut actual = TranscriptOverlay::new(cells.clone(), RuntimeKeymap::defaults().pager); + let mut expected = TranscriptOverlay::new(cells, RuntimeKeymap::defaults().pager); + for overlay in [&mut actual, &mut expected] { + overlay.sync_live_tail( + /*width*/ 40, + Some(ActiveCellTranscriptKey { + revision: 1, + is_stream_continuation: false, + animation_tick: None, + }), + |_| Some(vec![HyperlinkLine::from("live tail")]), + ); + } + + for width in [40, 24, 40] { + for selection in [Some(0), Some(1), Some(1), None, Some(99), Some(0)] { + actual.set_highlight_cell(selection); + let tail = expected.take_live_tail_renderable(); + expected.highlight_cell = selection; + expected.rebuild_renderables(tail); + if let Some(index) = selection { + expected.view.scroll_chunk_into_view(index); + } + + let area = Rect::new(/*x*/ 0, /*y*/ 0, width, /*height*/ 12); + let mut actual_buffer = Buffer::empty(area); + let mut expected_buffer = Buffer::empty(area); + actual.render(area, &mut actual_buffer); + expected.render(area, &mut expected_buffer); + assert_eq!(actual_buffer, expected_buffer); + assert_eq!(actual.view.scroll_offset, expected.view.scroll_offset); + } + } + + actual.set_highlight_cell(Some(1)); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 20, + ); + let mut buffer = Buffer::empty(area); + actual.render(area, &mut buffer); + let content = (1..area.height - 4) + .map(|y| { + (0..area.width) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + }) + .map(|line| line.trim_end().to_string()) + .filter(|line| !line.is_empty() && line != "~") + .collect::>() + .join("\n"); + insta::assert_snapshot!(content, @" + › first prompt + › second prompt + live tail + "); +}