diff --git a/codex-rs/tui/docs.md b/codex-rs/tui/docs.md index 1b222acd9..b543dd497 100644 --- a/codex-rs/tui/docs.md +++ b/codex-rs/tui/docs.md @@ -491,6 +491,28 @@ State fields on `ChatWidget`: `loop_remaining: Option` and `loop_total: Opt The loop is cancelled (both fields set to `None`) when an error occurs (`on_error()`) or the user interrupts (`on_interrupted_turn()`). The `/config` sub-picker is a custom `BottomPaneView` implemented by `LoopCountPickerView` in `@/codex-rs/tui/src/nori/loop_count_picker.rs`. It offers preset options (Disabled, 2, 3, 5, 10) plus a "Custom..." option that enters an input mode where the user can type an arbitrary number (2-1000). Values <= 1 are treated as disabled, values > 1000 are capped. This follows the same `BottomPaneView` pattern used by `HotkeyPickerView`. The setting persists to `[tui]` in `config.toml` via `persist_loop_count_setting()`. +**History Insertion and Scrollback (`insert_history.rs`):** + +`insert_history_lines()` pushes content into the terminal's native scrollback buffer above the ratatui viewport without disturbing ratatui's diff-based renderer. It works by manipulating ANSI scroll regions (DECSTBM, `\x1b[Pt;Pbr`) directly against the crossterm backend writer, bypassing the normal ratatui render pass. + +The insertion algorithm: + +``` +1. If viewport is not at screen bottom: scroll viewport downward using RI (ESC M) inside + a temporary scroll region covering [viewport.top()+1 .. screen_height]. +2. Early return if area.top() == 0 (viewport fills the whole screen; no space above it). +3. Set scroll region to [1 .. area.top()] (only the history area above the viewport). +4. Write lines into that region with \r\n advancement. +5. Reset scroll region to full screen. +6. Restore cursor to its pre-call position. +``` + +The critical invariant: **DECSTBM `Pb=0` means "bottom of screen"**, not row 0. Calling `SetScrollRegion(1..0)` when `area.top() == 0` produces `\x1b[1;0r`, which sets the scroll region to the entire terminal rather than an empty region. Any subsequent writes then scroll through the viewport, corrupting ratatui's content in ways the diff-based renderer cannot detect. The `area.top() == 0` early return guards against this. + +Two crossterm `Command` implementations support the function: +- `SetScrollRegion(Range)` — emits `\x1b[{start};{end}r` +- `ResetScrollRegion` — emits `\x1b[r` (restores full-screen scrolling) + ### Things to Know **Module Structure Convention:** diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 36ef47da5..5ea6f8f64 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -70,6 +70,12 @@ where area.top().saturating_sub(1) }; + // No room above the viewport for history lines. + if area.top() == 0 { + let _ = writer; + return Ok(()); + } + // Limit the scroll region to the lines from the top of the screen to the // top of the viewport. With this in place, when we add lines inside this // area, only the lines in this area will be scrolled. We place the cursor @@ -527,4 +533,109 @@ mod tests { ); } } + + /// When the viewport occupies the entire screen (area.top() == 0), there is + /// no room above the viewport. insert_history_lines must not corrupt the + /// viewport content by writing through a degenerate scroll region. + #[test] + fn full_screen_viewport_does_not_corrupt_display() { + let width: u16 = 40; + let height: u16 = 10; + let backend = VT100Backend::new(width, height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + + // Viewport fills the entire screen: y=0, height=10 + let viewport = Rect::new(0, 0, width, height); + term.set_viewport_area(viewport); + + // Draw some known content into the viewport first + term.draw(|frame| { + let buf = frame.buffer_mut(); + for y in 0..height { + let text = format!("Row {y}"); + buf.set_string(0, y, &text, ratatui::style::Style::default()); + } + }) + .expect("draw"); + // Flush the draw output so vt100 sees it + Backend::flush(term.backend_mut()).expect("flush"); + + // Capture the screen contents before insert_history_lines + let before: Vec = term.backend().vt100().screen().rows(0, width).collect(); + + // Now try to insert history lines — there's no room above the viewport + let line = Line::from("This should not corrupt the display"); + insert_history_lines(&mut term, vec![line]).expect("insert"); + Backend::flush(term.backend_mut()).expect("flush"); + + // The viewport content must be unchanged + let after: Vec = term.backend().vt100().screen().rows(0, width).collect(); + + pretty_assertions::assert_eq!( + before, + after, + "viewport content was corrupted by insert_history_lines when area.top()==0" + ); + } + + /// When there IS room above the viewport, history lines should appear + /// above the viewport and the viewport content should be preserved. + #[test] + fn history_lines_inserted_above_viewport_with_room() { + let width: u16 = 40; + let height: u16 = 10; + let viewport_y: usize = 5; + let viewport_h: usize = 5; + let backend = VT100Backend::new(width, height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + + // Viewport at y=5 with height=5, leaving 5 rows above for history + let viewport = Rect::new(0, viewport_y as u16, width, viewport_h as u16); + term.set_viewport_area(viewport); + + // Draw known viewport content + term.draw(|frame| { + let area = frame.area(); + let buf = frame.buffer_mut(); + for i in 0..area.height { + let text = format!("Viewport row {i}"); + buf.set_string(area.x, area.y + i, &text, ratatui::style::Style::default()); + } + }) + .expect("draw"); + Backend::flush(term.backend_mut()).expect("flush"); + + // Insert a history line + let line = Line::from("History entry"); + insert_history_lines(&mut term, vec![line]).expect("insert"); + Backend::flush(term.backend_mut()).expect("flush"); + + let rows: Vec = term.backend().vt100().screen().rows(0, width).collect(); + + // The history line should appear somewhere above the viewport + let history_row = rows + .iter() + .position(|r| r.contains("History entry")) + .unwrap_or_else(|| { + panic!("expected 'History entry' above viewport, got rows: {rows:?}") + }); + assert!( + history_row < viewport_y, + "history line at row {history_row} should be above viewport at y={viewport_y}", + ); + + // Viewport content should still be intact — find it by searching for + // "Viewport row 0" and checking consecutive rows from there. + let vp_start = rows + .iter() + .position(|r| r.contains("Viewport row 0")) + .expect("could not find 'Viewport row 0' on screen"); + for i in 0..viewport_h { + let row_text = &rows[vp_start + i]; + assert!( + row_text.contains(&format!("Viewport row {i}")), + "viewport row {i} should contain 'Viewport row {i}', got: {row_text:?}" + ); + } + } }