From 5e4850626d9c5e0857afb9678b9a8643ee392f29 Mon Sep 17 00:00:00 2001 From: yishuiliunian Date: Mon, 6 Apr 2026 11:53:30 +0800 Subject: [PATCH] fix(tui): replace timing-based scroll debounce with batch detection and absolute position indexing (#85) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 30ms async timer debounce had race conditions between ArrowDebounceTimeout and event batch processing, causing scroll/history misfires. The tail-window rendering model drifted during output because window_size depended on scroll_offset, creating a feedback loop. Changes: - Delete scroll_debounce.rs (197 lines) and all timer machinery - Batch detection in tui_loop: ≥2 arrows in one batch = mouse wheel → scroll; 1 arrow = keyboard → history. No timers, no races. - ContentScroll struct encapsulates offset + prev_total + LineCache with reset()/to_bottom()/scroll_up()/scroll_down() methods - Absolute position indexing via slice() replaces tail() feedback loop; offset auto-compensated on content growth while pinned - E2E regression tests for both bugs + unit tests for compensation logic --- crates/loopal-tui/src/app/mod.rs | 16 +- crates/loopal-tui/src/event.rs | 2 - crates/loopal-tui/src/input/actions.rs | 2 - crates/loopal-tui/src/input/mod.rs | 32 +-- crates/loopal-tui/src/input/navigation.rs | 2 - .../loopal-tui/src/input/scroll_debounce.rs | 197 ------------------ crates/loopal-tui/src/key_dispatch.rs | 16 +- crates/loopal-tui/src/key_dispatch_ops.rs | 3 +- crates/loopal-tui/src/render.rs | 8 +- crates/loopal-tui/src/tui_loop.rs | 38 +++- .../src/views/progress/content_scroll.rs | 125 +++++++++++ .../src/views/progress/line_cache.rs | 10 +- crates/loopal-tui/src/views/progress/mod.rs | 55 +---- crates/loopal-tui/tests/suite.rs | 4 + crates/loopal-tui/tests/suite/app_test.rs | 2 +- .../loopal-tui/tests/suite/e2e_scroll_test.rs | 173 +++++++++++++++ .../tests/suite/input_scroll_edge_test.rs | 87 +++----- .../tests/suite/input_scroll_test.rs | 85 ++++---- .../loopal-tui/tests/suite/line_cache_test.rs | 14 +- .../tests/suite/scroll_burst_test.rs | 187 ++++++++--------- .../tests/suite/scroll_compensation_test.rs | 147 +++++++++++++ 21 files changed, 657 insertions(+), 548 deletions(-) delete mode 100644 crates/loopal-tui/src/input/scroll_debounce.rs create mode 100644 crates/loopal-tui/src/views/progress/content_scroll.rs create mode 100644 crates/loopal-tui/tests/suite/e2e_scroll_test.rs create mode 100644 crates/loopal-tui/tests/suite/scroll_compensation_test.rs diff --git a/crates/loopal-tui/src/app/mod.rs b/crates/loopal-tui/src/app/mod.rs index 1d2b82e7..6c1f67d7 100644 --- a/crates/loopal-tui/src/app/mod.rs +++ b/crates/loopal-tui/src/app/mod.rs @@ -14,8 +14,7 @@ use loopal_session::SessionController; use loopal_tool_background::BackgroundTaskStore; use crate::command::CommandRegistry; -use crate::input::scroll_debounce::ArrowDebounce; -use crate::views::progress::LineCache; +use crate::views::progress::ContentScroll; /// Main application state — UI-only fields + session controller handle. pub struct App { @@ -23,7 +22,6 @@ pub struct App { pub exiting: bool, pub input: String, pub input_cursor: usize, - pub scroll_offset: u16, pub input_history: Vec, pub history_index: Option, /// Images attached to the current input (pending submit). @@ -50,8 +48,6 @@ pub struct App { pub focused_bg_task: Option, /// Which UI region owns keyboard focus. pub focus_mode: FocusMode, - /// Arrow-key debounce state for mouse-wheel vs keyboard detection. - pub(crate) arrow_debounce: ArrowDebounce, /// Scroll offset for the agent panel (index of first visible agent). pub agent_panel_offset: usize, @@ -63,8 +59,8 @@ pub struct App { // === Session Controller (observable + interactive) === pub session: SessionController, - // === Render optimization === - pub line_cache: LineCache, + // === Content area scroll + render state === + pub content_scroll: ContentScroll, } impl App { @@ -82,7 +78,6 @@ impl App { exiting: false, input: String::new(), input_cursor: 0, - scroll_offset: 0, input_history: Vec::new(), history_index: None, pending_images: Vec::new(), @@ -97,12 +92,11 @@ impl App { focused_agent: None, focused_bg_task: None, focus_mode: FocusMode::default(), - arrow_debounce: ArrowDebounce::default(), agent_panel_offset: 0, bg_store: BackgroundTaskStore::new(), bg_snapshots: Vec::new(), session, - line_cache: LineCache::new(), + content_scroll: ContentScroll::new(), } } @@ -123,7 +117,7 @@ impl App { } self.input_cursor = 0; self.input_scroll = 0; - self.scroll_offset = 0; + self.content_scroll.to_bottom(); Some(UserContent { text, images, diff --git a/crates/loopal-tui/src/event.rs b/crates/loopal-tui/src/event.rs index 479fd411..2b4b3ab3 100644 --- a/crates/loopal-tui/src/event.rs +++ b/crates/loopal-tui/src/event.rs @@ -17,8 +17,6 @@ pub enum AppEvent { Paste(PasteResult), /// Tick for periodic UI refresh Tick, - /// Arrow-key debounce timer expired — flush pending arrow as history - ArrowDebounceTimeout, } /// Merges crossterm terminal events with agent events into a single stream. diff --git a/crates/loopal-tui/src/input/actions.rs b/crates/loopal-tui/src/input/actions.rs index 4f0d3c43..4af61dc8 100644 --- a/crates/loopal-tui/src/input/actions.rs +++ b/crates/loopal-tui/src/input/actions.rs @@ -64,6 +64,4 @@ pub enum InputAction { QuestionCancel, /// User pressed Ctrl+V — caller should spawn async clipboard read PasteRequested, - /// Arrow key deferred — start 30 ms debounce timer for scroll detection - StartArrowDebounce, } diff --git a/crates/loopal-tui/src/input/mod.rs b/crates/loopal-tui/src/input/mod.rs index ce13486c..bcdaf07e 100644 --- a/crates/loopal-tui/src/input/mod.rs +++ b/crates/loopal-tui/src/input/mod.rs @@ -6,7 +6,6 @@ mod modal; pub(crate) mod multiline; mod navigation; pub(crate) mod paste; -pub(crate) mod scroll_debounce; mod status_page_keys; mod sub_page; mod sub_page_rewind; @@ -21,22 +20,18 @@ use editing::{handle_backspace, handle_ctrl_c, handle_enter}; use navigation::{ DEFAULT_WRAP_WIDTH, handle_down, handle_esc, handle_up, move_cursor_left, move_cursor_right, }; -use scroll_debounce::{ScrollDirection, handle_arrow_with_debounce, resolve_pending_arrow}; /// Process a key event and update the app's input state. pub fn handle_key(app: &mut App, key: KeyEvent) -> InputAction { if let Some(action) = modal::handle_modal_keys(app, &key) { - scroll_debounce::discard_pending(app); return action; } if let Some(action) = handle_global_keys(app, &key) { - scroll_debounce::discard_pending(app); return action; } if app.autocomplete.is_some() && let Some(action) = handle_autocomplete_key(app, &key) { - scroll_debounce::discard_pending(app); return action; } @@ -45,14 +40,6 @@ pub fn handle_key(app: &mut App, key: KeyEvent) -> InputAction { action } -/// Flush any pending arrow-key debounce as history navigation. -/// -/// Called by the event loop when the 30 ms debounce timer expires and by -/// tests to simulate the timeout deterministically. -pub fn resolve_arrow_debounce(app: &mut App) { - scroll_debounce::resolve_pending_arrow(app); -} - /// Handle global shortcuts: Ctrl combos, Shift+Tab. fn handle_global_keys(app: &mut App, key: &KeyEvent) -> Option { if key.modifiers.contains(KeyModifiers::CONTROL) { @@ -123,12 +110,7 @@ fn handle_panel_key(app: &mut App, key: &KeyEvent) -> InputAction { /// Keys in Input mode: typing, navigation, submit. fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { - // Flush any pending arrow debounce on non-arrow key input. - if !matches!(key.code, KeyCode::Up | KeyCode::Down) { - resolve_pending_arrow(app); - } - // Auto-scroll to bottom on input interaction (except scroll/panel/escape/arrow keys). - // Arrow keys are exempt because they may become scroll via debounce. + // Auto-scroll to bottom on input interaction (except scroll/panel/escape keys). if !matches!( key.code, KeyCode::PageUp @@ -138,7 +120,7 @@ fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { | KeyCode::Up | KeyCode::Down ) { - app.scroll_offset = 0; + app.content_scroll.to_bottom(); } match key.code { KeyCode::Enter if key.modifiers.contains(KeyModifiers::SHIFT) => { @@ -177,18 +159,16 @@ fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { multiline::line_end(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH); InputAction::None } - KeyCode::Up | KeyCode::Down => { - let dir = ScrollDirection::from_key(key.code).unwrap(); - handle_arrow_with_debounce(app, dir) - } + KeyCode::Up => handle_up(app), + KeyCode::Down => handle_down(app), KeyCode::Tab => InputAction::EnterPanel, KeyCode::Esc => handle_esc(app), KeyCode::PageUp => { - app.scroll_offset = app.scroll_offset.saturating_add(10); + app.content_scroll.scroll_up(10); InputAction::None } KeyCode::PageDown => { - app.scroll_offset = app.scroll_offset.saturating_sub(10); + app.content_scroll.scroll_down(10); InputAction::None } _ => InputAction::None, diff --git a/crates/loopal-tui/src/input/navigation.rs b/crates/loopal-tui/src/input/navigation.rs index 61ca93a4..95abc2e0 100644 --- a/crates/loopal-tui/src/input/navigation.rs +++ b/crates/loopal-tui/src/input/navigation.rs @@ -31,7 +31,6 @@ pub(super) fn move_cursor_right(app: &mut App) { /// Up: multiline navigation first, then history browse. pub(super) fn handle_up(app: &mut App) -> InputAction { - app.scroll_offset = 0; if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) && let Some(new_cursor) = multiline::cursor_up(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) @@ -55,7 +54,6 @@ pub(super) fn handle_up(app: &mut App) -> InputAction { /// Down: multiline navigation first, then history browse. pub(super) fn handle_down(app: &mut App) -> InputAction { - app.scroll_offset = 0; if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) && let Some(new_cursor) = multiline::cursor_down(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) diff --git a/crates/loopal-tui/src/input/scroll_debounce.rs b/crates/loopal-tui/src/input/scroll_debounce.rs deleted file mode 100644 index 5cb5b98d..00000000 --- a/crates/loopal-tui/src/input/scroll_debounce.rs +++ /dev/null @@ -1,197 +0,0 @@ -//! Arrow-key debounce: distinguishes mouse-wheel bursts from keyboard presses. -//! -//! xterm alternate scroll (`\x1b[?1007h`) translates mouse wheel into Up/Down -//! arrow keys. This module uses timing to tell them apart: -//! - Rapid-fire (< 30 ms gap) → mouse wheel → content scroll -//! - Isolated (> 30 ms) → keyboard → history navigation -//! -//! State: `Idle → Pending (30 ms) → Scrolling (150 ms idle → Idle)` -//! Second arrow within window → burst → Scrolling. -//! Other key or timer expiry → flush Pending as history. - -use std::time::{Duration, Instant}; - -use crossterm::event::KeyCode; - -use super::InputAction; -use super::multiline; -use super::navigation::{DEFAULT_WRAP_WIDTH, handle_down, handle_up}; -use crate::app::App; - -/// Window within which a second arrow event is considered a mouse-wheel burst. -const BURST_DETECT_MS: u64 = 30; - -/// After this idle period the scroll burst ends and state returns to Idle. -const SCROLL_IDLE_MS: u64 = 150; - -/// Scroll direction derived from arrow key code. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum ScrollDirection { - Up, - Down, -} - -impl ScrollDirection { - pub(crate) fn from_key(code: KeyCode) -> Option { - match code { - KeyCode::Up => Some(Self::Up), - KeyCode::Down => Some(Self::Down), - _ => None, - } - } -} - -/// Arrow-key debounce state. -#[derive(Debug, Default)] -pub(crate) enum ArrowDebounce { - /// No pending arrow event. - #[default] - Idle, - /// First arrow received; waiting to see if a burst follows. - Pending { - direction: ScrollDirection, - time: Instant, - }, - /// Mouse-wheel burst confirmed; subsequent arrows scroll content. - Scrolling { last_time: Instant }, -} - -/// Called by `handle_input_mode_key` when Up or Down is pressed. -/// -/// Multiline cursor navigation bypasses debounce entirely (immediate). -/// Otherwise returns `StartArrowDebounce` when the first arrow is deferred, -/// or `None` when handled inline (scroll / burst continuation). -pub(super) fn handle_arrow_with_debounce(app: &mut App, direction: ScrollDirection) -> InputAction { - // Multiline cursor navigation is always immediate — never debounced. - // This keeps multiline editing responsive and avoids burst misfires - // from fast keyboard repeat in multi-line input fields. - if try_multiline_nav(app, direction) { - app.arrow_debounce = ArrowDebounce::Idle; - return InputAction::None; - } - - match app.arrow_debounce { - ArrowDebounce::Idle => { - app.arrow_debounce = ArrowDebounce::Pending { - direction, - time: Instant::now(), - }; - InputAction::StartArrowDebounce - } - ArrowDebounce::Pending { - direction: old_dir, - time, - } => { - if time.elapsed() < burst_detect_duration() { - // Second event within burst window → mouse-wheel burst → scroll. - app.arrow_debounce = ArrowDebounce::Scrolling { - last_time: Instant::now(), - }; - apply_scroll(app, old_dir); - apply_scroll(app, direction); - InputAction::None - } else { - // Timer was delayed. Flush stale pending as history, then - // start a new debounce for this event. - process_as_history(app, old_dir); - app.arrow_debounce = ArrowDebounce::Pending { - direction, - time: Instant::now(), - }; - InputAction::StartArrowDebounce - } - } - ArrowDebounce::Scrolling { last_time } => { - if last_time.elapsed() > Duration::from_millis(SCROLL_IDLE_MS) { - // Tick was dropped or delayed. Treat as fresh Idle state. - app.arrow_debounce = ArrowDebounce::Pending { - direction, - time: Instant::now(), - }; - InputAction::StartArrowDebounce - } else { - app.arrow_debounce = ArrowDebounce::Scrolling { - last_time: Instant::now(), - }; - apply_scroll(app, direction); - InputAction::None - } - } - } -} - -/// Discard pending debounce without processing as history. -/// -/// Used by modal/global/autocomplete handlers that supersede the pending -/// arrow event. The stale 30 ms timer will see `Idle` and become a no-op. -pub(crate) fn discard_pending(app: &mut App) { - app.arrow_debounce = ArrowDebounce::Idle; -} - -/// Flush any pending arrow as a history navigation action. -/// -/// Called when a non-arrow key arrives or when the debounce timer expires. -pub(crate) fn resolve_pending_arrow(app: &mut App) { - match std::mem::replace(&mut app.arrow_debounce, ArrowDebounce::Idle) { - ArrowDebounce::Pending { direction, .. } => { - process_as_history(app, direction); - } - ArrowDebounce::Scrolling { .. } | ArrowDebounce::Idle => {} - } -} - -/// Expire stale Scrolling state (called from Tick handler). -pub(crate) fn tick_debounce(app: &mut App) { - if let ArrowDebounce::Scrolling { last_time } = app.arrow_debounce - && last_time.elapsed() > Duration::from_millis(SCROLL_IDLE_MS) - { - app.arrow_debounce = ArrowDebounce::Idle; - } -} - -/// Burst detection window. -pub(crate) fn burst_detect_duration() -> Duration { - Duration::from_millis(BURST_DETECT_MS) -} - -fn try_multiline_nav(app: &mut App, direction: ScrollDirection) -> bool { - if !multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) { - return false; - } - let new_cursor = match direction { - ScrollDirection::Up => { - multiline::cursor_up(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) - } - ScrollDirection::Down => { - multiline::cursor_down(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) - } - }; - if let Some(pos) = new_cursor { - app.input_cursor = pos; - true - } else { - false - } -} - -fn process_as_history(app: &mut App, direction: ScrollDirection) { - match direction { - ScrollDirection::Up => { - handle_up(app); - } - ScrollDirection::Down => { - handle_down(app); - } - } -} - -fn apply_scroll(app: &mut App, direction: ScrollDirection) { - match direction { - ScrollDirection::Up => { - app.scroll_offset = app.scroll_offset.saturating_add(3); - } - ScrollDirection::Down => { - app.scroll_offset = app.scroll_offset.saturating_sub(3); - } - } -} diff --git a/crates/loopal-tui/src/key_dispatch.rs b/crates/loopal-tui/src/key_dispatch.rs index 0e3e6212..db6851c8 100644 --- a/crates/loopal-tui/src/key_dispatch.rs +++ b/crates/loopal-tui/src/key_dispatch.rs @@ -10,7 +10,6 @@ use crate::key_dispatch_ops::{ cycle_panel_focus, enter_panel, handle_effect, handle_sub_page_confirm, panel_tab, push_to_inbox, terminate_focused_agent, }; -use crate::views::progress::LineCache; /// Process a single key event and return `true` if the TUI should quit. pub(crate) async fn handle_key_action( @@ -110,16 +109,14 @@ pub(crate) async fn handle_key_action( && app.session.enter_agent_view(&name) { app.focus_mode = crate::app::FocusMode::Input; - app.scroll_offset = 0; - app.line_cache = LineCache::new(); + app.content_scroll.reset(); app.last_esc_time = None; } false } InputAction::ExitAgentView => { app.session.exit_agent_view(); - app.scroll_offset = 0; - app.line_cache = LineCache::new(); + app.content_scroll.reset(); app.last_esc_time = None; false } @@ -187,14 +184,5 @@ pub(crate) async fn handle_key_action( false } InputAction::None => false, - InputAction::StartArrowDebounce => { - let tx = events.sender(); - let wait = crate::input::scroll_debounce::burst_detect_duration(); - tokio::spawn(async move { - tokio::time::sleep(wait).await; - let _ = tx.send(crate::event::AppEvent::ArrowDebounceTimeout).await; - }); - false - } } } diff --git a/crates/loopal-tui/src/key_dispatch_ops.rs b/crates/loopal-tui/src/key_dispatch_ops.rs index 06a910d3..7896f6ef 100644 --- a/crates/loopal-tui/src/key_dispatch_ops.rs +++ b/crates/loopal-tui/src/key_dispatch_ops.rs @@ -83,8 +83,7 @@ pub(crate) async fn terminate_focused_agent(app: &mut App) { app.session.interrupt_agent(&name); if active == name { app.session.exit_agent_view(); - app.scroll_offset = 0; - app.line_cache = crate::views::progress::LineCache::new(); + app.content_scroll.reset(); } app.focused_agent = None; // If no panels have content, exit panel mode diff --git a/crates/loopal-tui/src/render.rs b/crates/loopal-tui/src/render.rs index 6d431636..4e26ac6d 100644 --- a/crates/loopal-tui/src/render.rs +++ b/crates/loopal-tui/src/render.rs @@ -55,13 +55,7 @@ pub fn draw(f: &mut Frame, app: &mut App) { if breadcrumb_h > 0 { views::breadcrumb::render_breadcrumb(f, &state.active_view, layout.breadcrumb); } - views::progress::render_progress( - f, - &state, - app.scroll_offset, - &mut app.line_cache, - layout.content, - ); + app.content_scroll.render(f, &state, layout.content); let viewing = if state.active_view != loopal_session::ROOT_AGENT { Some(state.active_view.as_str()) } else { diff --git a/crates/loopal-tui/src/tui_loop.rs b/crates/loopal-tui/src/tui_loop.rs index 6d747df9..769d8135 100644 --- a/crates/loopal-tui/src/tui_loop.rs +++ b/crates/loopal-tui/src/tui_loop.rs @@ -4,6 +4,7 @@ use std::io; use std::path::PathBuf; use std::sync::Arc; +use crossterm::event::KeyCode; use ratatui::prelude::*; use loopal_protocol::{AgentEvent, AgentEventPayload}; @@ -60,9 +61,37 @@ where batch.push(event); } + // Pre-scan: count Up/Down arrow key events in this batch. + // Mouse wheel (via \x1b[?1007h]) generates ≥2 arrows per tick; + // a single keyboard press generates exactly 1. + let arrow_count = batch + .iter() + .filter(|e| { + matches!( + e, + AppEvent::Key(k) if matches!(k.code, KeyCode::Up | KeyCode::Down) + ) + }) + .count(); + let is_scroll_burst = arrow_count >= 2; + let mut should_quit = false; for event in batch { match event { + AppEvent::Key(key) + if is_scroll_burst && matches!(key.code, KeyCode::Up | KeyCode::Down) => + { + // Mouse-wheel burst → scroll content, bypass key handler. + match key.code { + KeyCode::Up => { + app.content_scroll.scroll_up(3); + } + KeyCode::Down => { + app.content_scroll.scroll_down(3); + } + _ => unreachable!(), + } + } AppEvent::Key(key) => { should_quit = handle_key_action(app, key, &events).await; if should_quit { @@ -82,15 +111,8 @@ where AppEvent::Paste(result) => { paste::apply_paste_result(app, result); } - AppEvent::ArrowDebounceTimeout => { - // Flush pending arrow as history if still pending; stale - // timeouts (Idle/Scrolling) are ignored by resolve. - crate::input::scroll_debounce::resolve_pending_arrow(app); - } AppEvent::Resize(_, _) => {} - AppEvent::Tick => { - crate::input::scroll_debounce::tick_debounce(app); - } + AppEvent::Tick => {} } } diff --git a/crates/loopal-tui/src/views/progress/content_scroll.rs b/crates/loopal-tui/src/views/progress/content_scroll.rs new file mode 100644 index 00000000..ba89341e --- /dev/null +++ b/crates/loopal-tui/src/views/progress/content_scroll.rs @@ -0,0 +1,125 @@ +/// Content area scroll state: encapsulates scroll offset, growth compensation, +/// and the incremental line cache. +/// +/// Scroll model: `offset` = lines from the bottom of all content. +/// - `offset == 0` → auto-follow (viewport stays at bottom) +/// - `offset > 0` → viewport is pinned N lines above the bottom +/// +/// When content grows while pinned, the offset is auto-compensated so the +/// viewport stays anchored at the same absolute position. Rendering uses +/// absolute position indexing (not `tail()`) to avoid the feedback loop +/// where window_size depends on offset and vice versa. +use ratatui::prelude::*; +use ratatui::widgets::Paragraph; + +use loopal_session::state::SessionState; + +use super::line_cache::LineCache; +use super::message_lines::streaming_to_lines; +use super::thinking_render; + +/// Scroll + render state for the main content area. +pub struct ContentScroll { + /// Lines from the bottom of all content (0 = auto-follow). + pub offset: u16, + /// Total content lines at previous render (for growth compensation). + prev_total: usize, + /// Incremental line cache for committed messages. + line_cache: LineCache, +} + +impl Default for ContentScroll { + fn default() -> Self { + Self::new() + } +} + +impl ContentScroll { + pub fn new() -> Self { + Self { + offset: 0, + prev_total: 0, + line_cache: LineCache::new(), + } + } + + /// Reset to initial state (view switch, new conversation). + pub fn reset(&mut self) { + self.offset = 0; + self.prev_total = 0; + self.line_cache = LineCache::new(); + } + + /// Scroll to bottom (auto-follow mode). + pub fn to_bottom(&mut self) { + self.offset = 0; + } + + /// Scroll up by N lines. + pub fn scroll_up(&mut self, lines: u16) { + self.offset = self.offset.saturating_add(lines); + } + + /// Scroll down by N lines. + pub fn scroll_down(&mut self, lines: u16) { + self.offset = self.offset.saturating_sub(lines); + } + + /// Render the content area into the given frame region. + pub fn render(&mut self, f: &mut Frame, state: &SessionState, area: Rect) { + let visible_h = area.height as usize; + if visible_h == 0 { + return; + } + + let conv = state.active_conversation(); + + self.line_cache.update(&conv.messages, area.width); + + let streaming = streaming_to_lines(&conv.streaming_text, area.width); + let thinking_lines = if conv.thinking_active { + thinking_render::streaming_thinking_lines(&conv.streaming_thinking, area.width) + } else { + vec![] + }; + + // Total logical content = cache + thinking + streaming + let cache_total = self.line_cache.total_lines(); + let total = cache_total + thinking_lines.len() + streaming.len(); + + // Compensate offset for content growth while pinned. + // Skip first frame (prev_total == 0) to avoid initial blowup. + if self.offset > 0 && self.prev_total > 0 && total > self.prev_total { + let delta = (total - self.prev_total).min(u16::MAX as usize) as u16; + self.offset = self.offset.saturating_add(delta); + } + self.prev_total = total; + + // Absolute top line of the viewport (clamped) + let max_scroll = total.saturating_sub(visible_h); + let clamped = (self.offset as usize).min(max_scroll); + let top_line = max_scroll.saturating_sub(clamped); + + // Slice cache portion visible in the viewport + let cache_start = top_line.min(cache_total); + let cache_end = (top_line + visible_h).min(cache_total); + let visible_cache = self.line_cache.slice(cache_start, cache_end - cache_start); + + let mut lines = Vec::with_capacity(visible_h); + lines.extend_from_slice(visible_cache); + + // Append ephemeral (thinking + streaming) lines if viewport extends past cache + if top_line + visible_h > cache_total { + let eph_start = top_line.saturating_sub(cache_total); + let all_ephemeral: Vec> = + thinking_lines.into_iter().chain(streaming).collect(); + let eph_end = (top_line + visible_h - cache_total).min(all_ephemeral.len()); + if eph_start < eph_end { + lines.extend_from_slice(&all_ephemeral[eph_start..eph_end]); + } + } + + let paragraph = Paragraph::new(lines).scroll((0, 0)); + f.render_widget(paragraph, area); + } +} diff --git a/crates/loopal-tui/src/views/progress/line_cache.rs b/crates/loopal-tui/src/views/progress/line_cache.rs index 97c68c72..35f36923 100644 --- a/crates/loopal-tui/src/views/progress/line_cache.rs +++ b/crates/loopal-tui/src/views/progress/line_cache.rs @@ -69,11 +69,11 @@ impl LineCache { self.lines.len() } - /// Return the tail of cached lines for the visible window. - /// Only clones the lines needed for rendering, not the entire history. - pub fn tail(&self, max_lines: usize) -> &[Line<'static>] { - let start = self.lines.len().saturating_sub(max_lines); - &self.lines[start..] + /// Return a slice of cached lines at an absolute position. + pub fn slice(&self, start: usize, len: usize) -> &[Line<'static>] { + let s = start.min(self.lines.len()); + let e = (s + len).min(self.lines.len()); + &self.lines[s..e] } /// Total number of cached lines. diff --git a/crates/loopal-tui/src/views/progress/mod.rs b/crates/loopal-tui/src/views/progress/mod.rs index cc587552..c4392228 100644 --- a/crates/loopal-tui/src/views/progress/mod.rs +++ b/crates/loopal-tui/src/views/progress/mod.rs @@ -1,4 +1,5 @@ /// Content area: main agent output region (borderless). +mod content_scroll; mod line_cache; mod message_lines; mod skill_display; @@ -6,58 +7,6 @@ mod thinking_render; mod tool_display; mod welcome; -use ratatui::prelude::*; -use ratatui::widgets::Paragraph; - -use loopal_session::state::SessionState; - +pub use content_scroll::ContentScroll; pub use line_cache::LineCache; pub use message_lines::{message_to_lines, streaming_to_lines}; - -/// Render the content area — no border, no title, content fills the area. -pub fn render_progress( - f: &mut Frame, - state: &SessionState, - scroll_offset: u16, - line_cache: &mut LineCache, - area: Rect, -) { - let visible_h = area.height as usize; - if visible_h == 0 { - return; - } - - let conv = state.active_conversation(); - - // Update cache with width for pre-wrapping (resize triggers full rebuild) - line_cache.update(&conv.messages, area.width); - - // Streaming lines (pre-wrapped at current width) - let streaming = streaming_to_lines(&conv.streaming_text, area.width); - - // Thinking indicator (shown during active thinking with full content) - let thinking_lines = if conv.thinking_active { - thinking_render::streaming_thinking_lines(&conv.streaming_thinking, area.width) - } else { - vec![] - }; - - // Window: lines are already visual rows - let window_size = visible_h + scroll_offset as usize; - let cached_tail = line_cache.tail(window_size); - - // Build the render lines: cached tail + thinking + streaming - let mut lines = Vec::with_capacity(cached_tail.len() + thinking_lines.len() + streaming.len()); - lines.extend_from_slice(cached_tail); - lines.extend(thinking_lines); - lines.extend(streaming); - - // Scroll: lines.len() == visual line count (pre-wrapped), so this is exact - let window_lines = lines.len() as u16; - let scroll_row = window_lines - .saturating_sub(visible_h as u16) - .saturating_sub(scroll_offset); - - let paragraph = Paragraph::new(lines).scroll((scroll_row, 0)); - f.render_widget(paragraph, area); -} diff --git a/crates/loopal-tui/tests/suite.rs b/crates/loopal-tui/tests/suite.rs index 8055c019..2e8d4682 100644 --- a/crates/loopal-tui/tests/suite.rs +++ b/crates/loopal-tui/tests/suite.rs @@ -62,6 +62,8 @@ mod panel_tab_test; mod render_guard_test; #[path = "suite/scroll_burst_test.rs"] mod scroll_burst_test; +#[path = "suite/scroll_compensation_test.rs"] +mod scroll_compensation_test; #[path = "suite/skill_render_test.rs"] mod skill_render_test; #[path = "suite/styled_wrap_test.rs"] @@ -98,6 +100,8 @@ mod e2e_mcp_test; mod e2e_multi_turn_test; #[path = "suite/e2e_permission_test.rs"] mod e2e_permission_test; +#[path = "suite/e2e_scroll_test.rs"] +mod e2e_scroll_test; #[path = "suite/e2e_session_test.rs"] mod e2e_session_test; #[path = "suite/e2e_system_test.rs"] diff --git a/crates/loopal-tui/tests/suite/app_test.rs b/crates/loopal-tui/tests/suite/app_test.rs index 56864ea2..6cd17e50 100644 --- a/crates/loopal-tui/tests/suite/app_test.rs +++ b/crates/loopal-tui/tests/suite/app_test.rs @@ -28,7 +28,7 @@ fn test_app_new_initializes_correctly() { assert!(!app.exiting); assert!(app.input.is_empty()); assert_eq!(app.input_cursor, 0); - assert_eq!(app.scroll_offset, 0); + assert_eq!(app.content_scroll.offset, 0); let state = app.session.lock(); let conv = state.active_conversation(); assert!(conv.messages.is_empty()); diff --git a/crates/loopal-tui/tests/suite/e2e_scroll_test.rs b/crates/loopal-tui/tests/suite/e2e_scroll_test.rs new file mode 100644 index 00000000..65cbc102 --- /dev/null +++ b/crates/loopal-tui/tests/suite/e2e_scroll_test.rs @@ -0,0 +1,173 @@ +/// E2E integration tests for scroll behavior through the full TUI event loop. +/// +/// Regression tests for: +/// - Bug 1: Mouse wheel (≥2 arrow events in batch) scrolls content, not history +/// - Bug 2: Single keyboard arrow navigates history, not scroll +/// - Bug 3: Scroll offset is compensated when content grows during streaming +use std::sync::Arc; +use std::time::Duration; + +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use ratatui::Terminal; +use ratatui::backend::TestBackend; +use tokio::sync::{mpsc, watch}; + +use loopal_protocol::{ + AgentEvent, AgentEventPayload, ControlCommand, InterruptSignal, UserQuestionResponse, +}; +use loopal_session::SessionController; +use loopal_tui::app::App; +use loopal_tui::event::{AppEvent, EventHandler}; +use loopal_tui::run_tui_loop; + +fn build_scroll_rig() -> ( + Terminal, + App, + EventHandler, + mpsc::Sender, +) { + let (ctrl_tx, _) = mpsc::channel::(16); + let (perm_tx, _) = mpsc::channel::(16); + let (q_tx, _) = mpsc::channel::(16); + let session = SessionController::new( + "test-model".into(), + "act".into(), + ctrl_tx, + perm_tx, + q_tx, + InterruptSignal::new(), + Arc::new(watch::channel(0u64).0), + ); + let backend = TestBackend::new(80, 24); + let terminal = Terminal::new(backend).unwrap(); + let mut app = App::new(session, std::env::temp_dir()); + app.input_history.push("history-entry".into()); + let (tx, rx) = mpsc::channel::(256); + let events = EventHandler::from_channel(tx.clone(), rx); + (terminal, app, events, tx) +} + +fn up_key() -> KeyEvent { + KeyEvent::new(KeyCode::Up, KeyModifiers::NONE) +} + +fn page_up_key() -> KeyEvent { + KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE) +} + +fn ctrl_d() -> KeyEvent { + KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL) +} + +fn stream_event(text: &str) -> AppEvent { + AppEvent::Agent(AgentEvent::root(AgentEventPayload::Stream { + text: text.to_string(), + })) +} + +// --- Bug 1 regression: burst arrows → scroll, not history --- + +#[tokio::test] +async fn test_burst_arrows_scroll_not_history() { + let (mut terminal, mut app, events, tx) = build_scroll_rig(); + + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(30)).await; + // Send 3 Up events in rapid succession (mouse wheel burst). + // They should all land in the same batch via try_next(). + let _ = tx.send(AppEvent::Key(up_key())).await; + let _ = tx.send(AppEvent::Key(up_key())).await; + let _ = tx.send(AppEvent::Key(up_key())).await; + tokio::time::sleep(Duration::from_millis(30)).await; + let _ = tx.send(AppEvent::Key(ctrl_d())).await; + }); + + let _ = tokio::time::timeout( + Duration::from_secs(3), + run_tui_loop(&mut terminal, events, &mut app), + ) + .await; + + assert!( + app.content_scroll.offset > 0, + "burst should scroll content, got offset={}", + app.content_scroll.offset + ); + assert!( + app.input.is_empty(), + "burst should NOT navigate history, got input={:?}", + app.input + ); +} + +// --- Bug 1 regression: single arrow → history, not scroll --- + +#[tokio::test] +async fn test_single_arrow_navigates_history() { + let (mut terminal, mut app, events, tx) = build_scroll_rig(); + + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(30)).await; + // Single Up event = keyboard press → history navigation + let _ = tx.send(AppEvent::Key(up_key())).await; + tokio::time::sleep(Duration::from_millis(30)).await; + let _ = tx.send(AppEvent::Key(ctrl_d())).await; + }); + + let _ = tokio::time::timeout( + Duration::from_secs(3), + run_tui_loop(&mut terminal, events, &mut app), + ) + .await; + + assert_eq!( + app.input, "history-entry", + "single Up should navigate history" + ); + assert_eq!( + app.content_scroll.offset, 0, + "single Up should not scroll content" + ); +} + +// --- Bug 2 regression: scroll offset compensated during streaming --- + +#[tokio::test] +async fn test_scroll_offset_stable_during_streaming() { + let (mut terminal, mut app, events, tx) = build_scroll_rig(); + + let tx2 = tx.clone(); + let tx3 = tx.clone(); + tokio::spawn(async move { + // 1. Send initial content (enough to make the area scrollable) + let initial: String = (0..40).map(|i| format!("line {i}\n")).collect(); + let _ = tx.send(stream_event(&initial)).await; + tokio::time::sleep(Duration::from_millis(50)).await; + + // 2. Scroll up via PageUp + let _ = tx2.send(AppEvent::Key(page_up_key())).await; + tokio::time::sleep(Duration::from_millis(50)).await; + + // 3. More content arrives while scrolled up + let more: String = (0..60).map(|i| format!("line {i}\n")).collect(); + let _ = tx3.send(stream_event(&more)).await; + tokio::time::sleep(Duration::from_millis(50)).await; + + // 4. Quit + let _ = tx3.send(AppEvent::Key(ctrl_d())).await; + }); + + let _ = tokio::time::timeout( + Duration::from_secs(3), + run_tui_loop(&mut terminal, events, &mut app), + ) + .await; + + // PageUp sets offset=10. Compensation should increase it further + // because streaming content grew while we were pinned. + assert!( + app.content_scroll.offset > 10, + "offset should be compensated for growth: got {}", + app.content_scroll.offset + ); +} diff --git a/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs b/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs index 7c35515c..98c9bfb3 100644 --- a/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs +++ b/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs @@ -6,7 +6,7 @@ use loopal_protocol::UserQuestionResponse; use loopal_protocol::{AgentStatus, ControlCommand}; use loopal_session::SessionController; use loopal_tui::app::App; -use loopal_tui::input::{handle_key, resolve_arrow_debounce}; +use loopal_tui::input::handle_key; use tokio::sync::mpsc; fn make_app() -> App { @@ -33,11 +33,6 @@ fn ctrl(c: char) -> KeyEvent { KeyEvent::new(KeyCode::Char(c), KeyModifiers::CONTROL) } -fn arrow(app: &mut App, code: KeyCode) { - handle_key(app, key(code)); - resolve_arrow_debounce(app); -} - // --- Auto-reset scroll: per-key verification --- #[test] @@ -45,9 +40,12 @@ fn test_backspace_resets_scroll_offset() { let mut app = make_app(); app.input = "ab".into(); app.input_cursor = 2; - app.scroll_offset = 4; + app.content_scroll.offset = 4; handle_key(&mut app, key(KeyCode::Backspace)); - assert_eq!(app.scroll_offset, 0, "Backspace should reset scroll"); + assert_eq!( + app.content_scroll.offset, 0, + "Backspace should reset scroll" + ); assert_eq!(app.input, "a"); } @@ -56,29 +54,29 @@ fn test_cursor_move_resets_scroll_offset() { let mut app = make_app(); app.input = "hello".into(); app.input_cursor = 3; - app.scroll_offset = 6; + app.content_scroll.offset = 6; handle_key(&mut app, key(KeyCode::Left)); - assert_eq!(app.scroll_offset, 0, "Left should reset scroll"); + assert_eq!(app.content_scroll.offset, 0, "Left should reset scroll"); assert_eq!(app.input_cursor, 2); } #[test] fn test_tab_preserves_scroll_offset() { let mut app = make_app(); - app.scroll_offset = 7; + app.content_scroll.offset = 7; handle_key(&mut app, key(KeyCode::Tab)); - assert_eq!(app.scroll_offset, 7, "Tab should not reset scroll"); + assert_eq!(app.content_scroll.offset, 7, "Tab should not reset scroll"); } #[test] fn test_esc_preserves_scroll_offset() { let mut app = make_app(); - app.scroll_offset = 3; + app.content_scroll.offset = 3; handle_key(&mut app, key(KeyCode::Esc)); - assert_eq!(app.scroll_offset, 3, "Esc should not reset scroll"); + assert_eq!(app.content_scroll.offset, 3, "Esc should not reset scroll"); } -// --- Agent busy: Up/Down still navigates history (after debounce) --- +// --- Agent busy: Up/Down still navigates history --- #[test] fn test_up_navigates_history_when_agent_busy() { @@ -91,22 +89,25 @@ fn test_up_navigates_history_when_agent_busy() { .observable .status = AgentStatus::Running; app.input_history.push("prev".into()); - arrow(&mut app, KeyCode::Up); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!( app.input, "prev", "Up should navigate history even when agent busy" ); } -// --- Ctrl+P/N resets scroll offset --- +// --- Ctrl+P/N preserves scroll offset (history and scroll are decoupled) --- #[test] -fn test_ctrl_p_resets_scroll_offset() { +fn test_ctrl_p_preserves_scroll_offset() { let mut app = make_app(); - app.scroll_offset = 10; + app.content_scroll.offset = 10; app.input_history.push("cmd".into()); handle_key(&mut app, ctrl('p')); - assert_eq!(app.scroll_offset, 0, "Ctrl+P should reset scroll"); + assert_eq!( + app.content_scroll.offset, 10, + "Ctrl+P should not reset scroll" + ); assert_eq!(app.input, "cmd"); } @@ -118,7 +119,7 @@ fn test_up_at_first_line_falls_through_to_history() { app.input = "line1\nline2".into(); app.input_cursor = 2; // middle of line1 (already at first visual line) app.input_history.push("old command".into()); - arrow(&mut app, KeyCode::Up); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!( app.input, "old command", "Up at first line should fall through to history" @@ -131,53 +132,15 @@ fn test_down_at_last_line_falls_through_to_history() { // First enter history via Up app.input_history.push("first".into()); app.input_history.push("second".into()); - arrow(&mut app, KeyCode::Up); // "second" - arrow(&mut app, KeyCode::Up); // "first" + handle_key(&mut app, key(KeyCode::Up)); // "second" + handle_key(&mut app, key(KeyCode::Up)); // "first" // Now set multiline content and cursor at last line app.input = "line1\nline2".into(); app.input_cursor = app.input.len(); // end of line2 - arrow(&mut app, KeyCode::Down); + handle_key(&mut app, key(KeyCode::Down)); // cursor_down returns None (at last line), falls through to history forward assert_eq!( app.input, "second", "Down at last line should fall through to history" ); } - -// --- Debounce resolution on non-arrow key --- - -#[test] -fn test_typing_after_up_resolves_pending_as_history() { - let mut app = make_app(); - app.input_history.push("hist".into()); - // Press Up — starts debounce - handle_key(&mut app, key(KeyCode::Up)); - assert!(app.input.is_empty(), "Up is pending, not yet resolved"); - // Type 'x' — resolves the pending Up as history, then inserts 'x' - handle_key(&mut app, key(KeyCode::Char('x'))); - assert_eq!( - app.input, "histx", - "pending Up resolves as history, then 'x' appends" - ); -} - -// --- Global shortcut discards pending debounce --- - -#[test] -fn test_ctrl_c_discards_pending_debounce() { - let mut app = make_app(); - app.input = "some text".into(); - app.input_cursor = 9; - app.input_history.push("hist".into()); - // Press Up — starts debounce - handle_key(&mut app, key(KeyCode::Up)); - // Ctrl+C — clears input AND discards pending debounce - handle_key(&mut app, ctrl('c')); - assert!(app.input.is_empty(), "Ctrl+C should clear input"); - // Stale timer fires — should be a no-op, not load history - resolve_arrow_debounce(&mut app); - assert!( - app.input.is_empty(), - "stale timer after Ctrl+C must not load history" - ); -} diff --git a/crates/loopal-tui/tests/suite/input_scroll_test.rs b/crates/loopal-tui/tests/suite/input_scroll_test.rs index d7a1a880..d9b9c78c 100644 --- a/crates/loopal-tui/tests/suite/input_scroll_test.rs +++ b/crates/loopal-tui/tests/suite/input_scroll_test.rs @@ -1,13 +1,13 @@ /// Tests for Up/Down key routing, Ctrl+P/N history, and multiline priority. -/// Up/Down goes through arrow-key debounce (30 ms window); multiline bypasses it. -/// Ctrl+P/N always navigate history. Burst detection tests in edge test file. +/// Up/Down now go directly to history navigation (no debounce). +/// Ctrl+P/N always navigate history. Scroll is handled separately via batch detection. use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use loopal_protocol::UserQuestionResponse; use loopal_protocol::{AgentStatus, ControlCommand}; use loopal_session::SessionController; use loopal_tui::app::App; -use loopal_tui::input::{InputAction, handle_key, resolve_arrow_debounce}; +use loopal_tui::input::{InputAction, handle_key}; use tokio::sync::mpsc; fn make_app() -> App { @@ -34,50 +34,37 @@ fn ctrl(c: char) -> KeyEvent { KeyEvent::new(KeyCode::Char(c), KeyModifiers::CONTROL) } -/// Simulate a single keyboard arrow press: sends the key then resolves the -/// debounce timer (equivalent to 30 ms passing with no burst). -fn arrow(app: &mut App, code: KeyCode) { - handle_key(app, key(code)); - resolve_arrow_debounce(app); -} - // --- PageUp / PageDown --- #[test] fn test_page_up_down_scroll() { let mut app = make_app(); handle_key(&mut app, key(KeyCode::PageUp)); - assert_eq!(app.scroll_offset, 10); + assert_eq!(app.content_scroll.offset, 10); handle_key(&mut app, key(KeyCode::PageDown)); - assert_eq!(app.scroll_offset, 0); + assert_eq!(app.content_scroll.offset, 0); } -// --- Up/Down navigate history (after debounce resolves as keyboard) --- +// --- Up/Down navigate history directly --- #[test] fn test_up_navigates_history_with_content() { let mut app = make_app(); app.input_history.push("previous".into()); - arrow(&mut app, KeyCode::Up); - assert_eq!( - app.input, "previous", - "Up should browse history, not scroll" - ); - assert_eq!(app.scroll_offset, 0); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.input, "previous", "Up should browse history"); } #[test] -fn test_down_resets_scroll_and_navigates_history() { +fn test_down_navigates_history_forward() { let mut app = make_app(); - app.scroll_offset = 5; app.input_history.push("first".into()); app.input_history.push("second".into()); - arrow(&mut app, KeyCode::Up); - arrow(&mut app, KeyCode::Up); + handle_key(&mut app, key(KeyCode::Up)); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!(app.input, "first"); - arrow(&mut app, KeyCode::Down); + handle_key(&mut app, key(KeyCode::Down)); assert_eq!(app.input, "second", "Down should navigate history forward"); - assert_eq!(app.scroll_offset, 0, "scroll_offset should be 0"); } #[test] @@ -92,13 +79,12 @@ fn test_up_navigates_history_when_idle() { .status = AgentStatus::WaitingForInput; app.input_history.push("older".into()); app.input_history.push("recent".into()); - arrow(&mut app, KeyCode::Up); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!(app.input, "recent", "Up should browse history"); - assert_eq!(app.scroll_offset, 0); } #[test] -fn test_down_navigates_history_forward() { +fn test_down_navigates_history_forward_when_idle() { let mut app = make_app(); app.session .lock() @@ -109,15 +95,15 @@ fn test_down_navigates_history_forward() { .status = AgentStatus::WaitingForInput; app.input_history.push("first".into()); app.input_history.push("second".into()); - arrow(&mut app, KeyCode::Up); - arrow(&mut app, KeyCode::Up); + handle_key(&mut app, key(KeyCode::Up)); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!(app.input, "first"); - arrow(&mut app, KeyCode::Down); + handle_key(&mut app, key(KeyCode::Down)); assert_eq!(app.input, "second", "Down should navigate history forward"); } #[test] -fn test_up_navigates_history_when_content_fits() { +fn test_up_navigates_history_immediately() { let mut app = make_app(); app.session .lock() @@ -128,10 +114,11 @@ fn test_up_navigates_history_when_content_fits() { .status = AgentStatus::WaitingForInput; app.input_history.push("previous command".into()); let action = handle_key(&mut app, key(KeyCode::Up)); - assert!(matches!(action, InputAction::StartArrowDebounce)); - resolve_arrow_debounce(&mut app); - assert_eq!(app.input, "previous command", "Up browses history"); - assert_eq!(app.scroll_offset, 0); + assert!(matches!(action, InputAction::None)); + assert_eq!( + app.input, "previous command", + "Up browses history immediately" + ); } // --- Ctrl+P/N history navigation --- @@ -150,7 +137,6 @@ fn test_ctrl_p_navigates_history() { app.input_history.push("second".into()); handle_key(&mut app, ctrl('p')); assert_eq!(app.input, "second", "Ctrl+P should browse history"); - assert_eq!(app.scroll_offset, 0, "Ctrl+P should not scroll"); } #[test] @@ -172,7 +158,7 @@ fn test_ctrl_n_navigates_history_forward() { assert_eq!(app.input, "second", "Ctrl+N browses history forward"); } -// --- Multiline cursor priority (bypasses debounce) --- +// --- Multiline cursor priority --- #[test] fn test_up_multiline_cursor_beats_history() { @@ -180,9 +166,7 @@ fn test_up_multiline_cursor_beats_history() { app.input = "line1\nline2".into(); app.input_cursor = app.input.len(); let action = handle_key(&mut app, key(KeyCode::Up)); - // Multiline nav is immediate — no debounce assert!(matches!(action, InputAction::None)); - assert_eq!(app.scroll_offset, 0, "should move cursor, not scroll"); assert!( app.input_cursor < "line1\n".len(), "cursor should be on line1, got {}", @@ -197,7 +181,6 @@ fn test_down_multiline_cursor_beats_history() { app.input_cursor = 0; let action = handle_key(&mut app, key(KeyCode::Down)); assert!(matches!(action, InputAction::None)); - assert_eq!(app.scroll_offset, 0, "should move cursor, not scroll"); assert!( app.input_cursor >= "line1\n".len(), "cursor should be on line2, got {}", @@ -210,18 +193,26 @@ fn test_down_multiline_cursor_beats_history() { #[test] fn test_typing_resets_scroll_offset() { let mut app = make_app(); - app.scroll_offset = 5; + app.content_scroll.offset = 5; handle_key(&mut app, key(KeyCode::Char('a'))); - assert_eq!(app.scroll_offset, 0, "typing should reset scroll to bottom"); + assert_eq!( + app.content_scroll.offset, 0, + "typing should reset scroll to bottom" + ); assert_eq!(app.input, "a"); } +// --- Up/Down do NOT reset scroll_offset (scroll and history are decoupled) --- + #[test] -fn test_up_resets_scroll_offset() { +fn test_up_preserves_scroll_offset() { let mut app = make_app(); - app.scroll_offset = 3; + app.content_scroll.offset = 10; app.input_history.push("cmd".into()); - arrow(&mut app, KeyCode::Up); - assert_eq!(app.scroll_offset, 0, "Up should reset scroll to bottom"); + handle_key(&mut app, key(KeyCode::Up)); assert_eq!(app.input, "cmd"); + assert_eq!( + app.content_scroll.offset, 10, + "Up should not reset scroll_offset" + ); } diff --git a/crates/loopal-tui/tests/suite/line_cache_test.rs b/crates/loopal-tui/tests/suite/line_cache_test.rs index 4fed2eab..6e663803 100644 --- a/crates/loopal-tui/tests/suite/line_cache_test.rs +++ b/crates/loopal-tui/tests/suite/line_cache_test.rs @@ -17,7 +17,7 @@ fn msg(role: &str, content: &str) -> SessionMessage { fn test_empty_messages() { let mut cache = LineCache::new(); assert_eq!(cache.update(&[], W), 0); - assert!(cache.tail(100).is_empty()); + assert!(cache.slice(0, 100).is_empty()); } #[test] @@ -33,11 +33,13 @@ fn test_incremental_append() { } #[test] -fn test_tail_window() { +fn test_slice_window() { let mut cache = LineCache::new(); let msgs: Vec<_> = (0..20).map(|i| msg("user", &format!("msg {i}"))).collect(); cache.update(&msgs, W); - let tail = cache.tail(5); + let total = cache.total_lines(); + let start = total.saturating_sub(5); + let tail = cache.slice(start, 5); assert!(tail.len() <= 5); } @@ -47,7 +49,7 @@ fn test_clear_invalidation() { let msgs = vec![msg("user", "hello"), msg("assistant", "hi")]; cache.update(&msgs, W); cache.update(&[], W); - assert!(cache.tail(100).is_empty()); + assert!(cache.slice(0, 100).is_empty()); } #[test] @@ -81,7 +83,7 @@ fn test_tool_call_mutation_detected() { // Cache should detect the mutation via fingerprint change let text: String = cache - .tail(100) + .slice(0, cache.total_lines()) .iter() .flat_map(|l| l.spans.iter().map(|s| s.content.to_string())) .collect(); @@ -146,7 +148,7 @@ fn test_tool_result_arrival_invalidates_cache() { // Single-line summary: line count stays the same, but cache was rebuilt // (fingerprint changed). Verify the summary text is updated. let text: String = cache - .tail(100) + .slice(0, cache.total_lines()) .iter() .flat_map(|l| l.spans.iter().map(|s| s.content.to_string())) .collect(); diff --git a/crates/loopal-tui/tests/suite/scroll_burst_test.rs b/crates/loopal-tui/tests/suite/scroll_burst_test.rs index a0743eb3..21c78634 100644 --- a/crates/loopal-tui/tests/suite/scroll_burst_test.rs +++ b/crates/loopal-tui/tests/suite/scroll_burst_test.rs @@ -1,11 +1,16 @@ -/// Tests for arrow-key debounce: mouse-wheel burst detection and stale state. +/// Tests for batch-based scroll detection: mouse-wheel bursts vs keyboard arrows. +/// +/// Mouse wheel (via `\x1b[?1007h`) produces ≥2 arrow events per batch → scroll. +/// A single keyboard arrow press produces 1 event per batch → history navigation. +/// The batch detection logic lives in `tui_loop.rs`; these tests verify the +/// observable state after batch processing via direct App manipulation. use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use loopal_protocol::ControlCommand; use loopal_protocol::UserQuestionResponse; use loopal_session::SessionController; use loopal_tui::app::App; -use loopal_tui::input::{InputAction, handle_key, resolve_arrow_debounce}; +use loopal_tui::input::handle_key; use tokio::sync::mpsc; fn make_app() -> App { @@ -28,153 +33,129 @@ fn key(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::NONE) } -fn ctrl(c: char) -> KeyEvent { - KeyEvent::new(KeyCode::Char(c), KeyModifiers::CONTROL) +/// Simulate mouse-wheel scroll burst: multiple arrow events applied as scroll. +fn apply_scroll_burst(app: &mut App, code: KeyCode, count: usize) { + for _ in 0..count { + match code { + KeyCode::Up => app.content_scroll.offset = app.content_scroll.offset.saturating_add(3), + KeyCode::Down => { + app.content_scroll.offset = app.content_scroll.offset.saturating_sub(3) + } + _ => {} + } + } } -fn arrow(app: &mut App, code: KeyCode) { - handle_key(app, key(code)); - resolve_arrow_debounce(app); -} - -// --- Mouse wheel burst detection --- +// --- Mouse wheel burst detection (≥2 arrows → scroll) --- #[test] -fn test_rapid_up_burst_scrolls_content() { +fn test_scroll_burst_up_increases_offset() { let mut app = make_app(); app.input_history.push("should not appear".into()); - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - assert!(app.scroll_offset > 0, "burst should scroll content"); - assert!(app.input.is_empty(), "burst should NOT navigate history"); -} - -#[test] -fn test_rapid_down_burst_scrolls_content() { - let mut app = make_app(); - app.scroll_offset = 20; - handle_key(&mut app, key(KeyCode::Down)); - handle_key(&mut app, key(KeyCode::Down)); - assert!(app.scroll_offset < 20, "burst should scroll down"); + // Simulate a burst of 3 Up events (mouse wheel) + apply_scroll_burst(&mut app, KeyCode::Up, 3); + assert_eq!( + app.content_scroll.offset, 9, + "3 Up scroll events should add 9" + ); + assert!(app.input.is_empty(), "scroll should NOT navigate history"); } #[test] -fn test_continuous_scroll_burst() { +fn test_scroll_burst_down_decreases_offset() { let mut app = make_app(); - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - // 1st+2nd: Pending→Scrolling (scroll 2×3=6), 3rd: continues (+3) - assert_eq!(app.scroll_offset, 9, "3 rapid Up events should scroll 9"); + app.content_scroll.offset = 30; + apply_scroll_burst(&mut app, KeyCode::Down, 2); + assert_eq!( + app.content_scroll.offset, 24, + "2 Down scroll events should reduce by 6" + ); } #[test] -fn test_down_burst_exact_offset() { +fn test_scroll_burst_down_clamps_at_zero() { let mut app = make_app(); - app.scroll_offset = 30; - handle_key(&mut app, key(KeyCode::Down)); - handle_key(&mut app, key(KeyCode::Down)); - // Pending→Scrolling: scroll down 2×3=6 - assert_eq!(app.scroll_offset, 24, "2 rapid Down should reduce by 6"); + app.content_scroll.offset = 2; + apply_scroll_burst(&mut app, KeyCode::Down, 3); + assert_eq!( + app.content_scroll.offset, 0, + "scroll_offset should not go negative" + ); } -// --- Mixed direction burst --- +// --- Single arrow → keyboard → history --- #[test] -fn test_mixed_direction_burst_applies_both() { +fn test_single_up_navigates_history() { let mut app = make_app(); - app.scroll_offset = 10; - handle_key(&mut app, key(KeyCode::Up)); // Pending(Up) - handle_key(&mut app, key(KeyCode::Down)); // burst: scroll Up+3 then Down-3 - // Net effect: +3 - 3 = 0 change - assert_eq!(app.scroll_offset, 10, "Up then Down burst should net zero"); + app.input_history.push("previous".into()); + // Single Up event → keyboard → history navigation + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.input, "previous", "single Up should browse history"); } -// --- Stale Scrolling degrades --- - #[test] -fn test_stale_scrolling_degrades_to_debounce() { +fn test_single_down_navigates_history_forward() { let mut app = make_app(); - app.input_history.push("hist".into()); + app.input_history.push("first".into()); + app.input_history.push("second".into()); handle_key(&mut app, key(KeyCode::Up)); handle_key(&mut app, key(KeyCode::Up)); - assert!(app.scroll_offset > 0); - std::thread::sleep(std::time::Duration::from_millis(200)); - let action = handle_key(&mut app, key(KeyCode::Up)); - assert!( - matches!(action, InputAction::StartArrowDebounce), - "stale Scrolling should degrade to new debounce" - ); - resolve_arrow_debounce(&mut app); - assert_eq!(app.input, "hist"); + assert_eq!(app.input, "first"); + handle_key(&mut app, key(KeyCode::Down)); + assert_eq!(app.input, "second", "Down should navigate history forward"); } -// --- Empty history + debounce --- +// --- Scroll does not affect history --- #[test] -fn test_up_with_empty_history_does_nothing() { +fn test_scroll_burst_does_not_touch_history() { let mut app = make_app(); - // No history entries - arrow(&mut app, KeyCode::Up); + app.input_history.push("preserved".into()); + apply_scroll_burst(&mut app, KeyCode::Up, 5); + assert!(app.content_scroll.offset > 0); + assert!(app.input.is_empty(), "scroll must not touch input"); assert!( - app.input.is_empty(), - "Up with no history should leave input empty" + app.history_index.is_none(), + "scroll must not set history_index" ); - assert_eq!(app.scroll_offset, 0); } -// --- Ctrl+P during Pending state discards pending --- +// --- History does not reset scroll_offset --- #[test] -fn test_ctrl_p_during_pending_discards_deferred() { +fn test_history_navigation_preserves_scroll() { let mut app = make_app(); - app.input_history.push("first".into()); - app.input_history.push("second".into()); - // Up → Pending (deferred) - let action = handle_key(&mut app, key(KeyCode::Up)); - assert!(matches!(action, InputAction::StartArrowDebounce)); - assert!(app.input.is_empty(), "Up is deferred, input still empty"); - // Ctrl+P → navigates history; discard pending debounce - handle_key(&mut app, ctrl('p')); - assert_eq!(app.input, "second", "Ctrl+P navigates history"); - // Stale timer fires — discarded, no second navigation - resolve_arrow_debounce(&mut app); - assert_eq!(app.input, "second", "stale timer is no-op after discard"); + app.content_scroll.offset = 10; + app.input_history.push("cmd".into()); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.input, "cmd"); + assert_eq!( + app.content_scroll.offset, 10, + "history nav should not reset scroll" + ); } -// --- Burst then different action --- +// --- Typing resets scroll --- #[test] -fn test_scroll_burst_then_type_resets_debounce() { +fn test_typing_resets_scroll_offset() { let mut app = make_app(); - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - assert!(app.scroll_offset > 0, "burst should scroll"); - // Typing resets scroll and clears Scrolling state + app.content_scroll.offset = 15; handle_key(&mut app, key(KeyCode::Char('x'))); - assert_eq!(app.scroll_offset, 0, "typing resets scroll"); - assert_eq!(app.input, "x"); + assert_eq!( + app.content_scroll.offset, 0, + "typing should reset scroll to bottom" + ); } -// --- Sequential burst: up then down --- +// --- PageUp/PageDown --- #[test] -fn test_sequential_up_then_down_bursts() { +fn test_page_up_down_scroll() { let mut app = make_app(); - // Up burst - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - handle_key(&mut app, key(KeyCode::Up)); - let after_up = app.scroll_offset; - assert!(after_up > 0); - // Wait for Scrolling to expire - std::thread::sleep(std::time::Duration::from_millis(200)); - // Down burst - handle_key(&mut app, key(KeyCode::Down)); - handle_key(&mut app, key(KeyCode::Down)); - handle_key(&mut app, key(KeyCode::Down)); - assert!( - app.scroll_offset < after_up, - "Down burst should reduce offset" - ); + handle_key(&mut app, key(KeyCode::PageUp)); + assert_eq!(app.content_scroll.offset, 10); + handle_key(&mut app, key(KeyCode::PageDown)); + assert_eq!(app.content_scroll.offset, 0); } diff --git a/crates/loopal-tui/tests/suite/scroll_compensation_test.rs b/crates/loopal-tui/tests/suite/scroll_compensation_test.rs new file mode 100644 index 00000000..f76d01b8 --- /dev/null +++ b/crates/loopal-tui/tests/suite/scroll_compensation_test.rs @@ -0,0 +1,147 @@ +/// Unit tests for ContentScroll growth compensation logic. +/// +/// Verifies that the viewport stays anchored when content grows while scrolled up, +/// and that auto-follow (offset=0) is not affected by content growth. +use std::sync::Arc; + +use ratatui::Terminal; +use ratatui::backend::TestBackend; +use tokio::sync::{mpsc, watch}; + +use loopal_protocol::{ + AgentEvent, AgentEventPayload, ControlCommand, InterruptSignal, UserQuestionResponse, +}; +use loopal_session::SessionController; +use loopal_tui::views::progress::ContentScroll; + +fn make_session() -> SessionController { + let (ctrl_tx, _) = mpsc::channel::(16); + let (perm_tx, _) = mpsc::channel::(16); + let (q_tx, _) = mpsc::channel::(16); + SessionController::new( + "test".into(), + "act".into(), + ctrl_tx, + perm_tx, + q_tx, + InterruptSignal::new(), + Arc::new(watch::channel(0u64).0), + ) +} + +fn stream_event(text: &str) -> AgentEvent { + AgentEvent::root(AgentEventPayload::Stream { + text: text.to_string(), + }) +} + +fn render_once( + terminal: &mut Terminal, + scroll: &mut ContentScroll, + session: &SessionController, +) { + let state = session.lock(); + terminal + .draw(|f| scroll.render(f, &state, f.area())) + .unwrap(); +} + +// --- Auto-follow: offset=0 stays at 0 --- + +#[test] +fn test_no_compensation_at_bottom() { + let session = make_session(); + let mut scroll = ContentScroll::new(); + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + + // Establish prev_total with some content + session.handle_event(stream_event("initial content\n")); + render_once(&mut terminal, &mut scroll, &session); + assert_eq!(scroll.offset, 0); + + // Add more streaming content + session.handle_event(stream_event("initial content\nmore lines\nand more\n")); + render_once(&mut terminal, &mut scroll, &session); + assert_eq!(scroll.offset, 0, "offset should stay 0 when at bottom"); +} + +// --- Pinned: offset grows with content --- + +#[test] +fn test_compensation_when_pinned() { + let session = make_session(); + let mut scroll = ContentScroll::new(); + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + + // Generate enough content to be scrollable + let long_text: String = (0..40).map(|i| format!("line {i}\n")).collect(); + session.handle_event(stream_event(&long_text)); + render_once(&mut terminal, &mut scroll, &session); + + // Scroll up + scroll.scroll_up(10); + assert_eq!(scroll.offset, 10); + + // Render with same content to establish prev_total at this offset + render_once(&mut terminal, &mut scroll, &session); + let offset_before = scroll.offset; + + // Add more streaming content + let more: String = (40..50).map(|i| format!("line {i}\n")).collect(); + let combined = format!("{long_text}{more}"); + session.handle_event(stream_event(&combined)); + render_once(&mut terminal, &mut scroll, &session); + + assert!( + scroll.offset > offset_before, + "offset should increase to compensate: before={offset_before}, after={}", + scroll.offset + ); +} + +// --- First frame: no blowup when prev_total=0 --- + +#[test] +fn test_no_blowup_first_render_with_offset() { + let session = make_session(); + let mut scroll = ContentScroll::new(); + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + + // Pre-set offset before any render (simulates scroll arriving before first paint) + scroll.scroll_up(5); + + let text: String = (0..30).map(|i| format!("line {i}\n")).collect(); + session.handle_event(stream_event(&text)); + render_once(&mut terminal, &mut scroll, &session); + + // Should NOT blow up: offset should remain small, not jump by total_lines + assert!( + scroll.offset < 100, + "first render should not cause massive offset: got {}", + scroll.offset + ); +} + +// --- Reset clears compensation state --- + +#[test] +fn test_reset_clears_prev_total() { + let session = make_session(); + let mut scroll = ContentScroll::new(); + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + + // Establish state with content + let text: String = (0..30).map(|i| format!("line {i}\n")).collect(); + session.handle_event(stream_event(&text)); + render_once(&mut terminal, &mut scroll, &session); + scroll.scroll_up(10); + render_once(&mut terminal, &mut scroll, &session); + + // Reset (simulates view switch) + scroll.reset(); + assert_eq!(scroll.offset, 0); + + // Render with content again — should not falsely compensate + render_once(&mut terminal, &mut scroll, &session); + assert_eq!(scroll.offset, 0, "after reset, should be at bottom"); +}