diff --git a/src/play/mod.rs b/src/play/mod.rs index fe03c15..fb00116 100644 --- a/src/play/mod.rs +++ b/src/play/mod.rs @@ -17,6 +17,7 @@ mod editor; mod history; pub mod keys; pub mod screen; +pub mod sync; #[cfg(not(coverage))] mod terminal; mod thinking; diff --git a/src/play/screen.rs b/src/play/screen.rs index f698c88..b97e70d 100644 --- a/src/play/screen.rs +++ b/src/play/screen.rs @@ -1,7 +1,7 @@ //! The render loop: the live region where a reply streams, the prompt the //! player types on, and the transcript above them both. -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::Ordering; use std::sync::mpsc::{Receiver, Sender, TryRecvError}; use ratatui::Frame; @@ -14,8 +14,9 @@ use ratatui::text::Text; use super::editor::Editor; use super::history::History; use super::keys::{Key, Keys}; +use super::sync::SyncGuard; use super::thinking; -use super::worker::TurnEvent; +use super::worker::{TurnEvent, Worker}; use super::wrap::{wrap, wrap_spans}; /// The rows the inline viewport takes at the bottom of the terminal. @@ -76,19 +77,25 @@ struct Screen { /// Runs the game until the player quits. /// -/// Reads keys from `keys`, sends submitted lines to `inputs`, and shows -/// what comes back on `turns`. Finished lines go above the viewport, into -/// the terminal's own scrollback, so they survive the game. `history` holds -/// the prompts the player can recall with Up and Down. Escape while a turn -/// is running sets `cancel`, which the worker checks to stop the turn. -pub fn play( +/// Reads keys from `keys`, sends submitted lines to `worker.inputs`, and +/// shows what comes back on `worker.events`. Finished lines go above the +/// viewport, into the terminal's own scrollback, so they survive the game. +/// `history` holds the prompts the player can recall with Up and Down. +/// Escape while a turn is running sets `worker.cancel`, which the worker +/// checks to stop the turn. +/// +/// `guard` brackets every render pass in a synchronized update, so the +/// terminal paints each pass in one go instead of painting whatever has +/// landed on the wire so far. The opening banner paints alone, before the +/// loop's first pass and nothing else, so there is nothing for it to race +/// and no update to bracket it with. +pub fn play( terminal: &mut Terminal, keys: &mut K, - inputs: &Sender, - turns: &Receiver, - cancel: &AtomicBool, + worker: &Worker, banner: &str, history: &mut History, + guard: &mut G, ) -> Result<(), B::Error> { let mut screen = Screen { input: Editor::default(), @@ -101,8 +108,13 @@ pub fn play( insert(terminal, &mut screen, banner, aside())?; loop { screen.tick = screen.tick.wrapping_add(1); - drain(&mut screen, terminal, turns)?; + guard.begin(); + // An error here returns with the guard still open. That is fine: + // the caller exits the program on this path, and a terminal that + // never sees a matching `end` recovers on its own safety timeout. + drain(&mut screen, terminal, &worker.events)?; terminal.draw(|frame| render(&screen, frame))?; + guard.end(); match keys.next_key() { None => {} Some(Key::Quit) => return Ok(()), @@ -130,8 +142,8 @@ pub fn play( let entry = history.down(); recall(&mut screen, entry); } - Some(Key::Enter) => submit(&mut screen, terminal, inputs, history)?, - Some(Key::Cancel) if screen.busy => cancel.store(true, Ordering::Relaxed), + Some(Key::Enter) => submit(&mut screen, terminal, &worker.inputs, history, guard)?, + Some(Key::Cancel) if screen.busy => worker.cancel.store(true, Ordering::Relaxed), Some(Key::Cancel) => {} } } @@ -152,18 +164,25 @@ fn recall(screen: &mut Screen, entry: Option) { /// the prompt until the turn ends. A send that fails means the worker is /// gone, which leaves nothing to do but keep drawing until the player /// quits. -fn submit( +/// +/// This insert happens while a key is handled, after the loop pass's own +/// `end`, so it brackets itself in its own synchronized update rather than +/// riding along with the next pass. +fn submit( screen: &mut Screen, terminal: &mut Terminal, inputs: &Sender, history: &mut History, + guard: &mut G, ) -> Result<(), B::Error> { if screen.busy || screen.input.text().trim().is_empty() { return Ok(()); } let input = screen.input.take(); let line = format!("{PLAYER_MARKER}{input}"); + guard.begin(); insert(terminal, screen, &line, player())?; + guard.end(); screen.busy = true; let _ = inputs.send(input.clone()); history.record(&input); @@ -367,3 +386,7 @@ mod tests; #[cfg(test)] #[path = "screen_turn_tests.rs"] mod turn_tests; + +#[cfg(test)] +#[path = "screen_sync_tests.rs"] +mod sync_tests; diff --git a/src/play/screen_sync_tests.rs b/src/play/screen_sync_tests.rs new file mode 100644 index 0000000..9a1b113 --- /dev/null +++ b/src/play/screen_sync_tests.rs @@ -0,0 +1,133 @@ +//! Tests that the play loop paints atomically: every render pass and the +//! submit insert bracket themselves in a synchronized update, the pairs +//! balance, updates never nest, and no update stays open across a key +//! poll. Shares `screen_tests.rs`'s harness rather than keeping its own +//! copy. + +use std::cell::RefCell; +use std::rc::Rc; + +use ratatui::text::Text; + +use super::tests::{Step, done, play_script, press, typing}; +use super::*; + +/// One step in the timeline a `RecordingGuard` and the harness's `Script` +/// keys fake share: a synchronized update opening or closing, or a key +/// poll landing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum Recorded { + Begin, + End, + Poll, +} + +/// A `SyncGuard` that appends every `begin` and `end` call to a timeline +/// shared with the harness's `Script` keys fake, so a test can see +/// whether a poll ever landed while a pass was still open. +#[derive(Clone, Default)] +pub(super) struct RecordingGuard { + pub(super) timeline: Rc>>, +} + +impl SyncGuard for RecordingGuard { + fn begin(&mut self) { + self.timeline.borrow_mut().push(Recorded::Begin); + } + + fn end(&mut self) { + self.timeline.borrow_mut().push(Recorded::End); + } +} + +/// Whether `timeline` never lets a poll land while a pass is open, and +/// every `Begin` is matched by exactly one `End` before the next `Begin`. +pub(super) fn brackets_hold(timeline: &[Recorded]) -> bool { + let closed = timeline + .iter() + .try_fold(false, |open, event| match (open, event) { + (false, Recorded::Begin) => Some(true), + (true, Recorded::End) => Some(false), + (false, Recorded::Poll) => Some(false), + _ => None, + }); + closed == Some(false) +} + +#[test] +fn an_idle_pass_begins_and_ends_before_polling() { + let played = play_script(vec![]); + + assert_eq!( + played.guard_timeline(), + vec![Recorded::Begin, Recorded::End, Recorded::Poll] + ); +} + +#[test] +fn submitting_brackets_its_own_insert_apart_from_the_pass_that_read_enter() { + let mut steps = typing("hi"); + steps.push(press(Key::Enter)); + + let played = play_script(steps); + let timeline = played.guard_timeline(); + + // Three passes read a key (h, i, Enter), and the submit that Enter + // triggers brackets its own insert, for five pairs in all. + let begins = timeline + .iter() + .filter(|event| **event == Recorded::Begin) + .count(); + let ends = timeline + .iter() + .filter(|event| **event == Recorded::End) + .count(); + assert_eq!(begins, 5); + assert_eq!(ends, 5); +} + +#[test] +fn a_turn_with_a_tool_call_and_a_cancel_keeps_every_pass_bracketed() { + let mut steps = typing("hi"); + steps.push(press(Key::Enter)); + steps.push(Step::Turn(TurnEvent::Tool(Text::raw("a tool line")))); + steps.push(done()); + steps.push(press(Key::Cancel)); + + let played = play_script(steps); + + assert!(brackets_hold(&played.guard_timeline())); +} + +#[test] +fn brackets_hold_accepts_a_balanced_timeline() { + let timeline = vec![Recorded::Begin, Recorded::End, Recorded::Poll]; + + assert!(brackets_hold(&timeline)); +} + +#[test] +fn brackets_hold_rejects_a_nested_begin() { + let timeline = vec![ + Recorded::Begin, + Recorded::Begin, + Recorded::End, + Recorded::End, + ]; + + assert!(!brackets_hold(&timeline)); +} + +#[test] +fn brackets_hold_rejects_a_poll_while_open() { + let timeline = vec![Recorded::Begin, Recorded::Poll, Recorded::End]; + + assert!(!brackets_hold(&timeline)); +} + +#[test] +fn brackets_hold_rejects_an_unmatched_begin() { + let timeline = vec![Recorded::Begin]; + + assert!(!brackets_hold(&timeline)); +} diff --git a/src/play/screen_tests.rs b/src/play/screen_tests.rs index 058c06b..d54c6c6 100644 --- a/src/play/screen_tests.rs +++ b/src/play/screen_tests.rs @@ -5,7 +5,9 @@ //! this file's harness rather than keeping its own copy, so `play`'s //! test-only type parameter is monomorphized once, not twice. +use std::cell::RefCell; use std::collections::VecDeque; +use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::mpsc::{self, Receiver, Sender, TryRecvError}; @@ -15,6 +17,7 @@ use ratatui::buffer::Buffer; use ratatui::layout::Position; use ratatui::{Terminal, TerminalOptions, Viewport}; +use super::sync_tests::{Recorded, RecordingGuard}; use super::*; /// One thing that happens while the loop runs: the player presses a key, @@ -40,13 +43,19 @@ pub(super) fn typing(text: &str) -> Vec { /// sending half of that channel, the same as a worker thread that panics /// mid-turn, and likewise reports no key. Quitting at the end of the /// script keeps every test bounded. +/// +/// `timeline` is the same timeline the play's `RecordingGuard` writes to. +/// Recording a poll onto it here lets a test see whether one ever landed +/// while a synchronized update was still open. struct Script { steps: VecDeque, turns: Option>, + timeline: Rc>>, } impl Keys for Script { fn next_key(&mut self) -> Option { + self.timeline.borrow_mut().push(Recorded::Poll); match self.steps.pop_front() { Some(Step::Press(key)) => Some(key), Some(Step::Turn(event)) => { @@ -67,8 +76,10 @@ pub(super) struct Played { terminal: Terminal, inputs: Receiver, pub(super) cancel: Arc, - /// Held open so an unsubmitted input reads as empty, not disconnected. - _sender: Sender, + /// Holds the worker stub's channels open, so an unsubmitted input on + /// `inputs` reads as empty, not disconnected. + _worker: Worker, + guard_timeline: Rc>>, } impl Played { @@ -91,6 +102,12 @@ impl Played { pub(super) fn submitted(&self) -> String { self.inputs.try_recv().unwrap() } + + /// The guard's begin/end calls, merged with every key poll, in the + /// order they happened. + pub(super) fn guard_timeline(&self) -> Vec { + self.guard_timeline.borrow().clone() + } } /// Joins every row of `buffer`, trailing spaces trimmed, one row per line. @@ -120,21 +137,27 @@ pub(super) fn play_script(steps: Vec) -> Played { .unwrap(); let (input_sender, inputs) = mpsc::channel(); let (turn_sender, turns) = mpsc::channel(); + let mut guard = RecordingGuard::default(); let mut keys = Script { steps: steps.into(), turns: Some(turn_sender), + timeline: Rc::clone(&guard.timeline), }; let mut history = History::in_memory(); let cancel = Arc::new(AtomicBool::new(false)); + let worker = Worker { + inputs: input_sender, + events: turns, + cancel: Arc::clone(&cancel), + }; play( &mut terminal, &mut keys, - &input_sender, - &turns, - &cancel, + &worker, "a banner", &mut history, + &mut guard, ) .unwrap(); @@ -142,7 +165,8 @@ pub(super) fn play_script(steps: Vec) -> Played { terminal, inputs, cancel, - _sender: input_sender, + _worker: worker, + guard_timeline: guard.timeline, } } diff --git a/src/play/sync.rs b/src/play/sync.rs new file mode 100644 index 0000000..5c314e5 --- /dev/null +++ b/src/play/sync.rs @@ -0,0 +1,29 @@ +//! The guard around one atomic terminal repaint. + +/// Brackets one render pass so the terminal paints it as a single update +/// instead of painting whatever has been written to it so far. +/// +/// `begin` starts a synchronized update, `CSI ?2026h`. From that point the +/// terminal holds the last frame it painted and queues every write that +/// follows without showing any of them. `end` sends the matching +/// `CSI ?2026l`, and the terminal paints everything queued since `begin` +/// in one pass. A terminal that does not support the sequence ignores +/// both codes and paints as it always did, so the guard is safe to use +/// unconditionally. +/// +/// The guard must close before the play loop polls for a key. A poll can +/// wait up to the loop's poll interval, and an open guard freezes the +/// display for as long as it stays open: leaving it open across the poll +/// would freeze the terminal between frames, not just during a repaint. +/// Terminals also carry their own safety timeout that ends an update on +/// its own a few seconds after `begin` with no matching `end`, but that +/// timeout exists to recover from a crash. It is not a substitute for +/// closing the guard on every pass. +pub trait SyncGuard { + /// Starts a synchronized update. + fn begin(&mut self); + + /// Ends a synchronized update, painting everything written since the + /// matching `begin`. + fn end(&mut self); +} diff --git a/src/play/terminal.rs b/src/play/terminal.rs index 58d3cb9..c2a6c9e 100644 --- a/src/play/terminal.rs +++ b/src/play/terminal.rs @@ -12,6 +12,7 @@ use std::time::Duration; use crossterm::cursor::MoveTo; use crossterm::event; use crossterm::execute; +use crossterm::terminal::{BeginSynchronizedUpdate, EndSynchronizedUpdate}; use ratatui::layout::Rect; use ratatui::{TerminalOptions, Viewport}; @@ -21,6 +22,7 @@ use crate::dm::Dm; use super::history::History; use super::keys::{self, Key, Keys}; use super::screen::{self, VIEWPORT_HEIGHT}; +use super::sync::SyncGuard; use super::worker::Worker; /// How long to wait for a key before looking at the worker again. @@ -52,11 +54,10 @@ pub fn run(overrides: &Overrides) -> Result<(), String> { let played = screen::play( &mut terminal, &mut CrosstermKeys, - &worker.inputs, - &worker.events, - &worker.cancel, + &worker, &banner, &mut history, + &mut CrosstermSyncGuard, ); let viewport = terminal.get_frame().area(); ratatui::restore(); @@ -98,3 +99,19 @@ impl Keys for CrosstermKeys { } } } + +/// The play loop's synchronized-update guard, backed by stdout. +struct CrosstermSyncGuard; + +impl SyncGuard for CrosstermSyncGuard { + /// A failed write here means the terminal is already in trouble, and + /// the next draw will fail too and end the game; there is nothing + /// useful to do with the error before then. + fn begin(&mut self) { + let _ = execute!(std::io::stdout(), BeginSynchronizedUpdate); + } + + fn end(&mut self) { + let _ = execute!(std::io::stdout(), EndSynchronizedUpdate); + } +}