mirror of
https://github.com/openai/codex.git
synced 2026-09-07 15:40:00 +00:00
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
This commit is contained in:
committed by
copyberry
parent
0e03f88a30
commit
c5a3700dd7
@@ -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<usize>) {
|
||||
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);
|
||||
}
|
||||
|
||||
144
codex-rs/tui/src/pager_overlay/highlight_tests.rs
Normal file
144
codex-rs/tui/src/pager_overlay/highlight_tests.rs
Normal file
@@ -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<Line<'static>> {
|
||||
vec!["history".into()]
|
||||
}
|
||||
|
||||
fn raw_lines(&self) -> Vec<Line<'static>> {
|
||||
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<dyn HistoryCell>)
|
||||
.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::<Vec<_>>()
|
||||
};
|
||||
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<Arc<dyn HistoryCell>> = ["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<dyn HistoryCell>
|
||||
})
|
||||
.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::<String>()
|
||||
})
|
||||
.map(|line| line.trim_end().to_string())
|
||||
.filter(|line| !line.is_empty() && line != "~")
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
insta::assert_snapshot!(content, @"
|
||||
› first prompt
|
||||
› second prompt
|
||||
live tail
|
||||
");
|
||||
}
|
||||
Reference in New Issue
Block a user