diff --git a/.gitignore b/.gitignore index 2a4881b7..46181235 100644 --- a/.gitignore +++ b/.gitignore @@ -67,3 +67,4 @@ apps/ # Claude Code runtime artifacts .claude/scheduled_tasks.lock +.claude/worktrees/ diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 551148b9..1e12ad94 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -355,6 +355,10 @@ pub struct App { pub mode: AppMode, pub input: String, pub cursor_position: usize, + /// Single-entry kill buffer for emacs-style `Ctrl+K` cut / `Ctrl+Y` yank. + /// Populated by `kill_to_end_of_line`; restored by `yank`. Persists across + /// composer clears (e.g. submit) so a yank can recover an accidental kill. + pub kill_buffer: String, pub paste_burst: PasteBurst, pub history: Vec, pub history_version: u64, @@ -522,6 +526,10 @@ pub struct App { pub thinking_started_at: Option, /// Whether context compaction is currently in progress. pub is_compacting: bool, + /// Set when the user scrolls up/down during a streaming turn so subsequent + /// streamed chunks don't yank the view back to the live tail. Cleared + /// when the user explicitly returns to bottom or the turn completes. + pub user_scrolled_during_stream: bool, /// Plain-language session coherence state for the footer. pub coherence_state: CoherenceState, /// Timestamp of the last user message send (for brief visual feedback). @@ -672,11 +680,12 @@ impl App { mode: initial_mode, input: String::new(), cursor_position: 0, + kill_buffer: String::new(), paste_burst: PasteBurst::default(), history: Vec::new(), history_version: 0, api_messages: Vec::new(), - transcript_scroll: TranscriptScroll::ToBottom, + transcript_scroll: TranscriptScroll::to_bottom(), pending_scroll_delta: 0, mouse_scroll: MouseScrollState::new(), transcript_cache: TranscriptViewCache::new(), @@ -789,6 +798,7 @@ impl App { needs_redraw: true, thinking_started_at: None, is_compacting: false, + user_scrolled_during_stream: false, coherence_state: CoherenceState::default(), last_send_at: None, } @@ -917,9 +927,18 @@ impl App { .transcript_selection .ordered_endpoints() .is_some_and(|(start, end)| start != end); - if matches!(self.transcript_scroll, TranscriptScroll::ToBottom) + // Auto-pin to live tail only when: + // 1. We're already at the tail (nothing to do otherwise) + // 2. The user isn't actively selecting text + // 3. The user hasn't scrolled away during this streaming turn + // Without (3), pressing Up while a tool result streams in would lose + // the keypress: scroll_up sets pending_scroll_delta, but before the + // render frame consumes it, mark_history_updated would fire here, + // call scroll_to_bottom, and zero the delta. + if self.transcript_scroll.is_at_tail() && !self.transcript_selection.dragging && !selection_has_range + && !self.user_scrolled_during_stream { self.scroll_to_bottom(); } @@ -1079,17 +1098,15 @@ impl App { // Invalidate transcript cache (will be rebuilt on next render) self.transcript_cache = TranscriptViewCache::new(); - // Preserve cell-level anchor through resize: line_in_cell depends - // on the old width so reset it to 0 (start of the same cell). - // ToBottom stays ToBottom; SpacerBeforeCell keeps its cell_index. - match self.transcript_scroll { - TranscriptScroll::Scrolled { cell_index, .. } => { - self.transcript_scroll = TranscriptScroll::Scrolled { - cell_index, - line_in_cell: 0, - }; - } - TranscriptScroll::ToBottom | TranscriptScroll::ScrolledSpacerBeforeCell { .. } => {} + // The flat line-offset model is width-dependent (line wrapping + // changes the meta length on resize), so a stored offset can no + // longer point at the same logical content. Snapping back to the + // tail keeps the user where they intuitively expect — at the + // most recent output — and matches what Codex does on resize. + // The renderer will clamp anyway, but resetting to tail avoids + // a frame where the offset shows stale wrapping. + if !self.transcript_scroll.is_at_tail() { + self.transcript_scroll = TranscriptScroll::to_bottom(); } // Clear pending scroll delta @@ -1227,18 +1244,26 @@ impl App { pub fn scroll_up(&mut self, amount: usize) { let delta = i32::try_from(amount).unwrap_or(i32::MAX); self.pending_scroll_delta = self.pending_scroll_delta.saturating_sub(delta); + // Sticky intent: once the user has scrolled up during a stream, they + // shouldn't be yanked back to the live tail by subsequent chunks. + // Cleared when they explicitly return to bottom or the stream ends. + self.user_scrolled_during_stream = true; self.needs_redraw = true; } pub fn scroll_down(&mut self, amount: usize) { let delta = i32::try_from(amount).unwrap_or(i32::MAX); self.pending_scroll_delta = self.pending_scroll_delta.saturating_add(delta); + self.user_scrolled_during_stream = true; self.needs_redraw = true; } pub fn scroll_to_bottom(&mut self) { - self.transcript_scroll = TranscriptScroll::ToBottom; + self.transcript_scroll = TranscriptScroll::to_bottom(); self.pending_scroll_delta = 0; + // Explicit return-to-tail clears the stream-lock; new chunks will + // again pull the view down with them. + self.user_scrolled_during_stream = false; self.needs_redraw = true; } @@ -1277,6 +1302,66 @@ impl App { self.needs_redraw = true; } + /// Cut from the cursor to the end of the current logical line into the + /// kill buffer. If the cursor is already at end-of-line and a trailing + /// newline exists, that newline is consumed so repeated invocations + /// continue to make progress (matching emacs/codex semantics). + /// + /// Returns `true` when bytes were moved into the kill buffer. + pub fn kill_to_end_of_line(&mut self) -> bool { + let total_chars = char_count(&self.input); + let cursor = self.cursor_position.min(total_chars); + let start_byte = byte_index_at_char(&self.input, cursor); + + // Find the byte offset of the next '\n' (relative to the whole string) + // or the end of the buffer if no newline exists at/after the cursor. + let eol_byte = self.input[start_byte..] + .find('\n') + .map(|rel| start_byte + rel) + .unwrap_or_else(|| self.input.len()); + + let end_byte = if start_byte == eol_byte { + // Cursor is at EOL — consume the newline itself if one is there. + if eol_byte < self.input.len() { + eol_byte + 1 + } else { + return false; + } + } else { + eol_byte + }; + + let removed: String = self.input[start_byte..end_byte].to_string(); + if removed.is_empty() { + return false; + } + + self.kill_buffer = removed; + self.input.replace_range(start_byte..end_byte, ""); + // Cursor stays at the same character index (start of removed range). + self.cursor_position = cursor; + self.slash_menu_hidden = false; + self.needs_redraw = true; + true + } + + /// Insert the contents of the kill buffer at the cursor, advancing it. + /// The kill buffer is left intact so multiple yanks duplicate the text. + /// Returns `true` if any text was inserted. + pub fn yank(&mut self) -> bool { + if self.kill_buffer.is_empty() { + return false; + } + let text = self.kill_buffer.clone(); + let cursor = self.cursor_position.min(char_count(&self.input)); + let byte_index = byte_index_at_char(&self.input, cursor); + self.input.insert_str(byte_index, &text); + self.cursor_position = cursor + char_count(&text); + self.slash_menu_hidden = false; + self.needs_redraw = true; + true + } + pub fn move_cursor_left(&mut self) { self.cursor_position = self.cursor_position.saturating_sub(1); self.needs_redraw = true; @@ -1741,4 +1826,72 @@ mod tests { // Navigate down app.history_down(); } + + #[test] + fn kill_to_end_of_line_cuts_from_middle_of_word() { + let mut app = App::new(test_options(false), &Config::default()); + app.input = "hello world".to_string(); + app.cursor_position = 6; // before 'w' + assert!(app.kill_to_end_of_line()); + assert_eq!(app.input, "hello "); + assert_eq!(app.cursor_position, 6); + assert_eq!(app.kill_buffer, "world"); + } + + #[test] + fn kill_at_eol_consumes_following_newline() { + let mut app = App::new(test_options(false), &Config::default()); + app.input = "line one\nline two".to_string(); + app.cursor_position = 8; // sitting on the '\n' + assert!(app.kill_to_end_of_line()); + assert_eq!(app.input, "line oneline two"); + assert_eq!(app.cursor_position, 8); + assert_eq!(app.kill_buffer, "\n"); + + // Empty input: kill is a no-op and the buffer is untouched. + let mut empty = App::new(test_options(false), &Config::default()); + assert!(!empty.kill_to_end_of_line()); + assert!(empty.input.is_empty()); + assert!(empty.kill_buffer.is_empty()); + } + + #[test] + fn yank_inserts_kill_buffer_and_preserves_it() { + let mut app = App::new(test_options(false), &Config::default()); + app.input = "abc def".to_string(); + app.cursor_position = 4; // before 'd' + assert!(app.kill_to_end_of_line()); + assert_eq!(app.input, "abc "); + assert_eq!(app.kill_buffer, "def"); + + // Move cursor to the start and yank twice — kill_buffer must persist. + app.cursor_position = 0; + assert!(app.yank()); + assert!(app.yank()); + assert_eq!(app.input, "defdefabc "); + assert_eq!(app.cursor_position, 6); + assert_eq!(app.kill_buffer, "def"); + + // Yank with empty buffer is a no-op. + let mut empty = App::new(test_options(false), &Config::default()); + assert!(!empty.yank()); + assert!(empty.input.is_empty()); + } + + #[test] + fn kill_and_yank_handle_multibyte_utf8() { + let mut app = App::new(test_options(false), &Config::default()); + // "café 你好" — char_count = 7 (c,a,f,é, ,你,好); UTF-8 bytes differ. + app.input = "café 你好".to_string(); + app.cursor_position = 5; // before '你' + assert!(app.kill_to_end_of_line()); + assert_eq!(app.input, "café "); + assert_eq!(app.cursor_position, 5); + assert_eq!(app.kill_buffer, "你好"); + + // Yank back at the same spot — must not panic on char boundaries. + assert!(app.yank()); + assert_eq!(app.input, "café 你好"); + assert_eq!(app.cursor_position, 7); + } } diff --git a/crates/tui/src/tui/history.rs b/crates/tui/src/tui/history.rs index 4de6f4f5..8660b280 100644 --- a/crates/tui/src/tui/history.rs +++ b/crates/tui/src/tui/history.rs @@ -1188,7 +1188,7 @@ fn render_thinking( lines.push(Line::from(vec![ Span::styled("▏ ", Style::default().fg(thinking_state_accent(state))), Span::styled( - "thinking collapsed; press v for full text", + "thinking collapsed; press Ctrl+O for full text", Style::default().fg(palette::TEXT_MUTED).italic(), ), ])); @@ -1657,7 +1657,7 @@ mod tests { .iter() .flat_map(|line| line.spans.iter().map(|span| span.content.as_ref())) .collect::(); - assert!(text.contains("thinking collapsed; press v for full text")); + assert!(text.contains("thinking collapsed; press Ctrl+O for full text")); assert!(text.contains("thinking")); } diff --git a/crates/tui/src/tui/pager.rs b/crates/tui/src/tui/pager.rs index 9de3e22c..54bb538c 100644 --- a/crates/tui/src/tui/pager.rs +++ b/crates/tui/src/tui/pager.rs @@ -1,6 +1,20 @@ //! Full-screen pager overlay for long outputs. +//! +//! Vim-style key bindings (mirroring the codex pager_overlay): +//! - `j` / Down — scroll down one line +//! - `k` / Up — scroll up one line +//! - `g g` / Home — jump to top +//! - `G` / End — jump to bottom +//! - `Ctrl+D` — half-page down +//! - `Ctrl+U` — half-page up +//! - `Ctrl+F` / PageDown / Space — full page down +//! - `Ctrl+B` / PageUp / Shift+Space — full page up +//! - `/` — start search; `n` / `N` — next / previous match +//! - `q` / Esc — close pager -use crossterm::event::{KeyCode, KeyEvent}; +use std::cell::Cell; + +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use ratatui::{ buffer::Buffer, layout::Rect, @@ -13,6 +27,11 @@ use unicode_width::UnicodeWidthStr; use crate::palette; use crate::tui::views::{ModalKind, ModalView, ViewAction}; +/// Footer hint shown along the bottom border of the pager. Kept short so it +/// fits on narrow terminals; full reference lives in the module docs. +const FOOTER_HINT: &str = + " j/k scroll Space/b page Ctrl+D/U half g/G top/bottom / search q quit "; + pub struct PagerView { title: String, lines: Vec>, @@ -23,6 +42,10 @@ pub struct PagerView { search_index: usize, search_mode: bool, pending_g: bool, + /// Cached visible content height from the last render. Used by paging + /// keys (Ctrl+D/U, Ctrl+F/B, Space, etc.) to compute scroll deltas + /// without access to the render area. + last_visible_height: Cell, } impl PagerView { @@ -38,6 +61,7 @@ impl PagerView { search_index: 0, search_mode: false, pending_g: false, + last_visible_height: Cell::new(0), } } @@ -70,6 +94,30 @@ impl PagerView { self.scroll = max_scroll; } + /// Return the page height (in lines) used for paging keys. + /// + /// Falls back to a small constant (10) before the first render so the + /// pager still responds to paging keys when invoked synthetically (e.g. + /// in unit tests). After the first render, the cached value reflects + /// the actual visible content area. + fn page_height(&self) -> usize { + let cached = self.last_visible_height.get(); + if cached == 0 { 10 } else { cached } + } + + /// Half a page, rounded up so a single press always moves at least one line. + fn half_page_height(&self) -> usize { + let page = self.page_height(); + page.div_ceil(2).max(1) + } + + fn max_scroll(&self) -> usize { + // Match the existing 1-line scroll convention used by `j`/`k`. Render + // clamps `self.scroll` to `lines.len() - visible_height` for display + // purposes, so over-scrolling here is harmless. + self.lines.len().saturating_sub(1) + } + fn start_search(&mut self) { self.search_mode = true; self.search_input.clear(); @@ -157,6 +205,38 @@ impl ModalView for PagerView { } } + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + let shift = key.modifiers.contains(KeyModifiers::SHIFT); + let max_scroll = self.max_scroll(); + + // Ctrl+chord paging keys are matched first because their KeyCode + // also matches the bare `KeyCode::Char(c)` arms below. + if ctrl { + match key.code { + KeyCode::Char('d') | KeyCode::Char('D') => { + self.scroll_down(self.half_page_height(), max_scroll); + self.pending_g = false; + return ViewAction::None; + } + KeyCode::Char('u') | KeyCode::Char('U') => { + self.scroll_up(self.half_page_height()); + self.pending_g = false; + return ViewAction::None; + } + KeyCode::Char('f') | KeyCode::Char('F') => { + self.scroll_down(self.page_height(), max_scroll); + self.pending_g = false; + return ViewAction::None; + } + KeyCode::Char('b') | KeyCode::Char('B') => { + self.scroll_up(self.page_height()); + self.pending_g = false; + return ViewAction::None; + } + _ => {} + } + } + match key.code { KeyCode::Esc | KeyCode::Char('q') => ViewAction::Close, KeyCode::Up | KeyCode::Char('k') => { @@ -165,17 +245,39 @@ impl ModalView for PagerView { ViewAction::None } KeyCode::Down | KeyCode::Char('j') => { - self.scroll_down(1, self.lines.len().saturating_sub(1)); + self.scroll_down(1, max_scroll); self.pending_g = false; ViewAction::None } KeyCode::PageUp => { - self.scroll_up(10); + self.scroll_up(self.page_height()); self.pending_g = false; ViewAction::None } KeyCode::PageDown => { - self.scroll_down(10, self.lines.len().saturating_sub(1)); + self.scroll_down(self.page_height(), max_scroll); + self.pending_g = false; + ViewAction::None + } + // Vim convention: Space pages down, Shift+Space pages up. Match + // Shift+Space first so it is not absorbed by the bare ' ' arm. + KeyCode::Char(' ') if shift => { + self.scroll_up(self.page_height()); + self.pending_g = false; + ViewAction::None + } + KeyCode::Char(' ') => { + self.scroll_down(self.page_height(), max_scroll); + self.pending_g = false; + ViewAction::None + } + KeyCode::Home => { + self.scroll_to_top(); + self.pending_g = false; + ViewAction::None + } + KeyCode::End => { + self.scroll_to_bottom(max_scroll); self.pending_g = false; ViewAction::None } @@ -189,7 +291,7 @@ impl ModalView for PagerView { ViewAction::None } KeyCode::Char('G') => { - self.scroll_to_bottom(self.lines.len().saturating_sub(1)); + self.scroll_to_bottom(max_scroll); self.pending_g = false; ViewAction::None } @@ -228,6 +330,9 @@ impl ModalView for PagerView { if self.search_mode { visible_height = visible_height.saturating_sub(1); } + // Cache for paging keys; the value is treated as advisory and + // clamped at use-time. + self.last_visible_height.set(visible_height); let max_scroll = self.lines.len().saturating_sub(visible_height); let scroll = self.scroll.min(max_scroll); let end = (scroll + visible_height).min(self.lines.len()); @@ -257,8 +362,13 @@ impl ModalView for PagerView { ))); } + let footer = Line::from(Span::styled( + FOOTER_HINT, + Style::default().fg(palette::TEXT_HINT), + )); let block = Block::default() .title(self.title.clone()) + .title_bottom(footer) .borders(Borders::ALL) .border_style(Style::default().fg(palette::BORDER_COLOR)) .style(Style::default().bg(palette::DEEPSEEK_INK)) @@ -315,3 +425,211 @@ fn wrap_text(text: &str, width: usize) -> Vec { lines } + +#[cfg(test)] +mod tests { + use super::*; + use ratatui::text::Line; + + fn make_pager(lines: usize) -> PagerView { + let lines: Vec> = (0..lines) + .map(|i| Line::from(format!("line-{i:03}"))) + .collect(); + PagerView::new("T", lines) + } + + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) + } + + fn key_mod(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent::new(code, mods) + } + + fn ctrl(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::CONTROL) + } + + /// Drive a render once so `last_visible_height` is populated and paging + /// keys use a deterministic page size. + fn prime_layout(view: &mut PagerView, height: u16) { + let area = Rect::new(0, 0, 40, height); + let mut buf = Buffer::empty(area); + view.render(area, &mut buf); + } + + #[test] + fn j_scrolls_down_one_line() { + let mut p = make_pager(50); + let _ = p.handle_key(key(KeyCode::Char('j'))); + assert_eq!(p.scroll, 1); + } + + #[test] + fn k_scrolls_up_one_line() { + let mut p = make_pager(50); + p.scroll = 5; + let _ = p.handle_key(key(KeyCode::Char('k'))); + assert_eq!(p.scroll, 4); + } + + #[test] + fn gg_jumps_to_top() { + let mut p = make_pager(50); + p.scroll = 30; + let _ = p.handle_key(key(KeyCode::Char('g'))); + assert!(p.pending_g, "first 'g' should arm pending_g"); + assert_eq!(p.scroll, 30, "first 'g' alone must not scroll"); + let _ = p.handle_key(key(KeyCode::Char('g'))); + assert_eq!(p.scroll, 0); + assert!(!p.pending_g); + } + + #[test] + fn home_jumps_to_top() { + let mut p = make_pager(50); + p.scroll = 30; + let _ = p.handle_key(key(KeyCode::Home)); + assert_eq!(p.scroll, 0); + } + + #[test] + fn shift_g_jumps_to_bottom() { + let mut p = make_pager(50); + let _ = p.handle_key(key(KeyCode::Char('G'))); + assert_eq!(p.scroll, p.max_scroll()); + } + + #[test] + fn end_jumps_to_bottom() { + let mut p = make_pager(50); + let _ = p.handle_key(key(KeyCode::End)); + assert_eq!(p.scroll, p.max_scroll()); + } + + #[test] + fn ctrl_d_half_page_down() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + let half = p.half_page_height(); + assert!(half >= 1, "half-page must move at least one line"); + let _ = p.handle_key(ctrl(KeyCode::Char('d'))); + assert_eq!(p.scroll, half); + } + + #[test] + fn ctrl_u_half_page_up() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + p.scroll = 50; + let half = p.half_page_height(); + let _ = p.handle_key(ctrl(KeyCode::Char('u'))); + assert_eq!(p.scroll, 50 - half); + } + + #[test] + fn ctrl_f_full_page_down() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + let page = p.page_height(); + let _ = p.handle_key(ctrl(KeyCode::Char('f'))); + assert_eq!(p.scroll, page); + } + + #[test] + fn ctrl_b_full_page_up() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + p.scroll = 80; + let page = p.page_height(); + let _ = p.handle_key(ctrl(KeyCode::Char('b'))); + assert_eq!(p.scroll, 80 - page); + } + + #[test] + fn space_pages_down() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + let page = p.page_height(); + let _ = p.handle_key(key(KeyCode::Char(' '))); + assert_eq!(p.scroll, page); + } + + #[test] + fn shift_space_pages_up() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + p.scroll = 80; + let page = p.page_height(); + let _ = p.handle_key(key_mod(KeyCode::Char(' '), KeyModifiers::SHIFT)); + assert_eq!(p.scroll, 80 - page); + } + + #[test] + fn page_down_uses_cached_visible_height() { + let mut p = make_pager(200); + prime_layout(&mut p, 22); + let page = p.page_height(); + let _ = p.handle_key(key(KeyCode::PageDown)); + assert_eq!(p.scroll, page); + } + + #[test] + fn q_closes_pager() { + let mut p = make_pager(10); + let action = p.handle_key(key(KeyCode::Char('q'))); + assert!(matches!(action, ViewAction::Close)); + } + + #[test] + fn esc_closes_pager() { + let mut p = make_pager(10); + let action = p.handle_key(key(KeyCode::Esc)); + assert!(matches!(action, ViewAction::Close)); + } + + #[test] + fn g_does_not_consume_search_input() { + // While in search mode, 'g' must be treated as a search character, + // not as the half of a `gg` jump-to-top sequence. + let mut p = make_pager(50); + p.scroll = 10; + let _ = p.handle_key(key(KeyCode::Char('/'))); + assert!(p.search_mode); + let _ = p.handle_key(key(KeyCode::Char('g'))); + assert_eq!(p.search_input, "g"); + assert_eq!(p.scroll, 10); + } + + #[test] + fn footer_hint_includes_new_bindings() { + // The rendered pager must surface the new vim-style bindings to + // the user; check the footer string covers the headline keys. + for needle in &["j/k", "g/G", "Space", "Ctrl+D", "/ search", "q quit"] { + assert!( + FOOTER_HINT.contains(needle), + "footer hint missing {needle:?}: {FOOTER_HINT}" + ); + } + } + + #[test] + fn footer_hint_is_rendered_in_buffer() { + let p = make_pager(5); + let area = Rect::new(0, 0, 100, 10); + let mut buf = Buffer::empty(area); + p.render(area, &mut buf); + // The pager renders into an inset popup_area = (1, 1, w-2, h-2), + // so the bottom border lives at y = popup_area.bottom() - 1, not + // at the outer area's last row. + let popup_bottom_y = (area.height as usize).saturating_sub(2); + let mut bottom = String::new(); + for x in 1..area.right().saturating_sub(1) { + bottom.push_str(buf[(x, popup_bottom_y as u16)].symbol()); + } + assert!( + bottom.contains("j/k") || bottom.contains("scroll"), + "expected footer hint on bottom border row {popup_bottom_y}, got: {bottom:?}" + ); + } +} diff --git a/crates/tui/src/tui/scrolling.rs b/crates/tui/src/tui/scrolling.rs index 85fdc620..d1f7cff2 100644 --- a/crates/tui/src/tui/scrolling.rs +++ b/crates/tui/src/tui/scrolling.rs @@ -1,10 +1,30 @@ //! Scroll state tracking for transcript rendering. +//! +//! The transcript view uses a flat line-index scroll model: a single `offset` +//! into the rendered line-meta buffer points at the top visible line, with +//! `usize::MAX` reserved as a sentinel meaning "stuck to the live tail." +//! +//! Why a flat offset, not cell anchors? An earlier design anchored the +//! viewport to a `(cell_index, line_in_cell)` pair on the assumption that +//! the cell list was append-only. It is not — content rewrites (RLM `repl` +//! blocks expanding into `Thinking + Text`, tool result replacements, and +//! compaction) can renumber or remove cells underneath the user. When the +//! anchor cell vanished the viewport teleported to the bottom (issue #56) +//! or "got stuck" because the next keypress would resolve from `max_start`. +//! +//! Codex's pager uses the same line-offset shape; see +//! `codex-rs/tui/src/pager_overlay.rs::PagerView`. use std::time::{Duration, Instant}; // === Transcript Line Metadata === /// Metadata describing how rendered transcript lines map to history cells. +/// +/// The scroll state itself does not consult this — it only stores a flat +/// line offset — but other render-time helpers (selection painting, +/// send-flash, jump-to-tool, scrollbar percent) still need the +/// line→cell mapping the cache exposes. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum TranscriptLineMeta { CellLine { @@ -28,94 +48,88 @@ impl TranscriptLineMeta { } } -// === Scroll Anchors === +// === Transcript Scroll State === -/// Scroll anchor for the transcript view. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub enum TranscriptScroll { - #[default] - ToBottom, - Scrolled { - cell_index: usize, - line_in_cell: usize, - }, - ScrolledSpacerBeforeCell { - cell_index: usize, - }, +/// Sentinel offset meaning "stuck to live tail" — the renderer translates +/// this to `max_start` at draw time, so newly appended lines pull the view +/// down with them. +const TAIL_SENTINEL: usize = usize::MAX; + +/// Flat line-offset scroll state for the transcript view. +/// +/// Stores the index of the top visible line into the cache's `line_meta` +/// buffer, or [`TAIL_SENTINEL`] (`usize::MAX`) to mean "stuck to bottom." +/// The renderer resolves the sentinel against the current line count and +/// viewport height every frame, so content rewrites simply clamp the +/// user's offset rather than triggering anchor recovery heuristics. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TranscriptScroll { + offset: usize, +} + +impl Default for TranscriptScroll { + /// Default state is "stuck to live tail" — matches the historical + /// `TranscriptScroll::ToBottom` behaviour callers already depend on. + fn default() -> Self { + Self::to_bottom() + } } impl TranscriptScroll { - /// Resolve the anchor to a top line index. - /// - /// When the original anchor cell no longer exists (because content was - /// rewritten — e.g. an inline RLM `repl` block expanded into - /// `Thinking + Text`, or a tool result was replaced) we used to fall - /// straight to `ToBottom`, which felt like the view "got stuck" because - /// the user's next Up press would teleport-then-recompute from the - /// bottom. Instead, clamp to the nearest surviving cell so scroll - /// position is preserved across rewrites. + /// State that follows the live tail (default). #[must_use] - pub fn resolve_top(self, line_meta: &[TranscriptLineMeta], max_start: usize) -> (Self, usize) { - match self { - TranscriptScroll::ToBottom => (TranscriptScroll::ToBottom, max_start), - TranscriptScroll::Scrolled { - cell_index, - line_in_cell, - } => { - if let Some(idx) = anchor_index(line_meta, cell_index, line_in_cell) { - return (self, idx.min(max_start)); - } - // Fallback 1: same cell, top line — handles cases where the - // line count for a cell shrank (e.g. text was condensed). - if let Some(idx) = anchor_index(line_meta, cell_index, 0) { - return ( - TranscriptScroll::Scrolled { - cell_index, - line_in_cell: 0, - }, - idx.min(max_start), - ); - } - // Fallback 2: nearest surviving cell at or before the - // requested cell index. Walks line_meta once. - if let Some((ci, li, idx)) = nearest_cell_at_or_before(line_meta, cell_index) { - return ( - TranscriptScroll::Scrolled { - cell_index: ci, - line_in_cell: li, - }, - idx.min(max_start), - ); - } - // Last resort — there are no cell lines at all (empty - // transcript). ToBottom is fine here. - (TranscriptScroll::ToBottom, max_start) - } - TranscriptScroll::ScrolledSpacerBeforeCell { cell_index } => { - if let Some(idx) = spacer_before_cell_index(line_meta, cell_index) { - return (self, idx.min(max_start)); - } - if let Some((ci, li, idx)) = nearest_cell_at_or_before(line_meta, cell_index) { - return ( - TranscriptScroll::Scrolled { - cell_index: ci, - line_in_cell: li, - }, - idx.min(max_start), - ); - } - (TranscriptScroll::ToBottom, max_start) - } + pub const fn to_bottom() -> Self { + Self { + offset: TAIL_SENTINEL, } } - /// Apply a delta scroll and return the updated anchor. + /// State pinned to a specific line index. + #[must_use] + pub const fn at_line(offset: usize) -> Self { + Self { offset } + } + + /// Returns true when the view is following the live tail. + #[must_use] + pub const fn is_at_tail(self) -> bool { + self.offset == TAIL_SENTINEL + } + + /// Resolve the scroll state to a concrete top line index. /// - /// When the existing anchor cell is gone (content rewrite), fall back to - /// the nearest surviving cell instead of `max_start`. Without that, an - /// Up press from a missing-anchor state would resolve `current_top` to - /// the bottom and then walk up by `delta`, teleporting the user near - /// the bottom of the transcript. + /// `max_start` is `total_lines.saturating_sub(visible_lines)`. The + /// returned `Self` is the canonicalized state — if the resolved top + /// reached the tail (or the transcript fits in one screen) we collapse + /// to [`TranscriptScroll::to_bottom`], so the caller can treat the + /// returned state as authoritative. + /// + /// `line_meta` is accepted for API compatibility with the previous + /// cell-anchored implementation. It is unused here because the flat + /// offset model needs no cell-index lookup; we just clamp. + #[must_use] + pub fn resolve_top(self, line_meta: &[TranscriptLineMeta], max_start: usize) -> (Self, usize) { + let _ = line_meta; + if self.offset == TAIL_SENTINEL { + return (Self::to_bottom(), max_start); + } + let top = self.offset.min(max_start); + if top >= max_start { + (Self::to_bottom(), max_start) + } else { + (Self::at_line(top), top) + } + } + + /// Apply a scroll delta and return the updated state. + /// + /// `delta_lines` is signed: negative scrolls up (toward the start), + /// positive scrolls down (toward the tail). When the resolved offset + /// hits `max_start` we snap to [`TranscriptScroll::to_bottom`] so + /// subsequent appended content pulls the view along. + /// + /// `line_meta` is accepted for API compatibility; only its length is + /// consulted. `visible_lines` controls the page size for clamping. #[must_use] pub fn scrolled_by( self, @@ -129,28 +143,15 @@ impl TranscriptScroll { let total_lines = line_meta.len(); if total_lines <= visible_lines { - return TranscriptScroll::ToBottom; + // Whole transcript fits; only "tail" is meaningful. + return Self::to_bottom(); } let max_start = total_lines.saturating_sub(visible_lines); - let current_top = match self { - TranscriptScroll::ToBottom => max_start, - TranscriptScroll::Scrolled { - cell_index, - line_in_cell, - } => anchor_index(line_meta, cell_index, line_in_cell) - .or_else(|| anchor_index(line_meta, cell_index, 0)) - .or_else(|| nearest_cell_at_or_before(line_meta, cell_index).map(|(_, _, idx)| idx)) - .unwrap_or(max_start) - .min(max_start), - TranscriptScroll::ScrolledSpacerBeforeCell { cell_index } => { - spacer_before_cell_index(line_meta, cell_index) - .or_else(|| { - nearest_cell_at_or_before(line_meta, cell_index).map(|(_, _, idx)| idx) - }) - .unwrap_or(max_start) - .min(max_start) - } + let current_top = if self.offset == TAIL_SENTINEL { + max_start + } else { + self.offset.min(max_start) }; let new_top = if delta_lines < 0 { @@ -161,135 +162,27 @@ impl TranscriptScroll { }; if new_top >= max_start { - TranscriptScroll::ToBottom + Self::to_bottom() } else { - TranscriptScroll::anchor_for(line_meta, new_top).unwrap_or(TranscriptScroll::ToBottom) + Self::at_line(new_top) } } - /// Create an anchor from a top line index. + /// Pin the scroll state to a specific line index in the rendered + /// transcript (saturating to the meta buffer length). + /// + /// Returns `None` if `line_meta` is empty (caller should default to + /// [`TranscriptScroll::to_bottom`] in that case). #[must_use] pub fn anchor_for(line_meta: &[TranscriptLineMeta], start: usize) -> Option { if line_meta.is_empty() { return None; } - - let start = start.min(line_meta.len().saturating_sub(1)); - match line_meta[start] { - TranscriptLineMeta::CellLine { - cell_index, - line_in_cell, - } => Some(TranscriptScroll::Scrolled { - cell_index, - line_in_cell, - }), - TranscriptLineMeta::Spacer => { - if let Some((cell_index, _)) = anchor_at_or_after(line_meta, start) { - Some(TranscriptScroll::ScrolledSpacerBeforeCell { cell_index }) - } else { - anchor_at_or_before(line_meta, start).map(|(cell_index, line_in_cell)| { - TranscriptScroll::Scrolled { - cell_index, - line_in_cell, - } - }) - } - } - } + let clamped = start.min(line_meta.len().saturating_sub(1)); + Some(Self::at_line(clamped)) } } -fn anchor_index( - line_meta: &[TranscriptLineMeta], - cell_index: usize, - line_in_cell: usize, -) -> Option { - line_meta - .iter() - .enumerate() - .find_map(|(idx, entry)| match *entry { - TranscriptLineMeta::CellLine { - cell_index: ci, - line_in_cell: li, - } if ci == cell_index && li == line_in_cell => Some(idx), - _ => None, - }) -} - -fn spacer_before_cell_index(line_meta: &[TranscriptLineMeta], cell_index: usize) -> Option { - line_meta.iter().enumerate().find_map(|(idx, entry)| { - if matches!(entry, TranscriptLineMeta::Spacer) - && line_meta - .get(idx + 1) - .and_then(TranscriptLineMeta::cell_line) - .is_some_and(|(ci, _)| ci == cell_index) - { - Some(idx) - } else { - None - } - }) -} - -fn anchor_at_or_after(line_meta: &[TranscriptLineMeta], start: usize) -> Option<(usize, usize)> { - line_meta - .iter() - .enumerate() - .skip(start) - .find_map(|(_, entry)| entry.cell_line()) -} - -fn anchor_at_or_before(line_meta: &[TranscriptLineMeta], start: usize) -> Option<(usize, usize)> { - line_meta - .iter() - .enumerate() - .take(start.saturating_add(1)) - .rev() - .find_map(|(_, entry)| entry.cell_line()) -} - -/// Walk `line_meta` once and return the highest-priority surviving cell -/// position whose `cell_index <= target`. Used as a fallback when the -/// original anchor cell was removed by a content rewrite — keeps the user -/// near where they were instead of teleporting to the bottom. -/// -/// Returns `(cell_index, line_in_cell, line_meta_index)`. -fn nearest_cell_at_or_before( - line_meta: &[TranscriptLineMeta], - target: usize, -) -> Option<(usize, usize, usize)> { - let mut best: Option<(usize, usize, usize)> = None; - for (idx, entry) in line_meta.iter().enumerate() { - if let TranscriptLineMeta::CellLine { - cell_index, - line_in_cell, - } = *entry - && cell_index <= target - { - match best { - None => best = Some((cell_index, line_in_cell, idx)), - Some((bci, _, _)) if cell_index >= bci => { - best = Some((cell_index, line_in_cell, idx)); - } - _ => {} - } - } - } - // Fall back to the very first surviving cell if nothing matched. - if best.is_none() { - for (idx, entry) in line_meta.iter().enumerate() { - if let TranscriptLineMeta::CellLine { - cell_index, - line_in_cell, - } = *entry - { - return Some((cell_index, line_in_cell, idx)); - } - } - } - best -} - /// Direction for mouse scroll input. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ScrollDirection { @@ -369,124 +262,175 @@ mod tests { meta } - /// Regression test for the "stuck after content rewrite" bug from - /// issue #56. When the anchor cell still exists, scroll position is - /// preserved. + /// Default state follows the live tail. Resolving against any + /// `max_start` returns `max_start` and the canonical tail state. #[test] - fn resolve_top_keeps_position_when_anchor_cell_exists() { - let meta = synth_line_meta(5, 3); // 5 cells × 3 lines + 4 spacers = 19 entries + fn default_state_is_tail() { + let state = TranscriptScroll::default(); + assert!(state.is_at_tail()); + let meta = synth_line_meta(5, 3); + let max_start = 6; + let (resolved, top) = state.resolve_top(&meta, max_start); + assert!(resolved.is_at_tail()); + assert_eq!(top, max_start); + } + + /// A pinned offset below `max_start` resolves to itself unchanged. + /// (Originally: "anchor cell still exists" — same intent: scroll + /// position is preserved when it is still valid.) + #[test] + fn resolve_top_keeps_position_when_offset_in_range() { + let meta = synth_line_meta(5, 3); // 19 entries let max_start = meta.len().saturating_sub(8); - let anchor = TranscriptScroll::Scrolled { - cell_index: 2, - line_in_cell: 1, - }; - let (state, top) = anchor.resolve_top(&meta, max_start); - assert_eq!(state, anchor); - // Cell 2, line 1 sits at: 0,1,2 (cell 0), spacer, 4,5,6 (cell 1), - // spacer, 8,9,10 (cell 2) — line 1 of cell 2 is index 9. + let state = TranscriptScroll::at_line(9); + let (resolved, top) = state.resolve_top(&meta, max_start); + assert_eq!(resolved, TranscriptScroll::at_line(9)); assert_eq!(top, 9); } - /// Regression test for issue #56: when a content rewrite removes the - /// anchor cell, the previous behaviour was to teleport to ToBottom. - /// Now we clamp to the nearest surviving cell at-or-before the - /// requested cell index. + /// Regression for issue #56: when a content rewrite shrinks the + /// transcript so the user's offset is past the new `max_start`, we + /// clamp to the new max — we must NOT teleport to the top, and we + /// must NOT silently lose the position by sending the user to the + /// raw bottom of pre-rewrite content. Snapping to the tail is the + /// correct behaviour because the user's intended position no longer + /// has any content under it. #[test] - fn resolve_top_clamps_to_nearest_cell_when_anchor_cell_removed() { - // Original transcript had cells 0..5; a rewrite shrunk it to 0..3. - // The anchor pointed at cell 4 — that cell no longer exists. - let meta = synth_line_meta(3, 2); // cells 0..3, 2 lines each + 2 spacers - let max_start = meta.len(); - let stale_anchor = TranscriptScroll::Scrolled { - cell_index: 4, - line_in_cell: 0, - }; - let (state, top) = stale_anchor.resolve_top(&meta, max_start); - // Should clamp to the highest-indexed surviving cell (cell 2) - // rather than ToBottom. - match state { - TranscriptScroll::Scrolled { cell_index, .. } => assert_eq!(cell_index, 2), - other => panic!("expected Scrolled, got {other:?}"), - } - // top should be a valid index into meta, not max_start. - assert!(top < meta.len()); + fn resolve_top_clamps_when_offset_past_max_start() { + let meta = synth_line_meta(3, 2); // 8 entries (cells 0..3, 2 lines + 2 spacers) + let max_start = meta.len().saturating_sub(4); + // User had scrolled to a line that no longer exists post-rewrite. + let state = TranscriptScroll::at_line(15); + let (resolved, top) = state.resolve_top(&meta, max_start); + // Past max_start collapses to tail (which is the right answer: + // there is no content beyond max_start to show). + assert!(resolved.is_at_tail()); + assert_eq!(top, max_start); } - /// Same fallback behaviour applies when scrolling further by a delta: - /// scrolled_by computes its current_top from the (stale) anchor and - /// the user's keypress should still move them up rather than locking - /// them near the bottom. + /// Regression for the new bug we are guarding against in this + /// refactor: scrolling up to mid-transcript, having the content + /// rewrite under us, and then drawing again must preserve the + /// offset (clamped if needed) and NOT teleport to top or to bottom + /// when the offset is still in-range. #[test] - fn scrolled_by_does_not_teleport_on_missing_anchor() { - let meta = synth_line_meta(3, 2); + fn resolve_top_preserves_midway_offset_after_content_rewrite() { + // Pre-rewrite transcript: 10 cells × 3 lines + 9 spacers = 39 lines. + let pre = synth_line_meta(10, 3); + let visible = 8; + let pre_max_start = pre.len().saturating_sub(visible); + + // User scrolls up to a midway line (line 12). + let state = TranscriptScroll::at_line(12); + let (state, top_before) = state.resolve_top(&pre, pre_max_start); + assert_eq!(top_before, 12); + assert_eq!(state, TranscriptScroll::at_line(12)); + + // Content rewrite: cell 4 expanded by two lines (e.g. inline + // RLM `repl` block became Thinking + Text). Total grows. + let mut post = pre.clone(); + post.insert(13, cell_line(4, 3)); + post.insert(14, cell_line(4, 4)); + let post_max_start = post.len().saturating_sub(visible); + let (state2, top_after) = state.resolve_top(&post, post_max_start); + // Critical: still at line 12, not pulled to bottom or top. + assert_eq!(state2, TranscriptScroll::at_line(12)); + assert_eq!(top_after, 12); + + // Content rewrite shrunk transcript below the offset. + let post_shrunk = synth_line_meta(3, 3); // 11 lines total + let shrunk_max_start = post_shrunk.len().saturating_sub(visible); + let (state3, top_shrunk) = state.resolve_top(&post_shrunk, shrunk_max_start); + // Offset 12 > 11; we clamp to tail (no content beyond max_start). + assert!(state3.is_at_tail()); + assert_eq!(top_shrunk, shrunk_max_start); + } + + /// `scrolled_by` from a stale offset: pressing Up should still move + /// the user up, not lock them at the bottom. The flat-offset model + /// makes this trivial — the offset is simply clamped to `max_start` + /// before applying the delta. + #[test] + fn scrolled_by_does_not_teleport_on_stale_offset() { + let meta = synth_line_meta(3, 2); // 8 entries let visible = 4; - let stale_anchor = TranscriptScroll::Scrolled { - cell_index: 4, - line_in_cell: 0, - }; - // User presses Up (negative delta) from a stale anchor. - let new_state = stale_anchor.scrolled_by(-1, &meta, visible); - // Either ends up Scrolled near the top of the surviving content - // or ToBottom if the transcript fits in one screen — but the - // failure mode we're testing for is "ToBottom even though Up was - // pressed and there's room to scroll," which depends on - // total_lines > visible_lines. + let max_start = meta.len().saturating_sub(visible); + // User had scrolled past the new end of transcript. + let stale = TranscriptScroll::at_line(20); + let new_state = stale.scrolled_by(-1, &meta, visible); + // Either ends up Scrolled near the bottom (max_start - 1) or + // already at tail if max_start was 0. if meta.len() > visible { - assert!(matches!(new_state, TranscriptScroll::Scrolled { .. })); + // Should be at max_start - 1 = 3. + assert_eq!(new_state, TranscriptScroll::at_line(max_start - 1)); } } - /// When the transcript fits entirely in the viewport, the scroll - /// state collapses to ToBottom regardless of where the anchor was. + /// When the transcript fits entirely in the viewport, scrolled_by + /// always collapses to tail. #[test] fn scrolled_by_collapses_to_bottom_when_view_fits() { let meta = synth_line_meta(2, 2); let visible = meta.len() + 5; - let anchor = TranscriptScroll::Scrolled { - cell_index: 0, - line_in_cell: 0, - }; - let new_state = anchor.scrolled_by(-1, &meta, visible); - assert_eq!(new_state, TranscriptScroll::ToBottom); + let state = TranscriptScroll::at_line(0); + let new_state = state.scrolled_by(-1, &meta, visible); + assert!(new_state.is_at_tail()); } - /// `nearest_cell_at_or_before` returns the highest cell_index that - /// is still ≤ the requested target. + /// `scrolled_by` from tail with positive delta stays at tail (we + /// can't scroll past the bottom). #[test] - fn nearest_cell_at_or_before_picks_highest_surviving() { - let meta = synth_line_meta(4, 1); // cells 0..4, one line each + spacers - let result = nearest_cell_at_or_before(&meta, 10); - let (cell_index, line_in_cell, _) = result.expect("a cell should match"); - assert_eq!(cell_index, 3); - assert_eq!(line_in_cell, 0); + fn scrolled_by_from_tail_down_stays_at_tail() { + let meta = synth_line_meta(5, 3); + let visible = 6; + let state = TranscriptScroll::to_bottom(); + let new_state = state.scrolled_by(5, &meta, visible); + assert!(new_state.is_at_tail()); } - /// And falls back to the very first surviving cell when target is - /// below all surviving cells (shouldn't happen in practice but the - /// helper guards against it). + /// `scrolled_by` from tail with negative delta moves up by |delta| + /// from `max_start`. #[test] - fn nearest_cell_at_or_before_falls_back_to_first_when_target_too_low() { - let mut meta = synth_line_meta(0, 0); - meta.push(cell_line(5, 0)); - meta.push(cell_line(6, 0)); - let result = nearest_cell_at_or_before(&meta, 2); - let (cell_index, _, _) = result.expect("falls back to first cell"); - assert_eq!(cell_index, 5); + fn scrolled_by_from_tail_up_walks_back_from_max_start() { + let meta = synth_line_meta(5, 3); // 19 entries + let visible = 6; + let max_start = meta.len().saturating_sub(visible); + let state = TranscriptScroll::to_bottom(); + let new_state = state.scrolled_by(-3, &meta, visible); + assert_eq!(new_state, TranscriptScroll::at_line(max_start - 3)); } - /// Empty line_meta returns None — caller falls through to ToBottom. + /// `anchor_for` clamps the requested start into the meta range and + /// produces a pinned state. #[test] - fn nearest_cell_at_or_before_empty_returns_none() { + fn anchor_for_clamps_start_into_range() { + let meta = synth_line_meta(4, 1); + let anchor = TranscriptScroll::anchor_for(&meta, 0).expect("non-empty"); + assert_eq!(anchor, TranscriptScroll::at_line(0)); + + let anchor = TranscriptScroll::anchor_for(&meta, 1_000_000).expect("non-empty"); + assert_eq!( + anchor, + TranscriptScroll::at_line(meta.len().saturating_sub(1)) + ); + } + + /// Empty `line_meta` returns `None` so callers can fall back to + /// [`TranscriptScroll::to_bottom`]. + #[test] + fn anchor_for_empty_returns_none() { let meta: Vec = Vec::new(); - assert!(nearest_cell_at_or_before(&meta, 0).is_none()); + assert!(TranscriptScroll::anchor_for(&meta, 0).is_none()); } + /// Tail state resolves to `max_start` regardless of the `line_meta` + /// contents. #[test] - fn to_bottom_anchor_resolves_to_max_start() { + fn to_bottom_resolves_to_max_start() { let meta = synth_line_meta(5, 2); let max_start = 7; - let (state, top) = TranscriptScroll::ToBottom.resolve_top(&meta, max_start); - assert_eq!(state, TranscriptScroll::ToBottom); + let (state, top) = TranscriptScroll::to_bottom().resolve_top(&meta, max_start); + assert!(state.is_at_tail()); assert_eq!(top, max_start); } } diff --git a/crates/tui/src/tui/transcript.rs b/crates/tui/src/tui/transcript.rs index c419b0aa..3a878d8a 100644 --- a/crates/tui/src/tui/transcript.rs +++ b/crates/tui/src/tui/transcript.rs @@ -1,4 +1,20 @@ //! Cached transcript rendering for the TUI. +//! +//! ## Per-cell revision caching +//! +//! Naive caching invalidates the whole transcript whenever ANY cell mutates. +//! During streaming the assistant content cell mutates on every delta — that +//! would force a re-wrap of every cell on every chunk. Codex avoids this by +//! tracking a per-cell revision counter; we mirror that pattern here. +//! +//! Each cell index has a paired `revision: u64`. The cache stores +//! `Vec` with `(cell_index, revision, lines, line_meta)`. On +//! `ensure`, walk the cells; if a cell's current `revision` matches the cached +//! one (and width/options haven't changed), reuse the rendered lines. +//! Otherwise re-render that cell only and reassemble. +//! +//! Width or render-option changes still bust the entire cache (correct: wrap +//! layout depends on width and which cells are visible at all). use ratatui::text::Line; @@ -6,13 +22,39 @@ use crate::tui::app::TranscriptSpacing; use crate::tui::history::{HistoryCell, TranscriptRenderOptions}; use crate::tui::scrolling::TranscriptLineMeta; +/// Per-cell cached render output. Reused across `ensure` calls when the +/// upstream cell's revision counter hasn't changed. +#[derive(Debug, Clone)] +struct CachedCell { + /// Revision the cell was at when the lines/meta were rendered. + revision: u64, + /// Rendered lines for this cell (without trailing inter-cell spacers). + lines: Vec>, + /// Whether this cell's rendered output was empty (e.g. Thinking hidden). + /// Cached so we can skip empty cells without re-rendering. + is_empty: bool, + /// Whether this cell is a stream continuation. Determines spacer rules. + /// Cached because `is_stream_continuation` is cheap but reading via the + /// cache lets us decide spacers without touching the cell. + is_stream_continuation: bool, + /// Whether this cell is conversational (User/Assistant/Thinking). Used + /// for spacer calculations. + is_conversational: bool, + /// Whether this cell is a System or Tool cell (affects spacer rules). + is_system_or_tool: bool, +} + /// Cache of rendered transcript lines for the current viewport. #[derive(Debug)] pub struct TranscriptViewCache { width: u16, - version: u64, options: TranscriptRenderOptions, + /// Per-cell rendered output, indexed by current cell position. + /// Length always equals the cell count seen on the last `ensure` call. + per_cell: Vec, + /// Flattened lines reassembled from `per_cell` plus spacers. lines: Vec>, + /// Per-line metadata aligned with `lines`. line_meta: Vec, } @@ -22,46 +64,110 @@ impl TranscriptViewCache { pub fn new() -> Self { Self { width: 0, - version: 0, options: TranscriptRenderOptions::default(), + per_cell: Vec::new(), lines: Vec::new(), line_meta: Vec::new(), } } - /// Ensure cached lines match the provided cells/width/version. + /// Ensure cached lines match the provided cells/widths/per-cell revisions. + /// + /// Reuses rendered lines for cells whose `cell_revisions[i]` matches the + /// previously cached revision (when the cell shape — empty/spacer flags — + /// also matches). Width or option changes bust the entire cache. + /// + /// `cell_revisions.len()` is expected to equal `cells.len()`. If they + /// disagree (shouldn't happen in normal use) the cache treats every cell + /// as dirty. pub fn ensure( &mut self, cells: &[HistoryCell], + cell_revisions: &[u64], width: u16, - version: u64, options: TranscriptRenderOptions, ) { - if self.width == width && self.version == version && self.options == options { - return; + let layout_changed = self.width != width || self.options != options; + if layout_changed { + self.per_cell.clear(); } self.width = width; - self.version = version; self.options = options; - let mut lines = Vec::new(); - let mut meta = Vec::new(); + // Track whether anything actually changed; if all cells are reused at + // the same indices, we can skip the reflatten. + let mut any_dirty = layout_changed || self.per_cell.len() != cells.len(); - for (cell_index, cell) in cells.iter().enumerate() { - let cell_lines = cell.lines_with_options(width, options); - if cell_lines.is_empty() { + let mut new_per_cell: Vec = Vec::with_capacity(cells.len()); + let revisions_match = cell_revisions.len() == cells.len(); + + for (idx, cell) in cells.iter().enumerate() { + let current_rev = if revisions_match { + cell_revisions[idx] + } else { + // No matching revisions — force a re-render this cycle. + u64::MAX + }; + + // Reuse cached entry if the revision matches AND it's at the same + // index (since cells can shift on insert/remove, we only reuse + // when the index is identical — a stricter invariant codex also + // uses for its active-cell tail). + if let Some(prev) = self.per_cell.get(idx) + && !layout_changed + && prev.revision == current_rev + && revisions_match + { + new_per_cell.push(prev.clone()); continue; } - for (line_in_cell, line) in cell_lines.into_iter().enumerate() { - lines.push(line); + + any_dirty = true; + let rendered = cell.lines_with_options(width, options); + let is_empty = rendered.is_empty(); + new_per_cell.push(CachedCell { + revision: current_rev, + lines: rendered, + is_empty, + is_stream_continuation: cell.is_stream_continuation(), + is_conversational: cell.is_conversational(), + is_system_or_tool: matches!( + cell, + HistoryCell::System { .. } | HistoryCell::Tool(_) + ), + }); + } + + self.per_cell = new_per_cell; + + if !any_dirty { + // All cells reused at the same indices: nothing to reflatten. + // (Width didn't change either, since that bumps `layout_changed`.) + return; + } + + self.flatten(options.spacing); + } + + /// Reassemble flat `lines` / `line_meta` from `per_cell` plus spacers. + fn flatten(&mut self, spacing: TranscriptSpacing) { + let mut lines = Vec::with_capacity(self.lines.capacity()); + let mut meta = Vec::with_capacity(self.line_meta.capacity()); + + for (cell_index, cached) in self.per_cell.iter().enumerate() { + if cached.is_empty { + continue; + } + for (line_in_cell, line) in cached.lines.iter().enumerate() { + lines.push(line.clone()); meta.push(TranscriptLineMeta::CellLine { cell_index, line_in_cell, }); } - if let Some(next_cell) = cells.get(cell_index + 1) { - let spacer_rows = spacer_rows_between(cell, next_cell, options.spacing); + if let Some(next) = self.per_cell.get(cell_index + 1) { + let spacer_rows = spacer_rows_between(cached, next, spacing); for _ in 0..spacer_rows { lines.push(Line::from("")); meta.push(TranscriptLineMeta::Spacer); @@ -93,11 +199,11 @@ impl TranscriptViewCache { } fn spacer_rows_between( - current: &HistoryCell, - next: &HistoryCell, + current: &CachedCell, + next: &CachedCell, spacing: TranscriptSpacing, ) -> usize { - if current.is_stream_continuation() { + if current.is_stream_continuation { return 0; } @@ -108,17 +214,301 @@ fn spacer_rows_between( }; let secondary_gap = match spacing { TranscriptSpacing::Compact => 0, - TranscriptSpacing::Comfortable => 1, - TranscriptSpacing::Spacious => 1, + TranscriptSpacing::Comfortable | TranscriptSpacing::Spacious => 1, }; - if current.is_conversational() && next.is_conversational() { + if current.is_conversational && next.is_conversational { conversational_gap - } else if matches!(current, HistoryCell::System { .. } | HistoryCell::Tool(_)) - || matches!(next, HistoryCell::System { .. } | HistoryCell::Tool(_)) - { + } else if current.is_system_or_tool || next.is_system_or_tool { secondary_gap } else { 0 } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::tui::history::HistoryCell; + + fn user_cell(content: &str) -> HistoryCell { + HistoryCell::User { + content: content.to_string(), + } + } + + fn assistant_cell(content: &str, streaming: bool) -> HistoryCell { + HistoryCell::Assistant { + content: content.to_string(), + streaming, + } + } + + #[test] + fn cache_reuses_cells_when_revision_unchanged() { + let cells = vec![ + user_cell("hello"), + assistant_cell("world", false), + user_cell("again"), + ]; + let revisions = vec![1u64, 1, 1]; + + let mut cache = TranscriptViewCache::new(); + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + let first_lines: Vec = cache + .lines() + .iter() + .map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect()) + .collect(); + let first_total = cache.total_lines(); + assert!(first_total > 0, "expected non-empty render"); + + // Capture per-cell lines snapshot to verify reuse. + let snapshot_per_cell: Vec> = cache + .per_cell + .iter() + .map(|c| { + c.lines + .iter() + .map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect()) + .collect() + }) + .collect(); + + // Same revisions => everything reused, output identical. + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + let second_lines: Vec = cache + .lines() + .iter() + .map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect()) + .collect(); + assert_eq!(first_lines, second_lines); + assert_eq!(cache.total_lines(), first_total); + + let snapshot_per_cell_2: Vec> = cache + .per_cell + .iter() + .map(|c| { + c.lines + .iter() + .map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect()) + .collect() + }) + .collect(); + assert_eq!(snapshot_per_cell, snapshot_per_cell_2); + } + + #[test] + fn bumping_one_cell_revision_only_rerenders_that_cell() { + // Track render counts per cell using a custom HistoryCell wrapper + // would require trait changes; instead, we detect reuse by inspecting + // CachedCell instances. After a bump, only the bumped cell's stored + // revision should differ from before; others remain identical. + + let cells_v1 = vec![ + user_cell("hello"), + assistant_cell("hi", true), + user_cell("again"), + ]; + let revs_v1 = vec![1u64, 1, 1]; + + let mut cache = TranscriptViewCache::new(); + cache.ensure(&cells_v1, &revs_v1, 80, TranscriptRenderOptions::default()); + + // Snapshot the cached lines for cells 0 and 2 (unchanged across the + // delta). + let cell0_lines_before = cache.per_cell[0] + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.to_string()) + .collect::() + }) + .collect::>(); + let cell2_lines_before = cache.per_cell[2] + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.to_string()) + .collect::() + }) + .collect::>(); + + // Mutate cell 1 (assistant streaming delta) and bump only its rev. + let cells_v2 = vec![ + user_cell("hello"), + assistant_cell("hi world", true), + user_cell("again"), + ]; + let revs_v2 = vec![1u64, 2, 1]; + + cache.ensure(&cells_v2, &revs_v2, 80, TranscriptRenderOptions::default()); + + // Cells 0 and 2 are byte-identical (proving reuse path didn't corrupt). + let cell0_lines_after = cache.per_cell[0] + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.to_string()) + .collect::() + }) + .collect::>(); + let cell2_lines_after = cache.per_cell[2] + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.to_string()) + .collect::() + }) + .collect::>(); + assert_eq!(cell0_lines_before, cell0_lines_after); + assert_eq!(cell2_lines_before, cell2_lines_after); + + // Cell 1 reflects the new content. + // The renderer interleaves role/whitespace spans, so the joined + // content has internal padding (e.g. "Assistant hi world"). + // Check for the new tokens individually rather than a literal + // "hi world" substring. + let cell1_after: String = cache.per_cell[1] + .lines + .iter() + .flat_map(|l| l.spans.iter().map(|s| s.content.to_string())) + .collect::>() + .join(" "); + assert!( + cell1_after.contains("hi") && cell1_after.contains("world"), + "cell1 should re-render with new content; got: {cell1_after}" + ); + + // Revisions in cache reflect the bump. + assert_eq!(cache.per_cell[0].revision, 1); + assert_eq!(cache.per_cell[1].revision, 2); + assert_eq!(cache.per_cell[2].revision, 1); + } + + #[test] + fn width_change_rerenders_all_cells() { + let cells = vec![ + user_cell("a fairly long message that may wrap at narrow widths"), + assistant_cell("another long message body content", false), + ]; + let revisions = vec![5u64, 7]; + + let mut cache = TranscriptViewCache::new(); + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + let wide_total = cache.total_lines(); + + // Narrow width should change layout — everything re-renders. + cache.ensure(&cells, &revisions, 20, TranscriptRenderOptions::default()); + let narrow_total = cache.total_lines(); + + assert_ne!( + wide_total, narrow_total, + "narrow width should produce a different number of lines" + ); + + // Restoring the original width re-renders again. + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + assert_eq!(cache.total_lines(), wide_total); + } + + #[test] + fn streaming_assistant_only_rebuilds_one_cell_render_count() { + // Verify behavior 6: when one Assistant cell streams a delta, only + // that one cell is re-rendered. We use a counting wrapper hooked into + // a custom History setup. Since `lines_with_options` is on `HistoryCell` + // (concrete enum), we can't mock it directly. Instead we verify the + // cache's invariant: cells with unchanged revisions retain their + // previous CachedCell entries (clone-equal), proving no re-render + // happened for them. + // + // We do this by storing revisions as monotonic u64 and verifying that + // a `Vec` snapshot of `per_cell.revision` only differs at the + // index that was bumped. + + let mut cells: Vec = + (0..50).map(|i| user_cell(&format!("cell {i}"))).collect(); + cells.push(assistant_cell("streaming", true)); + let mut revisions: Vec = vec![1; 51]; + + let mut cache = TranscriptViewCache::new(); + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + + // Snapshot total bytes rendered for cells 0..50 (unchanged). + let stable_snapshot: Vec = cache.per_cell[..50] + .iter() + .map(|c| { + c.lines + .iter() + .flat_map(|l| l.spans.iter().map(|s| s.content.to_string())) + .collect::>() + .join("|") + }) + .collect(); + + // Stream 10 deltas to the assistant cell, bumping only its revision. + for i in 0..10 { + if let HistoryCell::Assistant { content, .. } = &mut cells[50] { + content.push_str(&format!(" delta-{i}")); + } + revisions[50] += 1; + cache.ensure(&cells, &revisions, 80, TranscriptRenderOptions::default()); + + // After every delta, cells 0..50 must be byte-identical to the + // initial render. If we re-rendered them we'd observe identical + // bytes anyway (deterministic), but the test ALSO checks the + // CachedCell.revision values stayed at 1 — meaning the cache + // never replaced them, only reused them. + let stable_now: Vec = cache.per_cell[..50] + .iter() + .map(|c| { + c.lines + .iter() + .flat_map(|l| l.spans.iter().map(|s| s.content.to_string())) + .collect::>() + .join("|") + }) + .collect(); + assert_eq!( + stable_now, stable_snapshot, + "stable cells diverged at delta {i}" + ); + + for (idx, c) in cache.per_cell[..50].iter().enumerate() { + assert_eq!( + c.revision, 1, + "cell {idx} revision changed during streaming delta" + ); + } + } + } + + #[test] + fn missing_revisions_falls_back_to_full_render() { + // If callers pass a `cell_revisions` slice with the wrong length + // (shouldn't happen, but be defensive), the cache should still + // produce correct output rather than panic or skip cells. + let cells = vec![user_cell("a"), assistant_cell("b", false)]; + let bogus_revisions = vec![1u64]; // wrong length + + let mut cache = TranscriptViewCache::new(); + cache.ensure( + &cells, + &bogus_revisions, + 80, + TranscriptRenderOptions::default(), + ); + + // Both cells were rendered (no panic, output non-empty). + assert_eq!(cache.per_cell.len(), 2); + assert!(!cache.lines().is_empty()); + } +} diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index e3da9ef3..20f5ae6e 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -521,6 +521,10 @@ async fn run_event_loop( app.offline_mode = false; app.streaming_state.reset(); app.turn_started_at = None; + // Stream lock applies per-turn; clear it so the next + // turn's chunks pull the view down again until the + // user opts out by scrolling up. + app.user_scrolled_during_stream = false; app.runtime_turn_status = Some(match status { crate::core::events::TurnOutcomeStatus::Completed => { "completed".to_string() @@ -1058,6 +1062,13 @@ async fn run_event_loop( } if key.code == KeyCode::Char('k') && key.modifiers.contains(KeyModifiers::CONTROL) { + // When the composer is the active input target (no modal/pager + // intercepting keys), Ctrl+K performs an emacs-style kill to + // end-of-line. If the kill is a no-op (cursor at end of empty + // input), fall through to the existing command palette. + if app.view_stack.is_empty() && app.kill_to_end_of_line() { + continue; + } app.view_stack .push(CommandPaletteView::new(build_command_palette_entries( &app.skills_dir, @@ -1125,6 +1136,11 @@ async fn run_event_loop( { continue; } + KeyCode::Char('o') + if key.modifiers == KeyModifiers::CONTROL && open_thinking_pager(app) => + { + continue; + } KeyCode::Char('1') if key.modifiers.contains(KeyModifiers::ALT) => { if key.modifiers.contains(KeyModifiers::CONTROL) { app.set_sidebar_focus(SidebarFocus::Plan); @@ -1401,6 +1417,11 @@ async fn run_event_loop( KeyCode::Char('u') if key.modifiers.contains(KeyModifiers::CONTROL) => { app.clear_input(); } + KeyCode::Char('y') if key.modifiers.contains(KeyModifiers::CONTROL) => { + // Emacs-style yank from the kill buffer at the cursor. + // No-op when the buffer is empty. + app.yank(); + } KeyCode::Char('x') if key.modifiers.contains(KeyModifiers::CONTROL) => { let new_mode = match app.mode { AppMode::Plan => AppMode::Agent, @@ -3929,7 +3950,7 @@ fn handle_mouse_event(app: &mut App, mouse: MouseEvent) { app.transcript_selection.dragging = true; if app.is_loading - && matches!(app.transcript_scroll, TranscriptScroll::ToBottom) + && app.transcript_scroll.is_at_tail() && let Some(anchor) = TranscriptScroll::anchor_for( app.transcript_cache.line_meta(), app.last_transcript_top, @@ -4083,6 +4104,61 @@ fn open_pager_for_last_message(app: &mut App) -> bool { true } +/// Open a pager showing the full thinking block. Targets the cell at the +/// current selection if it's a Thinking cell; otherwise falls back to the +/// most recent Thinking cell in history. Bound to Ctrl+O so users can read +/// reasoning content that's been collapsed in calm-mode rendering. +fn open_thinking_pager(app: &mut App) -> bool { + let selected_cell = app + .transcript_selection + .ordered_endpoints() + .and_then(|(start, _)| { + app.transcript_cache + .line_meta() + .get(start.line_index) + .and_then(|meta| meta.cell_line()) + .map(|(cell_index, _)| cell_index) + }) + .filter(|&idx| { + matches!( + app.history.get(idx), + Some(crate::tui::history::HistoryCell::Thinking { .. }) + ) + }); + + let target_idx = selected_cell.or_else(|| { + app.history + .iter() + .enumerate() + .rev() + .find_map(|(idx, cell)| { + if matches!(cell, crate::tui::history::HistoryCell::Thinking { .. }) { + Some(idx) + } else { + None + } + }) + }); + + let Some(idx) = target_idx else { + app.status_message = Some("No thinking blocks to expand".to_string()); + return true; + }; + + let cell = &app.history[idx]; + let width = app + .last_transcript_area + .map(|area| area.width) + .unwrap_or(80); + let text = history_cell_to_text(cell, width); + app.view_stack.push(PagerView::from_text( + "Thinking", + &text, + width.saturating_sub(2), + )); + true +} + fn open_tool_details_pager(app: &mut App) -> bool { let target_cell = if let Some((start, _)) = app.transcript_selection.ordered_endpoints() { app.transcript_cache diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index ddd9aa66..9540a9ec 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -909,10 +909,11 @@ fn jump_to_adjacent_tool_cell_finds_next_and_previous() { })), ]; app.mark_history_updated(); + let cell_revisions = vec![app.history_version; app.history.len()]; app.transcript_cache.ensure( &app.history, + &cell_revisions, 100, - app.history_version, app.transcript_render_options(), ); @@ -921,10 +922,11 @@ fn jump_to_adjacent_tool_cell_finds_next_and_previous() { &mut app, SearchDirection::Forward )); - assert!(matches!( - app.transcript_scroll, - TranscriptScroll::Scrolled { .. } - )); + // Forward jump pins the scroll to a non-tail line offset (the tool + // cell's first line). Anything below the live tail is acceptable — + // the previous assertion checked `TranscriptScroll::Scrolled { .. }`, + // which under the new flat-offset model means "not at tail." + assert!(!app.transcript_scroll.is_at_tail()); app.last_transcript_top = app.transcript_cache.total_lines().saturating_sub(1); assert!(jump_to_adjacent_tool_cell( diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 747e86c7..9eeb9a7f 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -10,7 +10,7 @@ use crate::palette; use crate::tui::app::{App, AppMode, ComposerDensity}; use crate::tui::approval::{ApprovalRequest, ElevationOption, ElevationRequest, ToolCategory}; use crate::tui::history::HistoryCell; -use crate::tui::scrolling::{TranscriptLineMeta, TranscriptScroll}; +use crate::tui::scrolling::TranscriptLineMeta; use crate::{commands, config::COMMON_DEEPSEEK_MODELS}; use ratatui::{ buffer::Buffer, @@ -62,10 +62,16 @@ impl ChatWidget { }; } + // The transcript cache exposes a per-cell revision slice for fine + // grained caching; until App tracks per-cell revisions explicitly, + // we synthesize a uniform slice from the global history_version so + // any mutation invalidates every cell (matches the pre-cache + // semantics). + let cell_revisions = vec![app.history_version; app.history.len()]; app.transcript_cache.ensure( &app.history, + &cell_revisions, content_area.width.max(1), - app.history_version, render_options, ); @@ -85,6 +91,14 @@ impl ChatWidget { let max_start = total_lines.saturating_sub(visible_lines); let (scroll_state, top) = app.transcript_scroll.resolve_top(line_meta, max_start); app.transcript_scroll = scroll_state; + // If the user scrolled back to the live tail, the per-stream + // "leave me alone" lock is over — new chunks should pin to bottom + // again until they explicitly scroll up. Without this clear, content + // piles up off-screen below the visible area and the view appears + // frozen at the moment they returned to bottom. + if app.transcript_scroll.is_at_tail() { + app.user_scrolled_during_stream = false; + } app.last_transcript_area = Some(content_area); app.last_transcript_top = top; @@ -112,7 +126,7 @@ impl ChatWidget { apply_selection(&mut lines, top, app); - if matches!(app.transcript_scroll, TranscriptScroll::ToBottom) { + if app.transcript_scroll.is_at_tail() { app.last_transcript_padding_top = visible_lines.saturating_sub(lines.len()); pad_lines_to_bottom(&mut lines, visible_lines); }