mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
refactor(tui2): make transcript line metadata explicit (#8089)
This is a pure refactor only change.
Replace the flattened transcript line metadata from `Option<(usize,
usize)>` to an explicit
`TranscriptLineMeta::{CellLine { cell_index, line_in_cell }, Spacer}`
enum.
This makes spacer rows unambiguous, removes “tuple semantics” from call
sites, and keeps the
scroll anchoring model clearer and aligned with the viewport/history
design notes.
Changes:
- Introduce `TranscriptLineMeta` and update `TranscriptScroll` helpers
to consume it.
- Update `App::build_transcript_lines` and downstream consumers
(scrolling, row classification, ANSI rendering).
- Refresh scrolling module docs to describe anchors + spacer semantics
in context.
- Add tests and docs about the behavior
Tests:
- just fmt
- cargo test -p codex-tui2 tui::scrolling
Manual testing:
- Scroll the inline transcript with mouse wheel + PgUp/PgDn/Home/End,
then resize the terminal while staying scrolled up; verify the same
anchored content stays in view and you don’t jump to bottom
unexpectedly.
- Create a gap case (multiple non-continuation cells) and scroll so a
blank spacer row is at/near the top; verify scrolling doesn’t get stuck
on spacers and still anchors to nearby real lines.
- Start a selection while the assistant is streaming; verify the view
stops auto-following, the selection stays on the intended content, and
subsequent scrolling still behaves normally.
- Exit the TUI and confirm scrollback rendering still styles user rows
as blocks (background padding) and non-user rows as expected.
This commit is contained in:
1 parent
b9d1a087ee
commit
f074e5706b
3 files changed
+414
-148
No files matched your search
+47
-148
@@ -21,6 +21,8 @@ use crate::skill_error_prompt::SkillErrorPromptOutcome;
|
||||
use crate::skill_error_prompt::run_skill_error_prompt;
|
||||
use crate::tui;
|
||||
use crate::tui::TuiEvent;
|
||||
use crate::tui::scrolling::TranscriptLineMeta;
|
||||
use crate::tui::scrolling::TranscriptScroll;
|
||||
use crate::update_action::UpdateAction;
|
||||
use crate::wrapping::RtOptions;
|
||||
use crate::wrapping::word_wrap_line;
|
||||
@@ -339,21 +341,6 @@ pub(crate) struct App {
|
||||
skip_world_writable_scan_once: bool,
|
||||
}
|
||||
|
||||
/// Scroll state for the inline transcript viewport.
|
||||
///
|
||||
/// This tracks whether the transcript is pinned to the latest line or anchored
|
||||
/// at a specific cell/line pair so later viewport changes can implement
|
||||
/// scrollback without losing the notion of "bottom".
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, Clone, Copy, Default)]
|
||||
enum TranscriptScroll {
|
||||
#[default]
|
||||
ToBottom,
|
||||
Scrolled {
|
||||
cell_index: usize,
|
||||
line_in_cell: usize,
|
||||
},
|
||||
}
|
||||
/// Content-relative selection within the inline transcript viewport.
|
||||
///
|
||||
/// Selection endpoints are expressed in terms of flattened, wrapped transcript
|
||||
@@ -494,7 +481,7 @@ impl App {
|
||||
file_search,
|
||||
enhanced_keys_supported,
|
||||
transcript_cells: Vec::new(),
|
||||
transcript_scroll: TranscriptScroll::ToBottom,
|
||||
transcript_scroll: TranscriptScroll::default(),
|
||||
transcript_selection: TranscriptSelection::default(),
|
||||
transcript_view_top: 0,
|
||||
transcript_total_lines: 0,
|
||||
@@ -562,13 +549,13 @@ impl App {
|
||||
let session_lines = if width == 0 {
|
||||
Vec::new()
|
||||
} else {
|
||||
let (lines, meta) = Self::build_transcript_lines(&app.transcript_cells, width);
|
||||
let (lines, line_meta) = Self::build_transcript_lines(&app.transcript_cells, width);
|
||||
let is_user_cell: Vec<bool> = app
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.map(|cell| cell.as_any().is::<UserHistoryCell>())
|
||||
.collect();
|
||||
Self::render_lines_to_ansi(&lines, &meta, &is_user_cell, width)
|
||||
Self::render_lines_to_ansi(&lines, &line_meta, &is_user_cell, width)
|
||||
};
|
||||
|
||||
tui.terminal.clear()?;
|
||||
@@ -676,7 +663,7 @@ impl App {
|
||||
) -> u16 {
|
||||
let area = frame.area();
|
||||
if area.width == 0 || area.height == 0 {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
self.transcript_scroll = TranscriptScroll::default();
|
||||
self.transcript_view_top = 0;
|
||||
self.transcript_total_lines = 0;
|
||||
return area.bottom().saturating_sub(chat_height);
|
||||
@@ -685,7 +672,7 @@ impl App {
|
||||
let chat_height = chat_height.min(area.height);
|
||||
let max_transcript_height = area.height.saturating_sub(chat_height);
|
||||
if max_transcript_height == 0 {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
self.transcript_scroll = TranscriptScroll::default();
|
||||
self.transcript_view_top = 0;
|
||||
self.transcript_total_lines = 0;
|
||||
return area.y;
|
||||
@@ -698,10 +685,10 @@ impl App {
|
||||
height: max_transcript_height,
|
||||
};
|
||||
|
||||
let (lines, meta) = Self::build_transcript_lines(cells, transcript_area.width);
|
||||
let (lines, line_meta) = Self::build_transcript_lines(cells, transcript_area.width);
|
||||
if lines.is_empty() {
|
||||
Clear.render_ref(transcript_area, frame.buffer);
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
self.transcript_scroll = TranscriptScroll::default();
|
||||
self.transcript_view_top = 0;
|
||||
self.transcript_total_lines = 0;
|
||||
return area.y;
|
||||
@@ -709,7 +696,7 @@ impl App {
|
||||
|
||||
let wrapped = word_wrap_lines_borrowed(&lines, transcript_area.width.max(1) as usize);
|
||||
if wrapped.is_empty() {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
self.transcript_scroll = TranscriptScroll::default();
|
||||
self.transcript_view_top = 0;
|
||||
self.transcript_total_lines = 0;
|
||||
return area.y;
|
||||
@@ -731,10 +718,10 @@ impl App {
|
||||
.initial_indent(base_opts.subsequent_indent.clone())
|
||||
};
|
||||
let seg_count = word_wrap_line(line, opts).len();
|
||||
let is_user_row = meta
|
||||
let is_user_row = line_meta
|
||||
.get(idx)
|
||||
.and_then(Option::as_ref)
|
||||
.map(|(cell_index, _)| is_user_cell.get(*cell_index).copied().unwrap_or(false))
|
||||
.and_then(TranscriptLineMeta::cell_index)
|
||||
.map(|cell_index| is_user_cell.get(cell_index).copied().unwrap_or(false))
|
||||
.unwrap_or(false);
|
||||
wrapped_is_user_row.extend(std::iter::repeat_n(is_user_row, seg_count));
|
||||
first = false;
|
||||
@@ -745,30 +732,8 @@ impl App {
|
||||
let max_visible = std::cmp::min(max_transcript_height as usize, total_lines);
|
||||
let max_start = total_lines.saturating_sub(max_visible);
|
||||
|
||||
let top_offset = match self.transcript_scroll {
|
||||
TranscriptScroll::ToBottom => max_start,
|
||||
TranscriptScroll::Scrolled {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
} => {
|
||||
let mut anchor = None;
|
||||
for (idx, entry) in meta.iter().enumerate() {
|
||||
if let Some((ci, li)) = entry
|
||||
&& *ci == cell_index
|
||||
&& *li == line_in_cell
|
||||
{
|
||||
anchor = Some(idx);
|
||||
break;
|
||||
}
|
||||
}
|
||||
if let Some(idx) = anchor {
|
||||
idx.min(max_start)
|
||||
} else {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
max_start
|
||||
}
|
||||
}
|
||||
};
|
||||
let (scroll_state, top_offset) = self.transcript_scroll.resolve_top(&line_meta, max_start);
|
||||
self.transcript_scroll = scroll_state;
|
||||
self.transcript_view_top = top_offset;
|
||||
|
||||
let transcript_visible_height = max_visible as u16;
|
||||
@@ -974,69 +939,10 @@ impl App {
|
||||
return;
|
||||
}
|
||||
|
||||
let (lines, meta) = Self::build_transcript_lines(&self.transcript_cells, width);
|
||||
let total_lines = lines.len();
|
||||
if total_lines <= visible_lines {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
return;
|
||||
}
|
||||
|
||||
let max_start = total_lines.saturating_sub(visible_lines);
|
||||
|
||||
let current_top = match self.transcript_scroll {
|
||||
TranscriptScroll::ToBottom => max_start,
|
||||
TranscriptScroll::Scrolled {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
} => {
|
||||
let mut anchor = None;
|
||||
for (idx, entry) in meta.iter().enumerate() {
|
||||
if let Some((ci, li)) = entry
|
||||
&& *ci == cell_index
|
||||
&& *li == line_in_cell
|
||||
{
|
||||
anchor = Some(idx);
|
||||
break;
|
||||
}
|
||||
}
|
||||
anchor.unwrap_or(max_start).min(max_start)
|
||||
}
|
||||
};
|
||||
|
||||
if delta_lines == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
let new_top = if delta_lines < 0 {
|
||||
current_top.saturating_sub(delta_lines.unsigned_abs() as usize)
|
||||
} else {
|
||||
current_top
|
||||
.saturating_add(delta_lines as usize)
|
||||
.min(max_start)
|
||||
};
|
||||
|
||||
if new_top == max_start {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
} else {
|
||||
let anchor = meta.iter().skip(new_top).find_map(|entry| *entry);
|
||||
if let Some((cell_index, line_in_cell)) = anchor {
|
||||
self.transcript_scroll = TranscriptScroll::Scrolled {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
};
|
||||
} else if let Some(prev_idx) = (0..=new_top).rfind(|&idx| meta[idx].is_some()) {
|
||||
if let Some((cell_index, line_in_cell)) = meta[prev_idx] {
|
||||
self.transcript_scroll = TranscriptScroll::Scrolled {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
};
|
||||
} else {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
}
|
||||
} else {
|
||||
self.transcript_scroll = TranscriptScroll::ToBottom;
|
||||
}
|
||||
}
|
||||
let (_, line_meta) = Self::build_transcript_lines(&self.transcript_cells, width);
|
||||
self.transcript_scroll =
|
||||
self.transcript_scroll
|
||||
.scrolled_by(delta_lines, &line_meta, visible_lines);
|
||||
|
||||
tui.frame_requester().schedule_frame();
|
||||
}
|
||||
@@ -1053,8 +959,8 @@ impl App {
|
||||
return;
|
||||
}
|
||||
|
||||
let (lines, meta) = Self::build_transcript_lines(&self.transcript_cells, width);
|
||||
if lines.is_empty() || meta.is_empty() {
|
||||
let (lines, line_meta) = Self::build_transcript_lines(&self.transcript_cells, width);
|
||||
if lines.is_empty() || line_meta.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1073,22 +979,8 @@ impl App {
|
||||
}
|
||||
};
|
||||
|
||||
let mut anchor = None;
|
||||
if let Some((cell_index, line_in_cell)) = meta.iter().skip(top_offset).flatten().next() {
|
||||
anchor = Some((*cell_index, *line_in_cell));
|
||||
}
|
||||
if anchor.is_none()
|
||||
&& let Some((cell_index, line_in_cell)) =
|
||||
meta[..top_offset].iter().rev().flatten().next()
|
||||
{
|
||||
anchor = Some((*cell_index, *line_in_cell));
|
||||
}
|
||||
|
||||
if let Some((cell_index, line_in_cell)) = anchor {
|
||||
self.transcript_scroll = TranscriptScroll::Scrolled {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
};
|
||||
if let Some(scroll_state) = TranscriptScroll::anchor_for(&line_meta, top_offset) {
|
||||
self.transcript_scroll = scroll_state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1096,16 +988,17 @@ impl App {
|
||||
///
|
||||
/// Returns both the visible `Line` buffer and a parallel metadata vector
|
||||
/// that maps each line back to its originating `(cell_index, line_in_cell)`
|
||||
/// pair, or `None` for spacer lines. This allows the scroll state to anchor
|
||||
/// to a specific history cell even as new content arrives or the viewport
|
||||
/// size changes, and gives exit transcript renderers enough structure to
|
||||
/// style user rows differently from agent rows.
|
||||
/// pair (see `TranscriptLineMeta::CellLine`), or `TranscriptLineMeta::Spacer` for
|
||||
/// synthetic spacer rows inserted between cells. This allows the scroll state
|
||||
/// to anchor to a specific history cell even as new content arrives or the
|
||||
/// viewport size changes, and gives exit transcript renderers enough structure
|
||||
/// to style user rows differently from agent rows.
|
||||
fn build_transcript_lines(
|
||||
cells: &[Arc<dyn HistoryCell>],
|
||||
width: u16,
|
||||
) -> (Vec<Line<'static>>, Vec<Option<(usize, usize)>>) {
|
||||
) -> (Vec<Line<'static>>, Vec<TranscriptLineMeta>) {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
let mut meta: Vec<Option<(usize, usize)>> = Vec::new();
|
||||
let mut line_meta: Vec<TranscriptLineMeta> = Vec::new();
|
||||
let mut has_emitted_lines = false;
|
||||
|
||||
for (cell_index, cell) in cells.iter().enumerate() {
|
||||
@@ -1117,19 +1010,22 @@ impl App {
|
||||
if !cell.is_stream_continuation() {
|
||||
if has_emitted_lines {
|
||||
lines.push(Line::from(""));
|
||||
meta.push(None);
|
||||
line_meta.push(TranscriptLineMeta::Spacer);
|
||||
} else {
|
||||
has_emitted_lines = true;
|
||||
}
|
||||
}
|
||||
|
||||
for (line_in_cell, line) in cell_lines.into_iter().enumerate() {
|
||||
meta.push(Some((cell_index, line_in_cell)));
|
||||
line_meta.push(TranscriptLineMeta::CellLine {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
});
|
||||
lines.push(line);
|
||||
}
|
||||
}
|
||||
|
||||
(lines, meta)
|
||||
(lines, line_meta)
|
||||
}
|
||||
|
||||
/// Render flattened transcript lines into ANSI strings suitable for
|
||||
@@ -1144,7 +1040,7 @@ impl App {
|
||||
/// and tools see consistent escape sequences.
|
||||
fn render_lines_to_ansi(
|
||||
lines: &[Line<'static>],
|
||||
meta: &[Option<(usize, usize)>],
|
||||
line_meta: &[TranscriptLineMeta],
|
||||
is_user_cell: &[bool],
|
||||
width: u16,
|
||||
) -> Vec<String> {
|
||||
@@ -1152,10 +1048,10 @@ impl App {
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(idx, line)| {
|
||||
let is_user_row = meta
|
||||
let is_user_row = line_meta
|
||||
.get(idx)
|
||||
.and_then(|entry| entry.as_ref())
|
||||
.map(|(cell_index, _)| is_user_cell.get(*cell_index).copied().unwrap_or(false))
|
||||
.and_then(TranscriptLineMeta::cell_index)
|
||||
.map(|cell_index| is_user_cell.get(cell_index).copied().unwrap_or(false))
|
||||
.unwrap_or(false);
|
||||
|
||||
let mut merged_spans: Vec<ratatui::text::Span<'static>> = line
|
||||
@@ -2262,7 +2158,7 @@ mod tests {
|
||||
active_profile: None,
|
||||
file_search,
|
||||
transcript_cells: Vec::new(),
|
||||
transcript_scroll: TranscriptScroll::ToBottom,
|
||||
transcript_scroll: TranscriptScroll::default(),
|
||||
transcript_selection: TranscriptSelection::default(),
|
||||
transcript_view_top: 0,
|
||||
transcript_total_lines: 0,
|
||||
@@ -2306,7 +2202,7 @@ mod tests {
|
||||
active_profile: None,
|
||||
file_search,
|
||||
transcript_cells: Vec::new(),
|
||||
transcript_scroll: TranscriptScroll::ToBottom,
|
||||
transcript_scroll: TranscriptScroll::default(),
|
||||
transcript_selection: TranscriptSelection::default(),
|
||||
transcript_view_top: 0,
|
||||
transcript_total_lines: 0,
|
||||
@@ -2576,11 +2472,14 @@ mod tests {
|
||||
fn render_lines_to_ansi_pads_user_rows_to_full_width() {
|
||||
let line: Line<'static> = Line::from("hi");
|
||||
let lines = vec![line];
|
||||
let meta = vec![Some((0usize, 0usize))];
|
||||
let line_meta = vec![TranscriptLineMeta::CellLine {
|
||||
cell_index: 0,
|
||||
line_in_cell: 0,
|
||||
}];
|
||||
let is_user_cell = vec![true];
|
||||
let width: u16 = 10;
|
||||
|
||||
let rendered = App::render_lines_to_ansi(&lines, &meta, &is_user_cell, width);
|
||||
let rendered = App::render_lines_to_ansi(&lines, &line_meta, &is_user_cell, width);
|
||||
assert_eq!(rendered.len(), 1);
|
||||
assert!(rendered[0].contains("hi"));
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user