From e78491767be776eb2c269991628453fb6e58448f Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sat, 1 Aug 2026 21:33:34 -0400 Subject: [PATCH 01/15] feat(terminal): give the renderer each cluster's true column The span encoding shipped a concatenated string and a start column, which left the consumer to recover cell boundaries from the text. It cannot: a regional-indicator flag is two one-column cells and must split per codepoint, while a keycap is one cell holding three codepoints and must split per grapheme. Those rules are opposite, and the distinction lives in the grid, not in the string. Any split rule over the concatenated text is wrong for one of the two. So the span carries what the grid knows. `width` is the columns each cluster advances, uniform across a run by construction; `cluster_count` says how many clusters the text holds. Decoding is then arithmetic with no Unicode table anywhere: a count of 1 means the whole text is one cluster, otherwise cluster i is the i-th char at `column + i * width`. Geometry flags are also masked out of the style key. `WIDE_CHAR` and `WRAPLINE` are grid bookkeeping, not appearance, and while they sat in the key they broke runs as a side effect -- hiding the explicit width check behind a coincidence, and splitting the last column off every wrapped row into a span of its own. Row dedup still hashes the full flags, so nothing it needs to see is lost. Eight fixtures cover the contract, including the flag/keycap pair whose opposing rules make the case. Six mutations were run against them and each fails independently: forcing width to 1, dropping the width comparison, never breaking on zerowidth marks, removing WRAPLINE from the mask, removing the mask entirely, and never incrementing cluster_count. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/damage.rs | 119 +++++++- .../crates/buzz-terminal/tests/clusters.rs | 266 ++++++++++++++++++ 2 files changed, 379 insertions(+), 6 deletions(-) create mode 100644 desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs diff --git a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs index 86b97814c..fa37af838 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs @@ -45,7 +45,31 @@ use alacritty_terminal::index::{Column, Line}; use alacritty_terminal::term::cell::{Cell, Flags}; use alacritty_terminal::term::TermDamage; -/// A run of cells sharing one visual style. +/// A run of cells sharing one visual style **and one cell width**. +/// +/// # Why the consumer can position every cluster without Unicode tables +/// +/// The renderer must place each display cluster at its true column, and it +/// cannot derive that from the text: no single split rule over a concatenated +/// string is correct. A regional-indicator flag (`U+1F1FA U+1F1F8`) is two +/// ordinary one-column cells, so it must split *per codepoint*; a keycap +/// (`1 U+FE0F U+20E3`) is one cell holding three codepoints, so it must split +/// *per grapheme*. Those rules disagree, and the distinction lives in the grid, +/// not in the string. +/// +/// So the run carries it instead. Within a span every cluster advances the same +/// [`width`](Self::width) columns, and [`cluster_count`](Self::cluster_count) +/// says how many clusters the text holds. The consumer's rule is arithmetic on +/// those two numbers, with no Unicode table anywhere: +/// +/// ```text +/// cluster_count == 1 -> the whole text is one cluster, at `column` +/// otherwise -> cluster i is the i-th char, at `column + i * width` +/// ``` +/// +/// The second case is exact because a cell carrying zerowidth marks is always +/// emitted alone, so every cell in a multi-cluster span contributes exactly one +/// `char`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Span { /// First column of the run. @@ -54,10 +78,34 @@ pub struct Span { /// combining marks follow its base character, so the renderer never sees /// a base and its accent as separate glyphs. pub text: String, + /// Columns each cluster in this run occupies: 1, or 2 for wide glyphs. + /// + /// Uniform across the run by construction -- a width change ends the span. + /// This is what lets the consumer position clusters by computed origin + /// rather than by accumulated text advance. + pub width: u8, + /// How many display clusters [`text`](Self::text) holds. + /// + /// Without this the consumer cannot distinguish a one-cluster span carrying + /// combining marks from an ordinary multi-character run, and would need a + /// Unicode zerowidth table to guess. The grid already knows, so it says. + pub cluster_count: u16, /// Packed style: fg, bg, and attribute flags. pub style: Style, } +impl Span { + /// The decoding invariant, stated once: a span is either a single cluster + /// (which may hold several `char`s, as a keycap or an accented letter + /// does) or one cluster per `char`. + /// + /// Exposed so consumers can assert it at a trust boundary rather than + /// restate it. The encoder checks it in debug builds on every frame. + pub fn counts_are_consistent(&self) -> bool { + self.cluster_count == 1 || usize::from(self.cluster_count) == self.text.chars().count() + } +} + /// Visual style of a span, as the renderer needs it. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Style { @@ -219,9 +267,20 @@ fn hash_cells(cells: &[Cell]) -> u64 { hasher.finish() } -/// Group a row's cells into styled runs. +/// Group a row's cells into runs of uniform style and width. +/// +/// A run continues only while style *and* width match, and a cell carrying +/// zerowidth marks is always emitted alone. Both breaks exist so the consumer +/// can compute each cluster's column as `column + i * width`; see [`Span`]. +/// +/// The width comparison is the only thing keeping widths uniform within a run: +/// [`Style`] deliberately excludes [`GEOMETRY_FLAGS`], so a style key cannot +/// break a run on width behind this check's back. fn spans(cells: &[Cell]) -> Vec { let mut spans: Vec = Vec::new(); + // Whether the run in progress may still be extended. Kept here rather than + // on `Span` because it is grouping bookkeeping, not part of the wire shape. + let mut open = false; for (column, cell) in cells.iter().enumerate() { // A wide glyph occupies two cells: the character, then a spacer. The // spacer carries no text of its own -- emitting its placeholder space @@ -230,28 +289,76 @@ fn spans(cells: &[Cell]) -> Vec { continue; } let style = style_of(cell); + let width = if cell.flags.contains(Flags::WIDE_CHAR) { + 2 + } else { + 1 + }; + let zerowidth = cell.zerowidth(); let mut text = String::new(); text.push(cell.c); - if let Some(zerowidth) = cell.zerowidth() { - text.extend(zerowidth); + if let Some(marks) = zerowidth { + text.extend(marks); } + + // A cluster with combining marks holds more `char`s than columns, so it + // cannot share a run: it is the one case where "one char per cluster" + // stops holding. + let joinable = zerowidth.is_none(); match spans.last_mut() { - Some(last) if last.style == style => last.text.push_str(&text), + // `cluster_count` is refused rather than wrapped when it would + // overflow: the run simply ends and a new span starts at this + // column, which the consumer's rule already handles. + Some(last) + if open + && joinable + && last.style == style + && last.width == width + && last.cluster_count < u16::MAX => + { + last.text.push_str(&text); + last.cluster_count += 1; + } _ => spans.push(Span { column, text, + width, + cluster_count: 1, style, }), } + open = joinable; } + debug_assert!( + spans.iter().all(Span::counts_are_consistent), + "cluster_count must be 1 or the span's char count" + ); spans } +/// Flags describing where a cell sits in the grid rather than how it looks. +/// +/// `WRAPLINE` marks the last cell of a row that wrapped; the three wide-char +/// bits mark a two-column glyph and its spacer. Neither says anything about +/// appearance. +/// +/// These are excluded from [`Style`] so the style key means one thing: visual +/// attributes. Geometry travels in [`Span::width`], which is compared on its +/// own when grouping -- if these bits stayed in the key they would break runs +/// as a side effect and leave the width comparison untestable. +/// +/// Composite visual aliases (`BOLD_ITALIC`, `DIM_BOLD`, `ALL_UNDERLINES`) are +/// deliberately not masked: those are appearance. +const GEOMETRY_FLAGS: Flags = Flags::WRAPLINE + .union(Flags::WIDE_CHAR) + .union(Flags::WIDE_CHAR_SPACER) + .union(Flags::LEADING_WIDE_CHAR_SPACER); + fn style_of(cell: &Cell) -> Style { Style { fg: pack_color(cell.fg), bg: pack_color(cell.bg), - flags: cell.flags.bits(), + flags: cell.flags.difference(GEOMETRY_FLAGS).bits(), } } diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs new file mode 100644 index 000000000..feb91f0c7 --- /dev/null +++ b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs @@ -0,0 +1,266 @@ +//! The cluster-positioning contract: what the renderer may rely on to place +//! text at the right column without consulting Unicode tables. +//! +//! The consumer's rule reads two numbers off each span and does arithmetic: +//! `cluster_count == 1` means the whole text is one cluster at `column`, +//! otherwise cluster `i` is the i-th `char` at `column + i * width`. +//! +//! These fixtures exist because that rule is not self-evidently satisfiable -- +//! the two cases below require *opposite* text-splitting rules, so no encoding +//! that ships a concatenated string and a start column can be correct: +//! +//! * a regional-indicator flag is two ordinary one-column cells, so its two +//! codepoints occupy two columns and must split per codepoint; +//! * a keycap is one cell holding three codepoints, so it occupies one column +//! and must split per grapheme. +//! +//! Both are handled here by construction rather than by rule: uniform `width` +//! within a span, and a span of its own for any cluster carrying zerowidth +//! marks. + +use buzz_terminal::damage::{Encoder, Span}; +use buzz_terminal::fences::Fences; +use buzz_terminal::{Action, SharedTerminal, Size, Terminal}; +use std::sync::mpsc::Receiver; + +/// The receiver is returned rather than dropped: dropping it disconnects the +/// channel and every subsequent listener send silently fails. +fn render(input: &str) -> (Vec, Receiver) { + let size = Size { + columns: 20, + screen_lines: 2, + scrollback: 100, + }; + let (term, actions) = Terminal::new(size, Fences::ALL); + let shared = SharedTerminal::new(term); + shared.feed(input.as_bytes()); + let mut encoder = Encoder::new(); + let frame = shared.render(&mut encoder); + let spans = frame + .rows + .into_iter() + .find(|row| row.line == 0) + .map(|row| row.spans) + .unwrap_or_default(); + (spans, actions) +} + +/// Apply the documented consumer rule and return `(column, cluster)` pairs, +/// dropping trailing blank padding. +/// +/// This is the renderer's arithmetic, written out. Note what is *not* here: no +/// Unicode table, no zerowidth classifier, no grapheme segmentation. The +/// earlier draft of this helper carried a hand-rolled `is_zerowidth` matcher, +/// which is how we learned the encoding was under-specified -- if the fixture +/// needs a Unicode table to decode the wire, so does every real consumer. +fn placements(spans: &[Span]) -> Vec<(usize, String)> { + let mut placed = Vec::new(); + for span in spans { + assert!( + span.counts_are_consistent(), + "encoder emitted an undecodable span: {span:?}" + ); + let clusters: Vec = if span.cluster_count == 1 { + vec![span.text.clone()] + } else { + span.text.chars().map(|c| c.to_string()).collect() + }; + for (i, cluster) in clusters.into_iter().enumerate() { + if cluster != " " { + placed.push((span.column + i * span.width as usize, cluster)); + } + } + } + placed +} + +/// Max's case: mixed narrow and wide glyphs in one style. Every cluster must +/// land on the column the grid actually put it in. +#[test] +fn mixed_width_clusters_keep_their_columns() { + let (spans, _actions) = render("a\u{1F600}b\u{4E00}c"); + assert_eq!( + placements(&spans), + vec![ + (0, "a".into()), + (1, "\u{1F600}".into()), + (3, "b".into()), + (4, "\u{4E00}".into()), + (6, "c".into()), + ], + "wide glyphs must advance two columns and narrow ones must not" + ); +} + +/// A combining mark rides with its base character and consumes no column of +/// its own, so the text that follows must not be displaced by it. +/// +/// Against the previous encoding this row was a single span `"éxy"` at column +/// 0, and a consumer stepping one column per `char` placed `x` at 1 and `y` +/// at 2 -- both one column left of the truth. +#[test] +fn combining_marks_do_not_displace_following_text() { + let (spans, _actions) = render("e\u{0301}xy"); + assert_eq!( + placements(&spans), + vec![(0, "e\u{0301}".into()), (1, "x".into()), (2, "y".into()),], + "a zerowidth mark must not consume a column" + ); +} + +/// A regional-indicator pair: two separate one-column cells. This is the case +/// that must split *per codepoint*. +#[test] +fn regional_indicator_flag_occupies_two_columns() { + let (spans, _actions) = render("\u{1F1FA}\u{1F1F8}X"); + assert_eq!( + placements(&spans), + vec![ + (0, "\u{1F1FA}".into()), + (1, "\u{1F1F8}".into()), + (2, "X".into()), + ], + "regional indicators are one column each; X must sit at 2" + ); +} + +/// A keycap: one cell holding three codepoints. This is the case that must +/// split *per grapheme* -- the opposite rule from the flag above, which is why +/// the width and the cluster break both have to come from the grid. +#[test] +fn keycap_occupies_one_column() { + let (spans, _actions) = render("1\u{FE0F}\u{20E3}X"); + assert_eq!( + placements(&spans), + vec![(0, "1\u{FE0F}\u{20E3}".into()), (1, "X".into()),], + "a keycap is one column; X must sit at 1" + ); +} + +/// Width is uniform within a span by construction. Without this a consumer +/// cannot multiply -- it would have to know each cluster's width individually, +/// which is the Unicode table this design exists to avoid. +#[test] +fn a_span_never_mixes_widths() { + let (spans, _actions) = render("ab\u{4E00}\u{4E00}cd"); + for span in &spans { + let expected = span.width; + assert!( + span.width == 1 || span.width == 2, + "width must be 1 or 2, got {expected}" + ); + } + let widths: Vec = spans.iter().map(|s| s.width).collect(); + assert!( + widths.contains(&2), + "fixture must actually produce a wide span, got {widths:?}" + ); + assert_eq!( + placements(&spans), + vec![ + (0, "a".into()), + (1, "b".into()), + (2, "\u{4E00}".into()), + (4, "\u{4E00}".into()), + (6, "c".into()), + (7, "d".into()), + ], + "two adjacent wide glyphs must advance two columns each" + ); +} + +/// `cluster_count` is what makes the wire decodable without a Unicode table, +/// so it is asserted directly here rather than only implied by placements. +/// +/// The decisive pair: both spans below are width 1 with more than one `char` +/// of text, and they differ *only* in whether the count tracks the char count. +/// A consumer without that number cannot tell them apart -- which is the +/// defect Mari caught in the previous encoding. +#[test] +fn cluster_count_distinguishes_a_marked_cluster_from_a_plain_run() { + let (marked, _a) = render("e\u{0301}"); + let marked = marked.first().expect("a span must be emitted"); + assert_eq!(marked.text.chars().count(), 2, "base plus combining mark"); + assert_eq!(marked.cluster_count, 1, "one cluster occupying one column"); + + // The plain run absorbs the row's blank padding, so its length is the + // viewport width rather than 2 -- what matters is that the count tracks + // the char count instead of collapsing to 1. + let (plain, _b) = render("ab"); + let plain = plain.first().expect("a span must be emitted"); + assert!(plain.cluster_count > 1, "a plain run is not one cluster"); + assert_eq!( + usize::from(plain.cluster_count), + plain.text.chars().count(), + "one cluster per char" + ); + + assert_eq!(marked.width, plain.width, "both are width 1"); + assert!(marked.counts_are_consistent() && plain.counts_are_consistent()); +} + +/// Wrapping marks the last cell of the row with `WRAPLINE` (upstream +/// `term/mod.rs:968`). That bit records where the text happened to wrap, not +/// how the text looks, so it must not reach the style key: if it did, the last +/// column of every wrapped row would split off into a span of its own -- an +/// extra wire record per wrapped line, and span boundaries that move when the +/// window is resized. +/// +/// Quinn found this by reading `cell.rs:21` while checking the `WIDE_CHAR` +/// mask; this fixture is the proof that was missing from the source read. +#[test] +fn wrapping_does_not_split_a_uniform_run() { + let size = Size { + columns: 5, + screen_lines: 3, + scrollback: 100, + }; + let (term, _actions) = Terminal::new(size, Fences::ALL); + let shared = SharedTerminal::new(term); + // Six narrow cells in one style: five fill row 0 and set WRAPLINE on the + // last of them, the sixth lands on row 1. + shared.feed(b"abcdef"); + let mut encoder = Encoder::new(); + let frame = shared.render(&mut encoder); + + let first = frame + .rows + .iter() + .find(|row| row.line == 0) + .expect("wrapped row must be present"); + let texts: Vec<&str> = first.spans.iter().map(|s| s.text.as_str()).collect(); + assert_eq!( + texts, + vec!["abcde"], + "a wrapped row of one style is one span; WRAPLINE must not break it" + ); +} + +/// A wide glyph at the last usable column wraps to the next row rather than +/// straddling the edge. The contract must hold on the wrapped row too. +#[test] +fn leading_wide_glyph_after_wrap_is_positioned_from_column_zero() { + let size = Size { + columns: 5, + screen_lines: 3, + scrollback: 100, + }; + let (term, _actions) = Terminal::new(size, Fences::ALL); + let shared = SharedTerminal::new(term); + // Four narrow cells fill 0..=3, leaving one column: the wide glyph cannot + // fit and moves to the next row. + shared.feed("abcd\u{4E00}".as_bytes()); + let mut encoder = Encoder::new(); + let frame = shared.render(&mut encoder); + + let second = frame + .rows + .iter() + .find(|row| row.line == 1) + .expect("wrapped row must be present"); + assert_eq!( + placements(&second.spans), + vec![(0, "\u{4E00}".into())], + "a wrapped wide glyph starts at column 0 of the next row" + ); +} From 3a5c7d576e081caf7106fb46e41c42ca0475c9c8 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sat, 1 Aug 2026 21:43:48 -0400 Subject: [PATCH 02/15] feat(terminal): give an attaching subscriber the screen as it stands A subscriber that arrives mid-stream cannot start from `render()`. Damage describes what changed since someone last looked, so a newcomer is handed whatever happens next -- on a quiet terminal, the cursor's line alone -- painted onto a blank screen. Upstream's `mark_fully_damaged` is private, so an embedder cannot ask for a full frame that way either. `snapshot()` copies the whole visible viewport instead, marks the frame full so a reused encoder's dedup hashes realign, and stamps the geometry it was captured under. The delicate part is what it does not do. Damage is a single shared cursor across every subscriber, so a snapshot that consumed it would steal the incumbent renderer's pending rows: the newcomer's full frame would look perfect while the established renderer silently froze. `capture_all` therefore never calls `damage()` or `reset_damage()`, and their absence is the mechanism. Proving that needed more care than the interleaving alone. Because `Term::damage()` marks the cursor line on every call, an incumbent owed only the row it sits on gets that row back even when its damage was stolen -- so the first version of the fixture passed against a deliberately naive implementation. The committed fixture rewrites row 0 and parks the cursor on row 3, putting the owed row where the cursor cannot return it for free; it fails when snapshot consumes damage, and again when snapshot copies damaged rows only or omits the full flag. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/damage.rs | 52 +++++ .../crates/buzz-terminal/src/shared.rs | 18 ++ .../crates/buzz-terminal/tests/snapshot.rs | 187 ++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs diff --git a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs index fa37af838..b25f6c76a 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs @@ -207,6 +207,58 @@ pub fn capture(terminal: &mut crate::Terminal) -> RawFrame { } } +/// Copy the **entire visible viewport**, leaving damage untouched. +/// +/// This exists for subscribers that arrive mid-stream: attach, reattach, and +/// the successor side of a resize. Damage only describes what changed since +/// the last capture, so a newcomer that starts from [`capture`] sees whatever +/// happened to change next -- often just the cursor's line -- painted onto a +/// blank screen. Upstream's `mark_fully_damaged` is private, so an embedder +/// cannot ask for a full frame that way. +/// +/// **It must not consume damage, and that is the load-bearing property.** The +/// incumbent subscriber's next [`capture`] has to still see its rows. If this +/// called `damage()`/`reset_damage()` it would steal them, and the incumbent +/// would freeze on stale content while a newcomer's full-frame test passed. +/// The absence of those two calls below is the mechanism; `snapshot_test.rs` +/// is the proof. +/// +/// **Runs under the lock; does no encoding.** Costs a full grid copy rather +/// than a damaged-rows copy, so it belongs on attach, not in the frame loop. +pub fn capture_all(terminal: &mut crate::Terminal) -> RawFrame { + let viewport = terminal.viewport(); + let term = terminal.term_mut(); + let columns = term.columns(); + let screen_lines = term.screen_lines(); + let cursor_point = term.grid().cursor.point; + let visible = term + .mode() + .contains(alacritty_terminal::term::TermMode::SHOW_CURSOR); + + let grid = term.grid(); + let mut rows = Vec::with_capacity(screen_lines); + for line in 0..screen_lines { + let row = &grid[Line(line as i32)]; + rows.push((line, row[..Column(columns)].to_vec())); + } + let cursor = CursorFrame { + line: cursor_point.line.0.max(0) as usize, + column: cursor_point.column.0, + visible, + }; + + // No `damage()` and no `reset_damage()`: see the note above. + RawFrame { + rows, + cursor, + // A snapshot *is* a repaint, and marking it full also resets the + // consumer's `Encoder` hashes, so its dedup state describes the grid it + // was actually given rather than a predecessor's. + full: true, + viewport, + } +} + /// Suppresses rows whose content did not actually change. #[derive(Default)] pub struct Encoder { diff --git a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs index 577908ee0..c76d094e9 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs @@ -168,6 +168,24 @@ impl SharedTerminal { encoder.encode(raw) } + /// Copy the whole viewport for a subscriber that arrived mid-stream. + /// Renderer plane. + /// + /// Attach, reattach, and the successor side of a resize all need the + /// screen as it stands, not the next thing to change on it. Crucially this + /// leaves damage alone, so taking a snapshot for a newcomer cannot steal + /// the incumbent renderer's pending rows -- see [`damage::capture_all`]. + /// + /// Costs a full grid copy under the lock, so call it on attach rather than + /// per frame. + pub fn snapshot(&self, encoder: &mut Encoder) -> Frame { + let raw = { + let mut term = self.acquire(&self.renderer); + damage::capture_all(&mut term) + }; + encoder.encode(raw) + } + /// Apply a coalesced resize. Renderer plane: this competes with the /// renderer for the same lock and can hold it for milliseconds. pub fn resize(&self, size: crate::Size) -> crate::Viewport { diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs new file mode 100644 index 000000000..003550904 --- /dev/null +++ b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs @@ -0,0 +1,187 @@ +//! The attach contract: what a subscriber that arrives mid-stream is given, +//! and what taking it must not cost the subscriber already there. +//! +//! `render()` reports damage -- what changed since someone last looked. That +//! is the right thing for a steady-state renderer and the wrong thing for a +//! newcomer, who needs the screen as it stands. `snapshot()` supplies that, +//! and the delicate part is that it must do so *without* consuming damage: +//! two subscribers share one terminal, and damage is a single shared cursor. + +use buzz_terminal::damage::Encoder; +use buzz_terminal::fences::Fences; +use buzz_terminal::{Action, SharedTerminal, Size, Terminal}; +use std::sync::mpsc::Receiver; + +/// The receiver is returned rather than dropped: dropping it disconnects the +/// channel and every subsequent listener send silently fails. +fn terminal(columns: usize, screen_lines: usize) -> (SharedTerminal, Receiver) { + let size = Size { + columns, + screen_lines, + scrollback: 100, + }; + let (term, actions) = Terminal::new(size, Fences::ALL); + (SharedTerminal::new(term), actions) +} + +/// Collect the non-blank text of a frame's rows, for comparing what a +/// subscriber can actually see. +fn visible_text(frame: &buzz_terminal::damage::Frame) -> Vec { + frame + .rows + .iter() + .map(|row| { + row.spans + .iter() + .map(|span| span.text.as_str()) + .collect::() + .trim_end() + .to_string() + }) + .filter(|line| !line.is_empty()) + .collect() +} + +/// The reason `snapshot` exists. A subscriber that attaches mid-stream and +/// starts from `render()` is handed only what changes next -- with a quiet +/// terminal that is the cursor's line alone, so the scrollback-visible screen +/// never arrives. +#[test] +fn a_late_render_shows_only_the_next_change_but_a_snapshot_shows_the_screen() { + let (shared, _actions) = terminal(20, 4); + shared.feed(b"first\r\nsecond\r\nthird"); + + // The incumbent consumes the damage from that output. + let mut incumbent = Encoder::new(); + let seen = visible_text(&shared.render(&mut incumbent)); + assert_eq!(seen, vec!["first", "second", "third"]); + + // A newcomer rendering now sees essentially nothing: damage is spent. + let mut latecomer = Encoder::new(); + let by_render = visible_text(&shared.render(&mut latecomer)); + assert!( + !by_render.contains(&"first".to_string()), + "a late render cannot show scrollback it never saw damaged, got {by_render:?}" + ); + + // The same newcomer snapshotting sees the whole viewport. + let mut attaching = Encoder::new(); + let by_snapshot = shared.snapshot(&mut attaching); + assert_eq!( + visible_text(&by_snapshot), + vec!["first", "second", "third"], + "a snapshot must carry the visible viewport" + ); + assert!(by_snapshot.full, "a snapshot is a repaint"); +} + +/// **The law: `snapshot()` must not consume damage.** +/// +/// Two subscribers share one terminal and damage is one shared cursor, so a +/// snapshot taken for an attaching subscriber must leave the incumbent's +/// pending rows intact. A naive implementation that calls `damage()` passes a +/// full-frame test while freezing every other subscriber -- the newcomer looks +/// perfect and the incumbent silently stops updating. +/// +/// The interleaving is the point: write, snapshot, *then* let the incumbent +/// render. But the interleaving alone is not enough to discriminate, and the +/// reason is this module's own rule 2 -- `Term::damage()` marks the cursor +/// line on every call. So an incumbent owed only the line it is sitting on +/// gets that line back even when its damage was stolen, and a naive snapshot +/// passes. +/// +/// The owed row therefore has to be somewhere the cursor is *not*. Here row 0 +/// is rewritten and the cursor is parked on row 3, so a theft leaves the +/// incumbent holding a blank cursor line and nothing else. +#[test] +fn a_snapshot_does_not_steal_the_incumbents_damage() { + let (shared, _actions) = terminal(20, 4); + + // An established renderer, caught up to a quiet terminal. The initial + // content is shorter than its replacement so the rewrite below covers it + // completely and no tail of it survives. + let mut incumbent = Encoder::new(); + shared.feed(b"old"); + let _ = shared.render(&mut incumbent); + + // Rewrite row 0, then park the cursor on row 3. The incumbent is now owed + // row 0, which is not the row the cursor will re-damage for free. + shared.feed(b"\x1b[1;1HAFTER\x1b[4;1H"); + + // A second subscriber attaches and snapshots first. + let mut attaching = Encoder::new(); + let attached = shared.snapshot(&mut attaching); + assert_eq!( + visible_text(&attached), + vec!["AFTER"], + "the newcomer sees the whole screen" + ); + + // The incumbent must still be delivered row 0. + let follow_up = shared.render(&mut incumbent); + assert!( + follow_up.rows.iter().any(|row| row.line == 0), + "snapshot consumed the incumbent's damage: row 0 was never delivered, \ + got rows {:?}", + follow_up.rows.iter().map(|r| r.line).collect::>() + ); + assert!( + visible_text(&follow_up).contains(&"AFTER".to_string()), + "the incumbent must still see the row written before the snapshot, got {:?}", + visible_text(&follow_up) + ); +} + +/// A snapshot stamps the geometry it was captured under and resets the +/// consumer's dedup state, so an encoder reused across a resize cannot carry +/// hashes describing rows of a different width. +#[test] +fn a_snapshot_realigns_a_reused_encoders_dedup_state() { + let (shared, _actions) = terminal(20, 4); + shared.feed(b"wide enough line"); + + let mut encoder = Encoder::new(); + let before = shared.snapshot(&mut encoder); + assert_eq!(before.viewport.columns, 20); + + let resized = shared.resize(Size { + columns: 10, + screen_lines: 4, + scrollback: 100, + }); + assert_eq!(resized.columns, 10); + + // Same encoder, new geometry: every row must be re-sent, not suppressed + // as unchanged against hashes taken at the old width. + let after = shared.snapshot(&mut encoder); + assert_eq!( + after.viewport.columns, 10, + "the capture-time grid is stamped" + ); + assert!(after.full, "a snapshot is a repaint"); + assert!( + !after.rows.is_empty(), + "stale hashes must not suppress rows after a resize" + ); +} + +/// Two consecutive snapshots with no output between them still both carry the +/// screen. A snapshot is not a one-shot: reattach may happen repeatedly, and +/// nothing about the first may disarm the second. +#[test] +fn snapshots_are_repeatable() { + let (shared, _actions) = terminal(20, 4); + shared.feed(b"persistent"); + + let mut first = Encoder::new(); + let mut second = Encoder::new(); + assert_eq!( + visible_text(&shared.snapshot(&mut first)), + vec!["persistent"] + ); + assert_eq!( + visible_text(&shared.snapshot(&mut second)), + vec!["persistent"], + "a second subscriber attaching later must see the same screen" + ); +} From e887bdb2c4384dfafa8035a9ea0294a30945b5d5 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sat, 1 Aug 2026 22:02:32 -0400 Subject: [PATCH 03/15] test(terminal): close the review gaps in the cluster and snapshot contracts Six review findings, all coverage on code that was already correct, plus one enforcement change. Clusters. The join guard has two halves and only one was exercised: every fixture built a plain cluster after a marked one, none the reverse. Drop `joinable` and a release build silently emits `Span { text: "xye\u{301}", cluster_count: 3 }` -- four chars counted as three, so the consumer misplaces everything after it. Also fixture the `u16::MAX` join refusal, which is live rather than defensive: `Size.columns` is an unclamped `usize` with no caller bounding it, so a 70000-column row reaches it and must split rather than wrap. Promote the encoder's `debug_assert` to `assert!`. This is a wire invariant, and a check that vanishes in release vanishes exactly where the corruption ships: with the defect present, the promoted build fails at the producer rather than leaving an undecodable span for the renderer to misdraw. Measured at +5.9us on a 65us encode for 80x24 -- 0.04% of a 60Hz frame. Snapshot. The four fixtures asserted the damage plane and nothing else, so a capture that dropped the screen's last row, lied about the cursor, or stamped a stale grid identity passed all of them. `visible_text` trims and filters empty lines, which is what let the missing row disappear -- a helper written for legibility concealing the defect it should have exposed. Assert the row set rather than the text, the cursor against a real position and DECTCEM, the whole `Viewport` rather than `columns` alone, and that a full-grid copy is billed to the renderer's meter and not the reader's. Findings from Sami's P2a and P2b reviews; enforcement call from Eva; encode cost measured by Sami. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/damage.rs | 9 +- .../crates/buzz-terminal/tests/clusters.rs | 78 ++++++++++++++ .../crates/buzz-terminal/tests/snapshot.rs | 100 ++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs index b25f6c76a..74d2503de 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/damage.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/damage.rs @@ -381,7 +381,14 @@ fn spans(cells: &[Cell]) -> Vec { } open = joinable; } - debug_assert!( + // Enforced in release, not just in debug. This is a *wire* invariant: a + // span that violates it is undecodable by the rule in [`Span`], and the + // consumer's failure is silent misplacement of every cluster after it. + // A `debug_assert` here would vanish in exactly the build where that + // corruption ships. The cost is one pass over text already in cache -- + // the same order as building the spans -- and it buys a loud, local + // failure instead of a renderer quietly drawing the wrong columns. + assert!( spans.iter().all(Span::counts_are_consistent), "cluster_count must be 1 or the span's char count" ); diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs index feb91f0c7..8e5310283 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs @@ -199,6 +199,84 @@ fn cluster_count_distinguishes_a_marked_cluster_from_a_plain_run() { assert!(marked.counts_are_consistent() && plain.counts_are_consistent()); } +/// The join guard has two halves: the previous cell must not have carried +/// marks (`open`), and the current cell must not carry them (`joinable`). +/// Every fixture above exercises only the first half -- a plain cluster +/// following a marked one. This one exercises the second: a *marked* cluster +/// arriving after a plain run, which is the only path on which the run in +/// progress is handed text holding more `char`s than the one cluster its +/// count is about to be incremented by. +/// +/// Sami found the hole. With `joinable` dropped from the guard, a release +/// build silently emits `Span { column: 0, text: "xyé", cluster_count: 3 }`: +/// four chars counted as three, so the consumer's rule splits per char and +/// places the combining mark on top of `z`. +#[test] +fn a_marked_cluster_after_a_plain_run_starts_its_own_span() { + let (spans, _actions) = render("xye\u{0301}z"); + assert_eq!( + placements(&spans), + vec![ + (0, "x".into()), + (1, "y".into()), + (2, "e\u{0301}".into()), + (3, "z".into()), + ], + "a marked cluster must not be absorbed into the run in front of it" + ); +} + +/// `cluster_count` is a `u16` and `Size.columns` is an unclamped `usize` +/// (`lib.rs:50`) that no production caller bounds yet, so a row of uniform +/// cells wider than `u16::MAX` reaches the join guard's overflow refusal. +/// The guard is live code, not paranoia, and this fixture is what says so. +/// +/// Refusing to join produces a shape the consumer already handles -- the run +/// ends and a new span starts at the next column -- whereas wrapping produces +/// an undecodable span, the same failure as the marked-after-plain case above. +#[test] +fn a_run_longer_than_u16_max_splits_rather_than_wrapping() { + let columns = 70_000; + let size = Size { + columns, + screen_lines: 1, + scrollback: 0, + }; + let (term, _actions) = Terminal::new(size, Fences::ALL); + let shared = SharedTerminal::new(term); + // One character is enough: the rest of the row is blank cells of the same + // style, so the whole row is a single candidate run. + shared.feed(b"a"); + let mut encoder = Encoder::new(); + let frame = shared.render(&mut encoder); + + let spans = &frame + .rows + .iter() + .find(|row| row.line == 0) + .expect("the fed row must be present") + .spans; + + assert!( + spans.iter().all(|span| span.counts_are_consistent()), + "an oversized run must not wrap its count: {spans:?}" + ); + let counts: Vec = spans.iter().map(|span| span.cluster_count).collect(); + let columns_at: Vec = spans.iter().map(|span| span.column).collect(); + assert_eq!( + counts, + vec![u16::MAX, (columns - u16::MAX as usize) as u16], + "the run must end at the last representable count" + ); + assert_eq!( + columns_at, + vec![0, u16::MAX as usize], + "the second span starts where the first left off" + ); + let chars: usize = spans.iter().map(|span| span.text.chars().count()).sum(); + assert_eq!(chars, columns, "no cell may be dropped by the split"); +} + /// Wrapping marks the last cell of the row with `WRAPLINE` (upstream /// `term/mod.rs:968`). That bit records where the text happened to wrap, not /// how the text looks, so it must not reach the style key: if it did, the last diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs index 003550904..5ad8a62c9 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs @@ -143,6 +143,7 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() { let mut encoder = Encoder::new(); let before = shared.snapshot(&mut encoder); assert_eq!(before.viewport.columns, 20); + let first_generation = before.viewport.generation; let resized = shared.resize(Size { columns: 10, @@ -150,6 +151,10 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() { scrollback: 100, }); assert_eq!(resized.columns, 10); + assert!( + resized.generation > first_generation, + "an applied resize advances the generation" + ); // Same encoder, new geometry: every row must be re-sent, not suppressed // as unchanged against hashes taken at the old width. @@ -158,6 +163,17 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() { after.viewport.columns, 10, "the capture-time grid is stamped" ); + // Columns alone does not identify a grid. `Viewport`'s own doc says the + // three fields travel together *because* a consumer comparing two of the + // three can be wrong -- and this fixture used to compare one. A resize + // that changed only `screen_lines`, or 20 -> 10 -> 20, leaves columns + // matching while the generation has moved. `resize.rs` asserts this on + // `render()` frames five times and never once on a snapshot, which is + // what Sami's T3 mutant walked through; Mari's reattach reads this stamp. + assert_eq!( + after.viewport, resized, + "a snapshot stamps the identity of the grid it actually captured" + ); assert!(after.full, "a snapshot is a repaint"); assert!( !after.rows.is_empty(), @@ -165,6 +181,90 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() { ); } +/// A snapshot carries *every* row of the viewport, including the last one, +/// and stamps the cursor plane truthfully. +/// +/// Both properties are asserted here rather than in the fixtures above +/// because of what those fixtures' helper hides: `visible_text` trims and +/// drops empty lines, so a capture that skipped the bottom row of the screen +/// reads identically to one that didn't whenever the content sits in the top +/// rows -- which it does in every other fixture in this file. Sami's T2 +/// mutant (`0..screen_lines - 1`) survived all four for exactly that reason. +/// So this fixture puts content on the last row and asserts the row *set*, +/// not the text. +/// +/// The cursor half is the same shape of gap: nothing checked that a snapshot's +/// cursor was the terminal's cursor rather than a plausible default. +#[test] +fn a_snapshot_carries_every_row_and_the_true_cursor() { + let (shared, _actions) = terminal(20, 4); + // Write the bottom row of the screen, then park the cursor at line 4, + // column 6 (1-based) -- row 3, column 5 to us. + shared.feed(b"\x1b[4;1Hbottom\x1b[4;6H"); + + let mut attaching = Encoder::new(); + let frame = shared.snapshot(&mut attaching); + + let lines: Vec = frame.rows.iter().map(|row| row.line).collect(); + assert_eq!( + lines, + vec![0, 1, 2, 3], + "a snapshot must carry the whole viewport, last row included" + ); + assert!( + visible_text(&frame).contains(&"bottom".to_string()), + "content on the last row must reach an attaching subscriber, got {:?}", + visible_text(&frame) + ); + + assert_eq!(frame.cursor.line, 3, "the snapshot's cursor line is real"); + assert_eq!( + frame.cursor.column, 5, + "the snapshot's cursor column is real" + ); + assert!(frame.cursor.visible, "the cursor is shown by default"); + + // ...and a hidden cursor is reported hidden, so `visible` tracks the mode + // rather than being a constant that happens to match the default. + shared.feed(b"\x1b[?25l"); + let mut second = Encoder::new(); + assert!( + !shared.snapshot(&mut second).cursor.visible, + "DECTCEM off must reach the attaching subscriber" + ); +} + +/// Taking a snapshot is billed to the renderer plane. +/// +/// The two planes are metered separately because pooling them lets the +/// reader's millions of fast acquires dilute the renderer's tail into a false +/// pass (`shared.rs` module docs). A full-grid copy is the single most +/// expensive thing that takes this lock, so misfiling it under the reader +/// would corrupt the very instrument the renderer's budget is judged by -- +/// and no fixture noticed until Sami's T4. +#[test] +fn a_snapshot_is_billed_to_the_renderer_plane() { + let (shared, _actions) = terminal(20, 4); + shared.feed(b"content"); + + shared.reader_acquire().reset(); + shared.renderer_acquire().reset(); + + let mut attaching = Encoder::new(); + let _ = shared.snapshot(&mut attaching); + + assert_eq!( + shared.renderer_acquire().snapshot().acquisitions, + 1, + "the snapshot's lock acquisition belongs to the renderer plane" + ); + assert_eq!( + shared.reader_acquire().snapshot().acquisitions, + 0, + "a full-grid copy must not be charged to the reader plane" + ); +} + /// Two consecutive snapshots with no output between them still both carry the /// screen. A snapshot is not a one-shot: reattach may happen repeatedly, and /// nothing about the first may disarm the second. From a957d25b02881bad31bb0d6ab6c9571545ba9bc8 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 00:46:05 -0400 Subject: [PATCH 04/15] feat(terminal): bound the lock hold by weighted work, not by bytes Both existing fences meter bytes. That is right for memory and wrong for time: `ESC[m` and `ESC#8` are four bytes each, and the second rewrites every cell. A DECALN flood therefore reaches p95 65535us against a 4000us budget with **zero** F1 aborts -- the fence never fires, because nothing is buffered. Unfenced, one acquisition was observed at 22.1s. So add a third quantity. `Counting` wraps the handler and charges each callback what it touches: an O(cells) callback costs `columns * lines`, an O(1) callback costs 1. `Feeder` accepts bytes into a pending tail and parses only what one work budget affords, leaving the rest for the next call. That bounds the hold. It does not bound the queue, so the tail has a cap and a loud breach counter, and the reader is told to stop reading via a depth query rather than a pause flag the fence owns -- a flag is a state a reader can fail to clear, which is how a paused reader strands a child mid-teardown. The weights are read from `alacritty_terminal-0.26.0/src/term/mod.rs` and then checked against measurement, never fitted. Where source and measurement disagree the source wins and the slack is recorded in the table. Three findings from that reading are worth naming: * `delete_chars`/`insert_blank` cost `columns` for *every* N and are worst at N=1 -- the swap loop runs `columns - end` times, so cost falls as N rises. A sweep that varies only N reports them parameter-dependent *decreasing*, and the honest reading of that invites charging by N, which is backwards. * `move_backward_tabs` (CBT) assigns `col` inside the `if self.tabs[i]` test, so with tabstops cleared the cursor never moves, its `col == 0` exit is unreachable, and all N iterations rescan the row: `ESC[3g ESC[65535Z` is eight bytes for 82ms at 1600 columns. Its twin `move_forward_tabs` assigns outside the test and is fine. The wrapper stops it at the first fixed point -- permanent, because the scan depends only on the cursor -- which makes it O(columns) and, verified exhaustively over every tabstop subset of a 12-column grid, lands on the same column as the naive loop. * `reset_state` (RIS) resets both grids and walks the primary's scrollback, so two bytes can be worth more than the whole budget. Priced on *configured* depth, not `history_size()`, which observes only the active grid: a filled primary followed by `ESC[?1049h` reads as empty while the work is still paid. Slice size is derived from the densest atom the grid admits rather than chosen, so the bound holds on the first byte of a cold feeder. An earlier version sized slices from observed density and was strictly worse where it matters -- a fresh feeder has seen nothing, so its first slice is wide. `Terminal::feed` now returns whether a tail remains and the caller pumps `drain` between lock acquisitions. `feed_fully` is the whole-buffer form, deliberately a separate name because it reinstates the unbounded hold. Every fixture asserts exact counts, never `> 0` and never `<=` alone. Both directions are load-bearing: with the CBT break deleted the callback loops 65535 times while charging `work == 1`, so an upper bound passes over it, and the mutant lands on the same cursor column as the fix, so a semantic assertion passes too. 20 mutants dead in debug and release. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/fences.rs | 141 ++++ .../src-tauri/crates/buzz-terminal/src/lib.rs | 58 +- .../crates/buzz-terminal/src/reader.rs | 173 +++- .../crates/buzz-terminal/src/shared.rs | 24 +- .../crates/buzz-terminal/src/units.rs | 377 +++++++++ .../crates/buzz-terminal/tests/clusters.rs | 8 +- .../crates/buzz-terminal/tests/fences.rs | 10 +- .../crates/buzz-terminal/tests/latency.rs | 2 +- .../crates/buzz-terminal/tests/resize.rs | 8 +- .../crates/buzz-terminal/tests/slicing.rs | 760 ++++++++++++++++++ .../crates/buzz-terminal/tests/snapshot.rs | 16 +- 11 files changed, 1544 insertions(+), 33 deletions(-) create mode 100644 desktop/src-tauri/crates/buzz-terminal/src/units.rs create mode 100644 desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index 92e651c28..ed0bc1a1d 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -21,6 +21,116 @@ pub const SYNC_CAP: usize = 64 << 10; /// Max parser-visible bytes chargeable before the parser is rebuilt. pub const OSC_BUDGET: usize = 256 << 10; +/// Max cost-weighted work one [`crate::reader::Feeder::drain`] may spend +/// before returning, in cell-equivalents. +/// +/// Derived, not chosen: measured worst-case density across the 2-D op sweep +/// is 16.9 ns/work (`erase_chars` at N=1, 80x24 -- the cheapest real callback, +/// where fixed dispatch cost dominates the single cell it touches), so a +/// 16.67 ms frame is ~988_000 work units. This is a quarter of that. The +/// remaining three quarters are headroom for lock acquisition, the counting +/// wrapper's own bookkeeping, and platforms slower than the one measured; +/// 16.9 ns/work is the max of a sample, not a proven ceiling, so it is not +/// spent to the last unit. +pub const WORK_BUDGET: u64 = 250_000; + +/// Bounds on how many bytes are handed to the parser at once. +/// +/// The budget is checked *between* slices, so the slice is what actually +/// bounds one lock hold, and a fixed byte count cannot do it: `ESC#8` is +/// three bytes and costs a full grid, so 256 bytes of it is 1.6 ms at 200x50 +/// and 14 ms at 1600x50. [`slice_bytes`] therefore derives the size from the +/// grid, and these two clamp it -- `MAX` at the throughput plateau (measured: +/// plain-char parsing saturates by 64 bytes and is flat to 64 KiB), `MIN` +/// where a slice stops being able to hold a whole escape sequence: 4 bytes +/// covers `ESC#8` and `ESC c` intact, so the floor never splits the densest +/// atoms across slices for no benefit. Measured throughput at the floor is +/// ~74% of the plateau on plain text and ~77% on SGR, which is the price of +/// bounding a grid whose worst atom exceeds the budget outright. +pub const MIN_SLICE: usize = 4; +pub const MAX_SLICE: usize = 256; + +/// Bytes to hand the parser at once on a `columns x lines` grid. +/// +/// Sized against the **densest atom the grid admits**, so the bound holds on +/// the first byte of a cold feeder for any payload: two bytes of `ESC c` buy +/// [`max_atom_work`], which is the most work per byte upstream offers, and +/// `budget / (atom / 2)` is the widest slice that cannot exceed one budget. +/// +/// Deliberately *derived rather than learned*. An earlier version sized +/// slices from the density of preceding slices, which is strictly worse where +/// it matters: a fresh feeder has observed nothing, so its first slice is +/// wide, and a first wide slice of RIS spends many budgets before anything +/// looks. A bound that has to be taught is not a bound on the lesson. +/// +/// [`MIN_SLICE`] floors it, and on any grid with real scrollback the floor is +/// what binds -- RIS at the default 10k depth is worth more than the entire +/// budget on its own, so no slice size can keep a drain inside the budget and +/// the floor stops the arithmetic from asking for fractions of a byte. That +/// residual is not hidden: it is exactly [`max_drain_work`], and it is the +/// honest cost of an indivisible callback that upstream can be asked to make +/// smaller only by not calling it. +pub fn slice_bytes(columns: usize, lines: usize, scrollback: usize) -> usize { + let densest = (max_atom_work(columns, lines, scrollback) / 2).max(1); + ((WORK_BUDGET / densest) as usize).clamp(MIN_SLICE, MAX_SLICE) +} + +/// Work the single worst uninterruptible callback can cost on this grid. +/// +/// This is the irreducible overrun past [`WORK_BUDGET`]: no scheduler outside +/// the parser can cut inside a callback, so a caller converting a work budget +/// into a time bound must add it. +/// +/// It is `columns` because [`crate::units::Counting`] terminates CBT at its +/// first fixed point. Upstream's own loop is `N x columns` -- 82 ms for eight +/// bytes at 1600 columns -- and clamping `N` to `columns` only brings that to +/// `columns^2`, which at 1600 is 2.56M work, **10x the whole budget**: the +/// atom, not the budget, would decide the bound. Stopping at the fixed point +/// makes it `columns`, and the budget goes back to being the thing that sets +/// the bound. Every other callback is priced at or below `cells`, which is +/// larger, so this term never dominates. +pub fn max_atom_work(columns: usize, lines: usize, scrollback: usize) -> u64 { + // RIS: both grids plus the primary's configured scrollback. This is the + // largest single callback by a wide margin -- 16x the budget at the + // default 10k depth -- and it is genuinely indivisible, so it is stated + // rather than smoothed. CBT, once terminated at its fixed point, is + // `columns` and never competes. + let ris = 2 * (columns * lines) as u64 + (scrollback * columns) as u64; + ris.max(columns as u64) +} + +/// Upper bound on the work a single [`crate::reader::Feeder::drain`] can do. +/// +/// Two irreducible terms on top of [`WORK_BUDGET`], and it is worth being +/// exact about which is which, because I got this wrong first and the +/// fixtures caught it: +/// +/// * The budget is checked *between* slices, so a drain overshoots by up to +/// one whole slice -- not one atom. [`slice_bytes`] keeps that under one +/// budget wherever its derivation is unclamped. +/// * A callback already running cannot be preempted. RIS at the default 10k +/// scrollback is worth 16x the whole budget on its own, so on such a grid +/// the floor binds and the overshoot is a few of those atoms. No scheduler +/// outside the parser can fix that -- what it can do is *report* it, which +/// is why this is a function callers can read rather than an assumption +/// they inherit. +pub fn max_drain_work(columns: usize, lines: usize, scrollback: usize) -> u64 { + let atom = max_atom_work(columns, lines, scrollback); + // A slice of N bytes holds at most N/2 of the densest atoms, and the + // budget is only consulted between slices, so a whole slice is the + // overshoot. Where the slice derivation is unclamped this is one budget; + // where MIN_SLICE binds, it is this. + WORK_BUDGET + (slice_bytes(columns, lines, scrollback) as u64 / 2).max(1) * atom +} + +/// Max bytes that may sit unparsed before the reader must stop reading the +/// PTY. Bounds the *queue*; [`WORK_BUDGET`] bounds only the lock hold. +pub const TAIL_CAP: usize = 4 << 20; + +/// Depth at which a paused reader may resume. Strictly below [`TAIL_CAP`] so +/// the reader does not flap between full and one-byte-below-full. +pub const TAIL_RESUME: usize = 1 << 20; + /// Which fences are active. Both on in production. /// /// The mutation law requires exercising each fence with the other **disabled**, @@ -87,6 +197,37 @@ pub struct FenceStats { /// Parser-visible bytes charged against the F2 budget, cumulative across /// resets. Includes every flush route, not just directly-advanced input. pub charged_bytes: u64, + /// Parser units completed: one per `Handler` callback dispatched, which is + /// one per fully-parsed escape sequence or printed character. + /// + /// Separate from `charged_bytes` because they answer different questions + /// and can disagree by orders of magnitude: four bytes of `ESC#8` rewrite + /// the whole grid, four bytes of `ESC[m` set a flag. Bytes bound memory; + /// units are the proxy for time. See [`crate::units`]. + pub completed_units: u64, + /// Cost-weighted work completed, in cell-equivalents: an O(cells) callback + /// charges `columns * lines`, an O(1) callback charges 1. + /// + /// Deliberately a second number rather than a replacement for + /// `completed_units`. They answer different questions -- "how many things + /// happened" versus "how much did they cost" -- and a stream of `ESC#8` + /// makes them disagree by four orders of magnitude, which is the entire + /// reason this fence exists. + pub completed_work: u64, + /// Deepest the unparsed tail has been, in bytes. The high-water mark + /// rather than the current depth, because the current depth is zero again + /// by the time a test looks at it. + pub max_pending: usize, + /// Times the tail was at or over [`TAIL_CAP`] at the end of a drain. + /// + /// Loud on purpose. Reaching the cap means the reader kept reading past + /// the point it was told to stop, so the queue bound is being held by + /// nothing; a silent cap would make that indistinguishable from a reader + /// that is obeying. + pub tail_breaches: u64, + /// Bytes discarded unparsed by [`crate::reader::Feeder::abandon_tail`]. + /// Non-zero anywhere but session close is a bug that ate output. + pub abandoned_bytes: u64, } impl FenceStats { diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs index b45324342..9a26816e2 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs @@ -14,6 +14,7 @@ pub mod path; pub mod reader; pub mod shared; pub mod shell; +pub mod units; #[cfg(test)] mod context_tests; @@ -109,7 +110,12 @@ impl Terminal { ( Self { term, - feeder: reader::Feeder::new(fences), + feeder: reader::Feeder::new( + fences, + size.columns, + size.screen_lines, + size.scrollback, + ), size, generation: 0, }, @@ -118,8 +124,53 @@ impl Terminal { } /// Feed PTY output through the fences into the emulator. - pub fn feed(&mut self, bytes: &[u8]) { - self.feeder.feed(&mut self.term, bytes); + /// + /// Parses what one work budget affords and returns with the rest held as + /// a pending tail, so one call cannot hold the terminal for an unbounded + /// time. **The caller must pump [`Terminal::drain`] until it returns + /// false**, releasing the lock between calls; that is the whole point -- + /// the tail exists to give the renderer a chance at the lock, not to defer + /// work indefinitely. [`Terminal::pending_bytes`] and + /// [`Terminal::tail_full`] tell the reader when to stop reading the PTY. + pub fn feed(&mut self, bytes: &[u8]) -> bool { + self.feeder.feed(&mut self.term, bytes) + } + + /// Parse more of the pending tail. Returns whether any remains. + pub fn drain(&mut self) -> bool { + self.feeder.drain(&mut self.term); + self.feeder.pending_bytes() > 0 + } + + /// Feed and parse to completion, without the intervening lock releases. + /// + /// For tests and for callers with no renderer contending -- it reinstates + /// exactly the unbounded hold [`Terminal::feed`] exists to prevent, so it + /// is deliberately a separate name rather than a flag on `feed`. + pub fn feed_fully(&mut self, bytes: &[u8]) { + self.feed(bytes); + while self.drain() {} + } + + /// Bytes accepted but not yet parsed. + pub fn pending_bytes(&self) -> usize { + self.feeder.pending_bytes() + } + + /// Whether the tail is at its cap and the reader must stop reading. + pub fn tail_full(&self) -> bool { + self.feeder.tail_full() + } + + /// Whether a paused reader may resume. + pub fn tail_drained(&self) -> bool { + self.feeder.tail_drained() + } + + /// Discard the unparsed tail. Session close only -- see + /// [`reader::Feeder::abandon_tail`]. + pub fn abandon_tail(&mut self) -> usize { + self.feeder.abandon_tail() } pub fn stats(&self) -> FenceStats { @@ -170,6 +221,7 @@ impl Terminal { return self.viewport(); } self.term.resize(size); + self.feeder.resize(size.columns, size.screen_lines); self.size = size; self.generation += 1; self.viewport() diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index a55f4d1ed..dd4cf4e2d 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -6,7 +6,11 @@ use alacritty_terminal::vte::ansi::{Handler, Processor, StdSyncHandler}; -use crate::fences::{FenceStats, Fences, OSC_BUDGET, SYNC_CAP}; +use crate::fences::{ + slice_bytes, FenceStats, Fences, MAX_SLICE, OSC_BUDGET, SYNC_CAP, TAIL_CAP, TAIL_RESUME, + WORK_BUDGET, +}; +use crate::units::{Counting, CursorColumn}; /// Owns the parser and enforces F1/F2 on every byte fed to it. pub struct Feeder { @@ -15,18 +19,46 @@ pub struct Feeder { stats: FenceStats, /// Bytes charged since the last F2 reset. since_reset: usize, + /// Bytes accepted but not yet parsed. Grows when arrival outruns + /// retirement; drained by every [`Feeder::feed`] and [`Feeder::drain`]. + pending: Vec, + /// How much of `pending` has already been parsed. Kept as an index rather + /// than draining the front on every slice, so a large tail is not + /// re-shuffled once per slice; the prefix is dropped in one go on the next + /// enqueue. + pending_at: usize, + /// The grid the weights are computed against. Tracked here rather than + /// read from the `Term` because `feed` only has the handler, and kept in + /// sync by [`Feeder::resize`]: a stale grid misprices every O(cells) + /// callback for as long as it is wrong. + columns: usize, + lines: usize, + /// Configured scrollback depth. Fixed for the life of the feeder: it is a + /// config value, not grid state, and `resize` does not change it. + scrollback: usize, } impl Feeder { - pub fn new(fences: Fences) -> Self { + pub fn new(fences: Fences, columns: usize, lines: usize, scrollback: usize) -> Self { Self { parser: Processor::new(), fences, stats: FenceStats::default(), since_reset: 0, + pending: Vec::new(), + pending_at: 0, + columns, + lines, + scrollback, } } + /// Track a viewport change, so the cost weights describe the current grid. + pub fn resize(&mut self, columns: usize, lines: usize) { + self.columns = columns; + self.lines = lines; + } + pub fn stats(&self) -> FenceStats { self.stats } @@ -40,10 +72,127 @@ impl Feeder { self.parser.sync_bytes_count() } - /// Feed one chunk of PTY output to the parser, applying both fences. - pub fn feed(&mut self, handler: &mut H, bytes: &[u8]) { + /// Bytes accepted but not yet parsed, because a previous [`Feeder::feed`] + /// spent its work budget before reaching them. + pub fn pending_bytes(&self) -> usize { + self.pending.len() - self.pending_at + } + + /// Whether the pending tail has reached [`TAIL_CAP`]. + /// + /// Deliberately derived from the current depth rather than latched. A + /// latch is a state the fence owns and could fail to clear, which is + /// exactly how a paused reader strands a child mid-teardown; a reader that + /// simply stops asking resumes by default. + pub fn tail_full(&self) -> bool { + self.pending_bytes() >= TAIL_CAP + } + + /// Whether a paused reader may resume: the tail has drained to the low + /// water mark. Separate from `!tail_full()` so the reader does not flap + /// between full and one-byte-below-full. + pub fn tail_drained(&self) -> bool { + self.pending_bytes() <= TAIL_RESUME + } + + /// Discard the unparsed tail. + /// + /// For session close only, and lossless where it is used: publication is + /// detached before shutdown drains, so this tail is bytes no renderer can + /// consume. Draining the *PTY* remains lifecycle-critical -- this exists so + /// parser work cannot hold teardown behind it. + pub fn abandon_tail(&mut self) -> usize { + let abandoned = self.pending_bytes(); + self.pending.clear(); + self.pending_at = 0; + self.stats.abandoned_bytes += abandoned as u64; + abandoned + } + + /// Accept PTY output and parse what fits in one work budget. + /// + /// Returns whether bytes remain unparsed. Bytes beyond the budget are + /// retained and parsed by [`Feeder::drain`], so this bounds the *lock + /// hold*; it does not bound the queue. When arrival outruns retirement + /// the tail grows to [`TAIL_CAP`] and [`Feeder::tail_full`] goes true, + /// which is the reader's cue to stop reading the PTY and let the child + /// block. No policy is applied here: a fence that dropped input to protect + /// itself would corrupt the screen to avoid being slow. + pub fn feed(&mut self, handler: &mut H, bytes: &[u8]) -> bool { + self.enqueue(bytes); + self.drain(handler); + self.pending_bytes() > 0 + } + + /// Append to the pending tail, compacting the already-parsed prefix first. + fn enqueue(&mut self, bytes: &[u8]) { + if self.pending_at > 0 { + self.pending.drain(..self.pending_at); + self.pending_at = 0; + } + self.pending.extend_from_slice(bytes); + } + + /// Parse from the pending tail until the work budget is spent. + /// + /// The budget is checked between parser slices, never inside a callback: + /// vte's own mid-buffer stop is driven by `Perform::terminated()`, whose + /// implementor in the ansi layer is private, so the cut has to be made + /// from outside, and a callback already running cannot be preempted at + /// all. One atom is therefore the irreducible overrun -- and it is not + /// small: `ESC[65535Z` with tabstops cleared is 8 bytes and 82 ms at 1600 + /// columns, because upstream's `move_backward_tabs` rescans the row once + /// per count when it finds no stop. + /// + /// What *is* bounded is the number of atoms per slice, and that bound + /// holds from the first byte of a cold feeder: [`slice_bytes`] is derived + /// from the densest work-per-byte upstream can produce on this grid, so + /// no slice can contain more than one budget's worth of callbacks no + /// matter what the payload is or what the feeder has seen before. + /// + /// Returns the work spent, which is at least the budget whenever the tail + /// is still non-empty on return. + pub fn drain(&mut self, handler: &mut H) -> u64 { + // Slices are copied out of the tail rather than borrowed from it, + // because `advance_slice` needs `&mut self` and the tail is part of + // self. A stack buffer keeps that from allocating; the copy is a + // memcpy against a parse two orders of magnitude more expensive. + let mut buf = [0u8; MAX_SLICE]; + let width = slice_bytes(self.columns, self.lines, self.scrollback); + let mut spent = 0; + while self.pending_at < self.pending.len() { + let end = (self.pending_at + width).min(self.pending.len()); + let len = end - self.pending_at; + buf[..len].copy_from_slice(&self.pending[self.pending_at..end]); + self.pending_at = end; + spent += self.advance_slice(handler, &buf[..len]); + if spent >= WORK_BUDGET { + break; + } + } + if self.pending_at == self.pending.len() { + self.pending.clear(); + self.pending_at = 0; + } + let depth = self.pending_bytes(); + self.stats.max_pending = self.stats.max_pending.max(depth); + if depth >= TAIL_CAP { + self.stats.tail_breaches += 1; + } + spent + } + + /// Parse one slice, applying both fences to it. Returns the work it cost. + fn advance_slice(&mut self, handler: &mut H, bytes: &[u8]) -> u64 { + let mut spent = 0; let sync_before = self.parser.sync_bytes_count(); - self.parser.advance(handler, bytes); + { + let mut counting = Counting::new(handler, self.columns, self.lines, self.scrollback); + self.parser.advance(&mut counting, bytes); + self.stats.completed_units += counting.units(); + self.stats.completed_work += counting.work(); + spent += counting.work(); + } let sync_after = self.parser.sync_bytes_count(); // Charge exactly the bytes the parser could see, by route: @@ -73,7 +222,17 @@ impl Feeder { // per breach; the released bytes are parser-visible and are charged. if self.fences.sync_abort && self.parser.sync_bytes_count() >= SYNC_CAP { let released = self.parser.sync_bytes_count(); - self.parser.stop_sync(handler); + // Counted too: aborting flushes the buffered frame through the + // handler, so these are units the lock hold paid for. Leaving them + // out would undercount exactly on the fenced path. + { + let mut counting = + Counting::new(handler, self.columns, self.lines, self.scrollback); + self.parser.stop_sync(&mut counting); + self.stats.completed_units += counting.units(); + self.stats.completed_work += counting.work(); + spent += counting.work(); + } self.stats.sync_aborts += 1; self.note_release(released); self.charge(released); @@ -88,6 +247,8 @@ impl Feeder { self.stats.osc_resets += 1; self.since_reset = 0; } + + spent } fn charge(&mut self, bytes: usize) { diff --git a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs index c76d094e9..545d22bd8 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs @@ -149,8 +149,28 @@ impl SharedTerminal { } /// Feed PTY output into the emulator. Reader plane. - pub fn feed(&self, bytes: &[u8]) { - self.acquire(&self.reader).feed(bytes); + /// + /// Returns whether a tail remains: one acquisition parses one work + /// budget, then **drops the lock** so the renderer can have it. The + /// caller pumps [`SharedTerminal::drain`] until it returns false. Doing + /// the whole buffer under one acquisition is what an unbounded hold *is*, + /// so it is not offered here. + pub fn feed(&self, bytes: &[u8]) -> bool { + self.acquire(&self.reader).feed(bytes) + } + + /// Parse more of the pending tail under a fresh acquisition. Reader + /// plane. Returns whether any remains. + pub fn drain(&self) -> bool { + self.acquire(&self.reader).drain() + } + + /// Feed and pump to completion, re-acquiring between slices. + pub fn feed_fully(&self, bytes: &[u8]) { + let mut more = self.feed(bytes); + while more { + more = self.drain(); + } } /// Sample damage and encode a frame. Renderer plane. diff --git a/desktop/src-tauri/crates/buzz-terminal/src/units.rs b/desktop/src-tauri/crates/buzz-terminal/src/units.rs new file mode 100644 index 000000000..b30533a65 --- /dev/null +++ b/desktop/src-tauri/crates/buzz-terminal/src/units.rs @@ -0,0 +1,377 @@ +//! Counting what the parser *does*, not how many bytes it read. +//! +//! Both fences in [`crate::fences`] meter bytes. That is the right denominator +//! for memory -- a buffer's size is bytes -- and the wrong one for time. `ESC[m` +//! and `ESC#8` are four bytes each; the first sets an attribute and the second +//! rewrites every cell of the grid. Metering the reader's lock hold in bytes +//! therefore prices those identically, and a stream of the second one holds the +//! lock for as long as it likes without ever tripping a byte budget. +//! +//! Measured: a DECALN flood at 200x50 reaches p95 65535us against a 4000us +//! budget with **zero** F1 aborts -- the fence never fires, because nothing is +//! buffered. Unfenced, one acquisition was observed at 22.1s, about 1300 +//! dropped frames in a single lock hold. +//! +//! So this module adds a third quantity: the number of *completed parser +//! units* -- one per `Handler` callback the parser dispatches, which is one +//! per fully-parsed escape sequence or printed character. It is a proxy for +//! work rather than a measure of it, but it has the property the byte count +//! lacks: it advances once per thing the emulator actually did. +//! +//! ## Why a wrapper, and not vte's own stopping point +//! +//! `Parser::advance_until_terminated` already supports stopping mid-buffer, +//! but termination is driven by `Perform::terminated()`, and in the ansi layer +//! the implementor is `Performer`, which is private (`vte-0.15.0/src/ansi.rs`: +//! `struct Performer` at 425, `terminated` at 1825, set only for BSU handling). +//! An embedder cannot reach it, so the stopping point has to be built outside +//! the parser rather than inside it. +//! +//! ## What this deliberately does not do +//! +//! It does not skip or veto expensive callbacks once a budget is spent. That +//! would bound the lock hold perfectly and silently corrupt the screen, which +//! is a worse failure than the one being fixed: a slow terminal recovers, a +//! wrong one does not. Every unit is delegated; the count only decides where +//! the *caller* may cut the input. + +use alacritty_terminal::event::EventListener; +use alacritty_terminal::term::Term; +use alacritty_terminal::vte::ansi::cursor_icon::CursorIcon; +use alacritty_terminal::vte::ansi::{ + Attr, CharsetIndex, ClearMode, CursorShape, CursorStyle, Handler, Hyperlink, KeyboardModes, + KeyboardModesApplyBehavior, LineClearMode, Mode, ModifyOtherKeys, PrivateMode, Rgb, + ScpCharPath, ScpUpdateMode, StandardCharset, TabulationClearMode, +}; + +/// Read access to the cursor column of whatever the wrapper is driving. +/// +/// Exists for exactly one callback. CBT's cost is bounded by *cursor +/// movement*, and the only way to charge it honestly -- or to stop it early +/// -- is to watch the cursor between steps. Everything else in this module is +/// priced from the grid alone, which is why this is a separate trait and a +/// separate bound rather than a field on [`Counting`]. +/// +/// Implemented over the public path in `alacritty_terminal-0.26.0`: +/// `Term::grid` (term/mod.rs:645) -> `Grid::cursor` (grid/mod.rs:113) -> +/// `Cursor::point` (grid/mod.rs:36). No private field, no fork. +pub trait CursorColumn { + fn cursor_column(&self) -> usize; +} + +impl CursorColumn for Term { + #[inline] + fn cursor_column(&self) -> usize { + self.grid().cursor.point.column.0 + } +} + +/// Wraps a [`Handler`], forwarding every callback and counting them. +/// +/// Every one of the trait's 71 methods has an empty default body upstream, so +/// a method left undelegated here would compile cleanly and silently discard +/// that escape sequence. The delegations are therefore generated by a macro +/// over the full method list rather than written out: the failure mode of +/// hand-copying is invisible. +pub struct Counting<'a, H: Handler + CursorColumn> { + inner: &'a mut H, + /// Callbacks dispatched, one per unit regardless of cost. This is the + /// fixture-facing number: it says what the parser *did*, and it is kept + /// separate from `work` because collapsing them is precisely the mistake + /// that made the first version of this seam useless. + units: u64, + /// Cost-weighted work, in cell-equivalents. This is the scheduling number. + work: u64, + columns: u64, + lines: u64, + /// Configured scrollback depth, not current fill. See `reset_state`. + scrollback: u64, +} + +impl<'a, H: Handler + CursorColumn> Counting<'a, H> { + /// `columns` and `lines` are the grid the handler is about to act on, and + /// they are the weights' only input: an O(cells) callback is charged + /// `columns * lines` because that is what it touches. + pub fn new(inner: &'a mut H, columns: usize, lines: usize, scrollback: usize) -> Self { + Self { + inner, + units: 0, + work: 0, + columns: columns as u64, + lines: lines as u64, + scrollback: scrollback as u64, + } + } + + /// Callbacks dispatched since this wrapper was created. + pub fn units(&self) -> u64 { + self.units + } + + /// Cost-weighted work dispatched, in cell-equivalents. + pub fn work(&self) -> u64 { + self.work + } + + #[inline] + fn cells(&self) -> u64 { + self.columns * self.lines + } + + /// A parameter charged at its clamped value. + /// + /// Upstream clamps most counts to the grid before acting on them, so the + /// bound is the clamp, not the parameter: `ESC[65535X` on an 80-column + /// grid touches 80 cells. Charging the raw parameter would let a + /// four-byte escape spend the whole slice budget without doing the work, + /// which stalls the parser as surely as under-charging lets it run away. + #[inline] + fn clamp(&self, n: usize, bound: u64) -> u64 { + (n as u64).min(bound).max(1) + } + + #[inline] + fn charge(&mut self, weight: u64) { + self.units += 1; + self.work += weight; + } +} + +/// Generate a delegating, counting implementation for every `Handler` method. +/// +/// Two groups, because the methods differ in *cost*, not in kind. `plain` +/// methods are charged one unit. `weighted` methods are charged what they +/// touch, using the expressions in the table below -- these are the ones a +/// hostile stream can use to buy grid-sized work with a four-byte escape. +/// +/// The count is incremented *before* delegating, so a callback that panics +/// still leaves evidence it was attempted. +macro_rules! counting_handler { + ( + plain { $($pname:ident($($parg:ident: $pty:ty),* $(,)?);)* } + weighted { $($wname:ident($($warg:ident: $wty:ty),* $(,)?) => |$this:ident| $weight:expr;)* } + ) => { + impl Handler for Counting<'_, H> { + $( + #[inline] + fn $pname(&mut self $(, $parg: $pty)*) { + self.charge(1); + self.inner.$pname($($parg),*); + } + )* + $( + #[inline] + fn $wname(&mut self $(, $warg: $wty)*) { + let weight = { let $this = &*self; $weight }; + self.charge(weight); + self.inner.$wname($($warg),*); + } + )* + + /// The one callback this wrapper does not delegate verbatim. + /// + /// CBT (`ESC[NZ`) is upstream's only unbounded atom. With no + /// tabstop below the cursor, `move_backward_tabs` + /// (`term/mod.rs:1580`) assigns `col` *inside* the `if + /// self.tabs[i]` test, so the cursor never moves, the `col == 0` + /// break is unreachable, and all N iterations rescan the row. + /// `ESC[3g ESC[65535Z` is eight bytes and 82 ms at 1600 columns. + /// Its twin `move_forward_tabs` (1605) assigns *outside* the + /// test, always advances, and is fine: same file, same loop + /// skeleton, and the entire difference is one assignment's + /// placement relative to one branch. + /// + /// The fix is a termination condition, not a smaller number. + /// Each step either moves the cursor strictly left or is a fixed + /// point, and **a fixed point is permanent** -- the scan depends + /// only on the cursor, which did not move. So the loop can stop + /// at the first one. The leftward distances telescope to at most + /// the starting column, plus one final failed scan, so the whole + /// callback is O(columns) and the delegated-call count is at most + /// `columns - 1`. + /// + /// Equivalence is not argued, it is checked: every tabstop subset + /// of a 12-column grid x 4 start columns x 7 counts (114_688 + /// cases) lands on the same column as the naive loop, on a real + /// `Term`. See `examples/probe_cbt_equiv.rs`. Deleting the + /// fixed-point break leaves the *landing column correct* and only + /// the cost wrong, so the fixture that guards this must assert + /// units, never the cursor. + #[inline] + fn move_backward_tabs(&mut self, count: u16) { + // One unit for the escape, as every other callback gets. + self.charge(1); + for _ in 0..count { + let before = self.inner.cursor_column(); + // No `before == 0` guard: column 0 is already a fixed + // point (upstream's own `col == 0` break leaves the + // cursor alone), so the check below covers it and a + // second one would be unreachable-by-construction code + // that no test could distinguish. + self.inner.move_backward_tabs(1); + let after = self.inner.cursor_column(); + // Charge the cells this step scanned. A step that finds a + // stop scans the distance it moved; a step that finds + // none scans the whole prefix and moves nothing -- + // charging that one zero would leave a loop that spins + // without ever paying, which is precisely the mutant this + // pricing has to make visible. + self.work += if after == before { before } else { before - after } as u64; + if after == before { + // A fixed point is permanent: the scan depends only + // on the cursor, and the cursor did not move. + break; + } + } + } + } + }; +} + +counting_handler! { + plain { + set_title(a0: Option); + set_cursor_style(a0: Option); + set_cursor_shape(shape: CursorShape); + input(c: char); + goto(line: i32, col: usize); + goto_line(line: i32); + goto_col(col: usize); + move_up(a0: usize); + move_down(a0: usize); + identify_terminal(intermediate: Option); + device_status(a0: usize); + move_forward(col: usize); + move_backward(col: usize); + move_down_and_cr(row: usize); + move_up_and_cr(row: usize); + backspace(); + carriage_return(); + linefeed(); + bell(); + substitute(); + newline(); + set_horizontal_tabstop(); + save_cursor_position(); + restore_cursor_position(); + clear_tabs(mode: TabulationClearMode); + set_tabs(interval: u16); + reverse_index(); + terminal_attribute(attr: Attr); + set_mode(mode: Mode); + unset_mode(mode: Mode); + report_mode(mode: Mode); + set_private_mode(mode: PrivateMode); + unset_private_mode(mode: PrivateMode); + report_private_mode(mode: PrivateMode); + set_scrolling_region(top: usize, bottom: Option); + set_keypad_application_mode(); + unset_keypad_application_mode(); + set_active_charset(a0: CharsetIndex); + configure_charset(a0: CharsetIndex, a1: StandardCharset); + set_color(a0: usize, a1: Rgb); + dynamic_color_sequence(a0: String, a1: usize, a2: &str); + reset_color(a0: usize); + clipboard_store(a0: u8, a1: &[u8]); + clipboard_load(a0: u8, a1: &str); + push_title(); + pop_title(); + text_area_size_pixels(); + text_area_size_chars(); + set_hyperlink(a0: Option); + set_mouse_cursor_icon(a0: CursorIcon); + report_keyboard_mode(); + push_keyboard_mode(mode: KeyboardModes); + pop_keyboard_modes(to_pop: u16); + set_keyboard_mode(mode: KeyboardModes, behavior: KeyboardModesApplyBehavior); + set_modify_other_keys(mode: ModifyOtherKeys); + report_modify_other_keys(); + set_scp(char_path: ScpCharPath, update_mode: ScpUpdateMode); + } + weighted { + // Every weight below is an upper bound on the cells the callback can + // touch, **read from `alacritty_terminal-0.26.0/src/term/mod.rs`** and + // then checked against measurement -- never fitted to a curve. The + // direction of the error is the whole point: an over-charge slices + // early and costs throughput, an under-charge is an attack surface, so + // where source and measurement disagree the source bound wins and the + // slack is recorded here rather than tuned away. + // + // `min(N, ...)` appears wherever upstream clamps the parameter; a raw + // `N` would let `ESC[65535X` charge 65535 on an 80-column grid and + // stall the parser on a cheap escape. + + // O(min(N, columns)): `end = min(start + count, columns)`, loop + // `row[start..end]` (1519). Knee measured exactly at N == columns. + erase_chars(count: usize) => |this| this.clamp(count, this.columns); + // O(columns) for *every* N, worst at N=1: the swap loop runs + // `columns - end` times where `end = min(start + N, columns - 1)` + // (1538), so cost *falls* as N rises. Charging by N would be backwards + // and would under-charge the worst case by the full terminal width -- + // measured 3422ns at N=1/1600 columns against 863ns at N=65535. + delete_chars(a0: usize) => |this| this.columns; + // O(columns) for every N, worst at N=1. Same shape as `delete_chars`: + // `num_cells = columns - (column + count)` (1187). + insert_blank(a0: usize) => |this| this.columns; + // O(columns): scans to the next tabstop per count, and always advances + // (`col` is assigned unconditionally at 1592), so the whole loop is + // bounded by one traversal of the row. This is the sibling that CBT + // should have been, one asymmetric line apart in the same file. + put_tab(count: u16) => |this| this.columns; + move_forward_tabs(count: u16) => |this| this.columns; + // NOTE: `move_backward_tabs` is NOT in this table. It is the one + // callback whose argument is rewritten, so it is written out by hand + // below the macro's generated methods -- a weight can price an atom but + // cannot shrink one. + // O(min(N, lines) x columns) in steady state: the row rotation is O(1) + // on the ring buffer, but `positions` rows are `reset()`, and a row + // reset is O(columns). + // + // **Known overshoot, measured and frequency-bounded.** While scrollback + // is still growing, `Grid::increase_scroll_limit` -> `Storage::initialize` + // reallocates in blocks of `MAX_CACHE_SIZE` = 1000 rows and `rezero`s + // the ring (`grid/storage.rs`). That is not chargeable from here -- the + // weight function cannot see history depth -- and it is real: at 1600 + // columns the spikes land at call 0, 1000, 2000, 3000 of a 4000-call + // scroll, ~4 ms each, against a 125 ns median. It is bounded in + // frequency (once per 1000 new history rows, and never once history + // saturates: with scrollback=100 only call 0 spikes) and it is upstream + // allocation rather than anything a stream can amplify, so it is + // recorded here instead of being priced into every scroll -- charging + // 1000x on 999 calls out of 1000 to cover the thousandth would make + // ordinary scrolling the slow path. + scroll_up(n: usize) => |this| this.clamp(n, this.lines) * this.columns; + delete_lines(n: usize) => |this| this.clamp(n, this.lines) * this.columns; + scroll_down(n: usize) => |this| this.clamp(n, this.lines) * this.columns; + insert_blank_lines(n: usize) => |this| this.clamp(n, this.lines) * this.columns; + // O(columns): one row, `damage_line(line, 0, columns - 1)`. + clear_line(mode: LineClearMode) => |this| this.columns; + // O(cells): 18.7us at 200x50, doubling on both axes. + clear_screen(mode: ClearMode) => |this| this.cells(); + // O(cells): rewrites every cell. + decaln() => |this| this.cells(); + // Both grids, plus the scrollback the primary owns. + // + // `reset_state` (1835) resets the primary *and* the alternate, and each + // `Grid::reset` runs `clear_history` -> `shrink_lines` -> `truncate` + + // `rezero`, which walks the raw buffer. So the cost carries a history + // axis that `cells` alone cannot see: measured 0.5 us empty against + // 1.68 ms with 10k rows filled at 400x100, a 42x per-cell miss, with + // the knee exactly at `screen_lines + MAX_CACHE_SIZE` where + // `shrink_lines` starts calling `truncate`. + // + // Priced on **configured** depth rather than current fill, which is the + // conservative choice and the only correct one: `history_size()` reads + // the *active* grid, so a filled primary followed by `ESC[?1049h` + // reports an empty history while RIS still pays for the inactive + // primary's rows -- underpriced 41x on exactly the arm an attacker + // would pick. The inactive grid is private, so there is no stateless + // way to observe the real fill; the configured depth bounds both. + // + // This is the one weight that can exceed [`crate::fences::WORK_BUDGET`] + // on its own -- 16x at the default 10k scrollback -- which is correct: + // it is a genuinely oversized uninterruptible atom, and a budget that + // hid that would be lying about what one drain can cost. + reset_state() => |this| 2 * this.cells() + this.scrollback * this.columns; + } +} diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs index 8e5310283..9486aa874 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/clusters.rs @@ -33,7 +33,7 @@ fn render(input: &str) -> (Vec, Receiver) { }; let (term, actions) = Terminal::new(size, Fences::ALL); let shared = SharedTerminal::new(term); - shared.feed(input.as_bytes()); + shared.feed_fully(input.as_bytes()); let mut encoder = Encoder::new(); let frame = shared.render(&mut encoder); let spans = frame @@ -246,7 +246,7 @@ fn a_run_longer_than_u16_max_splits_rather_than_wrapping() { let shared = SharedTerminal::new(term); // One character is enough: the rest of the row is blank cells of the same // style, so the whole row is a single candidate run. - shared.feed(b"a"); + shared.feed_fully(b"a"); let mut encoder = Encoder::new(); let frame = shared.render(&mut encoder); @@ -297,7 +297,7 @@ fn wrapping_does_not_split_a_uniform_run() { let shared = SharedTerminal::new(term); // Six narrow cells in one style: five fill row 0 and set WRAPLINE on the // last of them, the sixth lands on row 1. - shared.feed(b"abcdef"); + shared.feed_fully(b"abcdef"); let mut encoder = Encoder::new(); let frame = shared.render(&mut encoder); @@ -327,7 +327,7 @@ fn leading_wide_glyph_after_wrap_is_positioned_from_column_zero() { let shared = SharedTerminal::new(term); // Four narrow cells fill 0..=3, leaving one column: the wide glyph cannot // fit and moves to the next row. - shared.feed("abcd\u{4E00}".as_bytes()); + shared.feed_fully("abcd\u{4E00}".as_bytes()); let mut encoder = Encoder::new(); let frame = shared.render(&mut encoder); diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/fences.rs b/desktop/src-tauri/crates/buzz-terminal/tests/fences.rs index a753841e3..72bb8c2e6 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/fences.rs @@ -23,12 +23,12 @@ fn size() -> Size { } fn feed_synchronized(term: &mut Terminal, payload: &[u8], close: bool) { - term.feed(b"\x1b[?2026h"); + term.feed_fully(b"\x1b[?2026h"); for chunk in payload.chunks(CHUNK) { - term.feed(chunk); + term.feed_fully(chunk); } if close { - term.feed(b"\x1b[?2026l"); + term.feed_fully(b"\x1b[?2026l"); } } @@ -107,9 +107,9 @@ fn g1_sync_abort_bounds_all_hostile_shapes() { #[test] fn g2_hostile_unsynchronized_osc_resets_parser() { let (mut term, _) = Terminal::new(size(), Fences::ALL); - term.feed(b"\x1b]0;"); + term.feed_fully(b"\x1b]0;"); for chunk in repeated(b"A", OSC_BUDGET * 4).chunks(CHUNK) { - term.feed(chunk); + term.feed_fully(chunk); } assert!(term.stats().osc_resets > 0, "F2 never rebuilt the parser"); } diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/latency.rs b/desktop/src-tauri/crates/buzz-terminal/tests/latency.rs index 57d672d91..0edbefce1 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/latency.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/latency.rs @@ -78,7 +78,7 @@ fn flood(shared: &SharedTerminal, stop: &AtomicBool) { if stop.load(Ordering::Relaxed) { return; } - shared.feed(chunk); + shared.feed_fully(chunk); } } } diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs b/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs index 5b1dfbe8f..6548de9b4 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/resize.rs @@ -45,7 +45,7 @@ fn grid(columns: usize, screen_lines: usize) -> Size { fn resize_forces_a_full_frame_at_the_new_width() { let (shared, _actions) = shared(size(40)); let mut encoder = Encoder::new(); - shared.feed(b"\x1b[2J\x1b[Hhello world\r\nsecond line\r\n"); + shared.feed_fully(b"\x1b[2J\x1b[Hhello world\r\nsecond line\r\n"); let first = shared.render(&mut encoder); assert!(first.full, "first frame after a fresh Term must be full"); @@ -100,7 +100,7 @@ fn resize_forces_a_full_frame_at_the_new_width() { fn identical_resize_is_inert() { let (shared, _actions) = shared(size(40)); let mut encoder = Encoder::new(); - shared.feed(b"hello"); + shared.feed_fully(b"hello"); shared.render(&mut encoder); let applied = shared.resize(size(40)); @@ -134,7 +134,7 @@ fn identical_resize_is_inert() { fn full_frame_after_height_resize_republishes_unchanged_rows() { let (shared, _actions) = shared(grid(40, 10)); let mut encoder = Encoder::new(); - shared.feed(b"\x1b[2J\x1b[Hhello world\r\nsecond line"); + shared.feed_fully(b"\x1b[2J\x1b[Hhello world\r\nsecond line"); let first = shared.render(&mut encoder); assert!(first.full); assert_eq!(first.rows.len(), 10); @@ -173,7 +173,7 @@ fn full_frame_after_height_resize_republishes_unchanged_rows() { fn a_frame_is_stamped_with_the_grid_it_was_captured_on() { let (shared, _actions) = shared(grid(40, 10)); let mut encoder = Encoder::new(); - shared.feed(b"\x1b[2J\x1b[Hhello world"); + shared.feed_fully(b"\x1b[2J\x1b[Hhello world"); let in_flight = shared.render(&mut encoder); assert_eq!(in_flight.viewport.generation, 0); diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs new file mode 100644 index 000000000..d597ece7e --- /dev/null +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -0,0 +1,760 @@ +//! The work-denominated slicing seam: what bounds one lock hold, what bounds +//! the queue behind it, and what proves the work was actually done. +//! +//! Every assertion here is an **exact** expected value, never a `> 0`. A fix +//! that bounds the lock by *dropping* work instead of deferring it reports a +//! beautiful latency and a perfect screen-content receipt -- DECALN fills the +//! grid with `E`, and the second DECALN overwrites the first, so grid content +//! saturates after one of ten thousand. `completed_units == expected` is the +//! only predicate that separates "deferred the work" from "skipped it", and +//! `> 0` is satisfied by a seam that executed exactly one unit. + +use buzz_terminal::fences::{ + max_atom_work, max_drain_work, slice_bytes, Fences, MAX_SLICE, MIN_SLICE, SYNC_CAP, TAIL_CAP, + WORK_BUDGET, +}; +use buzz_terminal::{Size, Terminal}; + +const COLUMNS: usize = 200; +const LINES: usize = 50; +const CELLS: u64 = (COLUMNS * LINES) as u64; + +fn terminal() -> Terminal { + Terminal::new( + Size { + columns: COLUMNS, + screen_lines: LINES, + scrollback: 100, + }, + Fences::ALL, + ) + .0 +} + +/// A deliberately tiny grid, for the arms that must fill the 4 MiB tail. +/// +/// Filling the cap is cheap; *draining* it is not, and on a 200x50 grid a +/// full tail of DECALN is ~1e10 work units of real parsing. The cap is a +/// property of the byte depth, not of the grid, so a small grid exercises the +/// same thresholds in seconds instead of minutes -- but it does change what +/// is being tested, so it is named rather than reused silently: these arms +/// test the *depth* predicates, and the arms above test the work bound. +fn tiny() -> Terminal { + Terminal::new( + Size { + columns: 10, + screen_lines: 2, + scrollback: 10, + }, + Fences::ALL, + ) + .0 +} + +/// Feed until the tail reaches its cap, or give up. +/// +/// Bounded on purpose. A test that loops until a predicate goes true hangs +/// forever when the predicate is what broke, which turns a killed mutant into +/// a wedged CI job -- and a suite that hangs instead of failing is a suite +/// nobody can bisect. +fn fill_tail(term: &mut Terminal, payload: &[u8]) -> bool { + for _ in 0..10_000 { + if term.tail_full() { + return true; + } + term.feed(payload); + } + false +} + +/// Pump to completion, counting acquisitions. A drain that needed no second +/// call returns 1. +fn pump(term: &mut Terminal, bytes: &[u8]) -> usize { + let mut calls = 1; + let mut more = term.feed(bytes); + while more { + more = term.drain(); + calls += 1; + } + calls +} + +/// One `feed` may not spend an unbounded amount of work, however much the +/// stream asks for. +/// +/// Kills: deleting the `spent >= WORK_BUDGET` break, which restores the +/// unbounded hold this whole seam exists to prevent. Deliberately asserts on +/// *work* rather than wall time -- a time assertion is a flake on a loaded +/// machine, and the work bound is the thing the code actually promises. +#[test] +fn one_feed_spends_at_most_one_budget_plus_a_slice() { + let mut term = terminal(); + let decalns = 10_000; + term.feed(&b"\x1b#8".repeat(decalns)); + + let spent = term.stats().completed_work; + // Two terms, both irreducible: the budget is checked between slices, and + // a slice is sized so it holds at most one budget of the densest payload; + // and the callback that crosses the line cannot be preempted. + let ceiling = max_drain_work(COLUMNS, LINES, 100); + assert!( + spent <= ceiling, + "one feed spent {spent} work, over budget+overshoot ({ceiling})", + ); + assert!( + term.pending_bytes() > 0, + "10000 DECALNs is {} work and the budget is {WORK_BUDGET}; if nothing \ + is pending the seam ran the whole payload in one hold", + decalns as u64 * CELLS, + ); +} + +/// Every deferred byte is eventually executed -- exactly once, and all of it. +/// +/// Kills: bounding the hold by dropping the remainder instead of keeping it +/// (`self.pending.clear()` in place of the tail), which passes any latency +/// gate and any grid-content check. The unit count is the only witness. +#[test] +fn a_deferred_tail_executes_every_unit_exactly_once() { + let mut term = terminal(); + let decalns = 10_000; + + let calls = pump(&mut term, &b"\x1b#8".repeat(decalns)); + + assert!( + calls > 1, + "a payload this dense must have needed a second call" + ); + assert_eq!( + term.stats().completed_units, + decalns as u64, + "every DECALN must execute exactly once: no drops, no double-parse", + ); + assert_eq!(term.stats().completed_work, decalns as u64 * CELLS); + assert_eq!(term.pending_bytes(), 0, "nothing may be left behind"); +} + +/// The tail drains without another `feed` -- a reader with nothing new to +/// read must still be able to retire what it already accepted. +/// +/// Kills: draining only from `feed`, which strands the tail whenever the +/// child goes quiet (`cat bigfile` then no more output: the last screenful +/// never appears). +#[test] +fn a_tail_drains_without_a_second_feed() { + let mut term = terminal(); + let decalns = 2_000; + assert!(term.feed(&b"\x1b#8".repeat(decalns)), "expected a tail"); + + // Never feed again. Only drain. + while term.drain() {} + + assert_eq!(term.stats().completed_units, decalns as u64); + assert_eq!(term.pending_bytes(), 0); +} + +/// A slice is cut only at a byte boundary the parser has already passed, so +/// an escape sequence split across two slices still executes once. +/// +/// Kills: cutting mid-sequence and restarting the parser, or double-feeding +/// the straddling bytes. `\x1b#8` is 3 bytes and slices are a multiple of +/// neither, so at this length hundreds of sequences straddle a cut. +#[test] +fn a_sequence_split_across_slices_executes_exactly_once() { + let mut term = terminal(); + let decalns = 3_000; + pump(&mut term, &b"\x1b#8".repeat(decalns)); + assert_eq!( + term.stats().completed_units, + decalns as u64, + "a straddling sequence was dropped or executed twice", + ); + + // Same payload, delivered one byte per feed: every sequence straddles. + let mut byte_at_a_time = terminal(); + for chunk in b"\x1b#8".repeat(decalns).chunks(1) { + byte_at_a_time.feed(chunk); + } + while byte_at_a_time.drain() {} + assert_eq!(byte_at_a_time.stats().completed_units, decalns as u64); +} + +/// The tail is a bound on the queue, and the breach counter is loud. +/// +/// Kills: a silent cap -- a tail that grows past `TAIL_CAP` without saying +/// so is indistinguishable from a reader that is obeying backpressure, which +/// is exactly the confusion that hides an unbounded queue. +#[test] +fn an_overrun_tail_is_capped_and_counted() { + let mut term = tiny(); + assert!(!term.tail_full(), "a fresh terminal is not full"); + assert!(term.tail_drained(), "a fresh terminal is drained"); + assert_eq!(term.stats().tail_breaches, 0); + + // A reader that ignores `tail_full` and keeps shovelling. + assert!( + fill_tail(&mut term, &b"\x1b#8".repeat(20_000)), + "the tail never reached its cap: the queue is not bounded", + ); + + assert!(term.pending_bytes() >= TAIL_CAP); + assert!( + term.stats().tail_breaches > 0, + "reaching the cap must be counted, not absorbed silently", + ); + assert!(!term.tail_drained(), "a full tail is not a drained tail"); +} + +/// Resume is hysteretic: `tail_drained` does not go true the instant the tail +/// falls one byte below the cap. +/// +/// Kills: `tail_drained() == !tail_full()`, which makes a reader flap between +/// paused and reading once per slice at exactly the moment it is most loaded. +#[test] +fn resume_waits_for_a_low_water_mark_not_merely_a_non_full_tail() { + let mut term = tiny(); + assert!( + fill_tail(&mut term, &b"\x1b#8".repeat(20_000)), + "expected a full tail" + ); + + // Drain until the reader is allowed to resume, watching for a window in + // which it is neither full nor drained -- that gap *is* the hysteresis. + let mut saw_gap = false; + for _ in 0..1_000_000 { + if term.tail_drained() { + break; + } + assert!(term.drain() || term.tail_drained()); + if !term.tail_full() && !term.tail_drained() { + saw_gap = true; + } + } + assert!( + term.tail_drained(), + "the tail never drained to the resume mark" + ); + assert!( + saw_gap, + "no depth was both non-full and non-drained: the two thresholds are \ + the same value and the reader will flap", + ); +} + +/// Close must not be held behind parser work. +/// +/// Kills: draining the tail on close instead of discarding it. Measured +/// elsewhere in this project: teardown that finishes parsing before killing +/// the child costs ~600 ms on macOS, and no byte of that work reaches a +/// renderer -- publication is detached before shutdown drains. +#[test] +fn close_may_abandon_the_tail_and_says_how_much_it_dropped() { + let mut term = terminal(); + term.feed(&b"\x1b#8".repeat(10_000)); + let stranded = term.pending_bytes(); + assert!(stranded > 0); + + let abandoned = term.abandon_tail(); + + assert_eq!(abandoned, stranded); + assert_eq!(term.pending_bytes(), 0); + assert_eq!( + term.stats().abandoned_bytes, + stranded as u64, + "dropped bytes must be counted: this is lossy by design and silent \ + loss is how it stops being by design", + ); + assert!( + term.tail_drained(), + "an abandoned tail cannot strand a reader" + ); +} + +/// The grid the weights are priced against tracks resizes. +/// +/// Kills: dropping `Feeder::resize`. A stale grid misprices every O(cells) +/// charge for as long as it is wrong -- and it is wrong in the *unsafe* +/// direction whenever the window grows, which is the common case. +#[test] +fn a_resize_reprices_the_same_escape() { + let mut small = terminal(); + small.feed_fully(b"\x1b#8"); + let before = small.stats().completed_work; + assert_eq!(before, CELLS); + + small.resize(Size { + columns: COLUMNS * 2, + screen_lines: LINES, + scrollback: 100, + }); + small.reset_stats(); + small.feed_fully(b"\x1b#8"); + + assert_eq!( + small.stats().completed_work, + CELLS * 2, + "the same escape on a grid twice as wide must cost twice as much", + ); + assert_eq!(small.stats().completed_units, 1, "still one callback"); +} + +/// A resize *between* slices of one payload reprices the remainder. +/// +/// Kills: caching the slice size or the grid across a drain. The tail +/// outlives the call that accepted it, so a resize can land in the middle of +/// it -- the untouched remainder must be charged at the new grid, not the one +/// that was current when the bytes arrived. +#[test] +fn a_resize_mid_tail_reprices_the_remainder() { + let mut term = terminal(); + let decalns = 4_000; + assert!(term.feed(&b"\x1b#8".repeat(decalns)), "expected a tail"); + let done_before = term.stats().completed_units; + let work_before = term.stats().completed_work; + assert_eq!(work_before, done_before * CELLS); + + term.resize(Size { + columns: COLUMNS * 2, + screen_lines: LINES, + scrollback: 100, + }); + while term.drain() {} + + let after = term.stats(); + assert_eq!(after.completed_units, decalns as u64, "no unit may be lost"); + assert_eq!( + after.completed_work, + work_before + (decalns as u64 - done_before) * CELLS * 2, + "the remainder must be priced at the resized grid", + ); +} + +/// Slice size is derived from the worst atom the grid admits, because a fixed +/// byte count cannot bound a lock hold: `ESC c` is two bytes and resets both +/// grids plus scrollback. +/// +/// Kills: replacing `slice_bytes` with a constant, or deriving it from +/// `cells` while the worst atom is larger than `cells`. Measured: 256 bytes +/// of DECALN is 1.6 ms at 200x50 and ~14 ms at 1600x50, so no one constant +/// serves both. +#[test] +fn slice_size_shrinks_as_the_worst_atom_grows() { + let small = slice_bytes(80, 24, 0); + let large = slice_bytes(1600, 50, 0); + assert!( + small > large, + "a bigger grid makes each byte more expensive, so slices must shrink: \ + 80x24 -> {small}, 1600x50 -> {large}", + ); + assert!( + slice_bytes(200, 50, 10_000) <= slice_bytes(200, 50, 0), + "scrollback makes RIS more expensive, so it may only shrink slices", + ); + for (columns, lines, scrollback) in [(80, 24, 0), (200, 50, 0), (400, 100, 0), (1600, 50, 0)] { + assert!((MIN_SLICE..=MAX_SLICE).contains(&slice_bytes(columns, lines, scrollback))); + // One slice holds at most N/2 of the densest atom. Either that fits a + // budget, or the floor binds -- and then the overshoot is stated by + // `max_drain_work` rather than being an accident. + let worst = (slice_bytes(columns, lines, scrollback) as u64 / 2) + * max_atom_work(columns, lines, scrollback); + assert!( + worst <= WORK_BUDGET || slice_bytes(columns, lines, scrollback) == MIN_SLICE, + "{columns}x{lines}: a slice buys {worst} work against a \ + {WORK_BUDGET} budget without the MIN clamp to excuse it", + ); + } +} + +/// Work released by an F1 abort is counted. +/// +/// Kills: leaving the `stop_sync` flush out of the accounting. F1 aborts a +/// runaway synchronized update by flushing its buffer through the handler -- +/// those callbacks run, cost time, and hold the lock, so a scheduler that +/// does not see them is blind on exactly the path the fence created. The +/// escapes here are `ESC#8` so the flushed work is unmistakable against the +/// buffered bytes. +#[test] +fn work_flushed_by_a_sync_abort_is_counted() { + let (mut term, _a) = Terminal::new( + Size { + columns: 80, + screen_lines: 24, + scrollback: 0, + }, + Fences::SYNC_ONLY, + ); + let cells = 80 * 24; + + // Open a synchronized update and never close it: F1 must abort it once + // the buffer passes SYNC_CAP, flushing everything buffered so far. + term.feed_fully(b"\x1b[?2026h"); + let decalns = SYNC_CAP / 3 + 1000; + term.feed_fully(&b"\x1b#8".repeat(decalns)); + + let stats = term.stats(); + assert!(stats.sync_aborts > 0, "the fence must have fired"); + // Every DECALN fed must be accounted for. The comparison is against the + // *input*, not against the counters' own internal consistency: an + // uncounted flush leaves both counters small together, so checking them + // against each other would pass over the mutant. + // Two bookkeeping callbacks besides the DECALNs: the `ESC[?2026h` that + // opened the update, and the `unset_private_mode` that `stop_sync` emits + // per abort to report the mode off (`vte-0.15.0/src/ansi.rs:353`). + let bookkeeping = 1 + stats.sync_aborts; + assert_eq!( + stats.completed_units, + decalns as u64 + bookkeeping, + "every DECALN must be counted, including the ones released by the \ + abort, plus {bookkeeping} mode callbacks", + ); + assert_eq!( + stats.completed_work, + decalns as u64 * cells + bookkeeping, + "and their work: {decalns} DECALNs at {cells} cells each", + ); +} + +/// Cheap traffic is not taxed by slicing: an ordinary screenful retires in +/// one call. +/// +/// Kills: a budget so small, or a slice so small, that normal output pays the +/// deferral machinery. This is the companion to the DECALN arm -- a seam that +/// bounds the hold by making everything slow has not fixed anything. +#[test] +fn ordinary_output_needs_no_second_call() { + let mut term = terminal(); + let line = b"\x1b[1;32mbuzz\x1b[0m substrate line of output 0123456789\r\n"; + let screenful = line.repeat(LINES); + + assert!( + !term.feed(&screenful), + "a screenful of ordinary output must retire in one call, not defer", + ); + assert_eq!(term.pending_bytes(), 0); + assert_eq!(term.stats().tail_breaches, 0); +} + +/// The work bound holds on the **first drain of a fresh feeder**, for the +/// densest payload upstream offers. +/// +/// Kills: sizing slices from observed density. A learned bound is not a bound +/// on the first slice -- a cold feeder has seen nothing, so it hands the +/// parser a wide slice, and a wide slice of `ESC c` spends many budgets +/// before anything checks. This is the arm that a warm-up-based scheduler +/// passes on the second call and fails on the first, so it asserts on a +/// terminal that has never parsed a byte. +#[test] +fn a_cold_feeder_bounds_its_very_first_slice() { + for (columns, lines) in [(80, 24), (200, 50), (400, 100), (1600, 50)] { + for (label, atom) in [("RIS", &b"\x1bc"[..]), ("DECALN", &b"\x1b#8"[..])] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: lines, + scrollback: 100, + }, + Fences::ALL, + ); + // Never fed before: `density`-style state, if any existed, is at + // its initial value. + term.feed(&atom.repeat(5_000)); + + let spent = term.stats().completed_work; + let ceiling = max_drain_work(columns, lines, 100); + assert!( + spent <= ceiling, + "{label} at {columns}x{lines}: first drain of a cold feeder \ + spent {spent} work, over budget+overshoot ({ceiling})", + ); + assert!(term.pending_bytes() > 0, "{label}: expected a tail"); + } + } +} + +/// Exact price of every escape whose cost the grid can amplify. +/// +/// One table, exact `completed_work` per escape, at two widths so a weight +/// that dropped its `columns` factor cannot hide. Kills, one row each: +/// +/// * `delete_chars`/`insert_blank` charged by `N` -- their cost *falls* as N +/// rises (the swap loop runs `columns - end` times), so N=1 is the worst +/// case and pricing by N is backwards. +/// * `erase_chars` charged raw `N` -- upstream clamps to the row, so +/// `ESC[65535X` on an 80-column grid touches 80 cells, not 65535. +/// * `scroll_up`/`delete_lines` losing their `columns` factor -- the rows are +/// reset, and a row reset is O(columns). +/// * `clear_line`, `decaln`, `clear_screen` mispriced by an axis. +/// +/// Exact equality, never a bound: a `<=` assertion passes for every weight +/// smaller than the truth, which is the direction that hurts. +#[test] +fn every_amplifiable_escape_is_priced_exactly() { + for (columns, lines) in [(80usize, 24usize), (400, 50)] { + let cells = (columns * lines) as u64; + let c = columns as u64; + let cases: &[(&str, String, u64)] = &[ + ("decaln", "\u{1b}#8".into(), cells), + ("clear_screen", "\u{1b}[2J".into(), cells), + ("clear_line", "\u{1b}[2K".into(), c), + ("erase_chars N=1", "\u{1b}[1X".into(), 1), + ("erase_chars N=20", "\u{1b}[20X".into(), 20), + ("erase_chars N=huge", "\u{1b}[65535X".into(), c), + ("delete_chars N=1", "\u{1b}[1P".into(), c), + ("delete_chars N=huge", "\u{1b}[65535P".into(), c), + ("insert_blank N=1", "\u{1b}[1@".into(), c), + ("scroll_up N=1", "\u{1b}[1S".into(), c), + ("scroll_up N=5", "\u{1b}[5S".into(), 5 * c), + ("scroll_up N=huge", "\u{1b}[65535S".into(), lines as u64 * c), + ("delete_lines N=3", "\u{1b}[3M".into(), 3 * c), + ("sgr", "\u{1b}[m".into(), 1), + ("goto", "\u{1b}[1;1H".into(), 1), + ]; + for (label, seq, expected) in cases { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: lines, + scrollback: 0, + }, + Fences::ALL, + ); + // Home first so nothing scrolls, then measure only the escape. + term.feed_fully(b"\x1b[1;1H"); + term.reset_stats(); + term.feed_fully(seq.as_bytes()); + + assert_eq!(term.stats().completed_units, 1, "{label}: one callback"); + assert_eq!( + term.stats().completed_work, + *expected, + "{label} at {columns}x{lines} priced wrong", + ); + } + } +} + +/// RIS is priced with its history axis, not just its cells. +/// +/// Kills: charging `cells`, or charging the *active* grid's history. RIS +/// resets both grids and walks the primary's scrollback, and `history_size()` +/// observes only the active grid -- so a filled primary followed by +/// `ESC[?1049h` reads as empty while the work is still paid. Configured depth +/// is the only stateless quantity that bounds both, and this asserts the +/// charge tracks it. +#[test] +fn ris_is_priced_for_both_grids_and_the_scrollback_it_walks() { + let (columns, lines) = (80usize, 24usize); + let cells = (columns * lines) as u64; + for scrollback in [0usize, 100, 10_000] { + for (label, prefix) in [("primary", ""), ("alt screen", "\u{1b}[?1049h")] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: lines, + scrollback, + }, + Fences::ALL, + ); + term.feed_fully(prefix.as_bytes()); + term.reset_stats(); + term.feed_fully(b"\x1bc"); + + assert_eq!( + term.stats().completed_work, + 2 * cells + (scrollback * columns) as u64, + "{label}, scrollback {scrollback}: RIS must be priced on \ + configured depth, which is the same on both grids", + ); + } + } +} + +/// CBT is charged for exactly the cells it scans -- an equality, in both +/// directions. +/// +/// Kills: delegating `move_backward_tabs` verbatim, and deleting the +/// fixed-point break. With tabstops cleared and the cursor at the right +/// margin, upstream never advances the cursor, so its `col == 0` exit is +/// unreachable and all N iterations rescan the row -- `ESC[3g ESC[65535Z` is +/// 8 bytes for 82 ms at 1600 columns. +/// +/// Two traps this had to be written around, both of which I walked into +/// first: +/// +/// * **The cursor is not the witness.** Deleting the break lands on the same +/// column; only the cost differs. A fixture checking where the cursor ended +/// up passes over the mutant. +/// * **An upper bound is not the witness either.** Deleting the break makes +/// the loop run without charging -- measured `work == 1` for 29 ms of real +/// scanning -- so `spent <= bound` *passes*. Under-charging is exactly the +/// direction that hurts, and only an equality sees it. +/// +/// The expected value is the scan the source performs: with no stop below the +/// cursor, one pass over `cursor_column` cells, then a permanent fixed point. +/// Both arms come to `columns` -- the telescoping sum of a walk, or one +/// failed pass -- which is the bound this whole change buys. +#[test] +fn the_worst_atom_is_charged_for_exactly_what_it_scans() { + for columns in [80usize, 400, 1600] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: 50, + scrollback: 0, + }, + Fences::ALL, + ); + // Adversarial for cost: every tabstop gone, cursor at the right + // margin, count far past the width. + term.feed_fully(format!("\u{1b}[3g\u{1b}[1;{columns}H").as_bytes()); + term.reset_stats(); + + term.feed_fully(b"\x1b[65535Z"); + + assert_eq!(term.stats().completed_units, 1, "one escape, one callback"); + assert_eq!( + term.stats().completed_work, + 1 + (columns as u64 - 1), + "one failed scan over the whole prefix, then a permanent fixed \ + point: the charge is the escape plus that one scan. A loop that \ + kept going would charge this much per iteration, 65535 times", + ); + // The real guard on the loop: with a stop reachable, the charge must + // equal the distance actually travelled. A break-less loop scans the + // row 65535 times and charges for one crossing. + let (mut walk, _a) = Terminal::new( + Size { + columns, + screen_lines: 50, + scrollback: 0, + }, + Fences::ALL, + ); + // Default tabstops every 8: from the right margin a huge count walks + // to column 0, crossing every column on the way. + walk.feed_fully(format!("\u{1b}[1;{columns}H").as_bytes()); + walk.reset_stats(); + walk.feed_fully(b"\x1b[65535Z"); + + assert_eq!(walk.term().grid().cursor.point.column.0, 0); + assert_eq!( + walk.stats().completed_work, + 1 + (columns as u64 - 1), + "the charge must be the distance travelled: one unit for the \ + escape plus one per column crossed", + ); + } +} + +/// CBT at column 0 is free, and stays free. +/// +/// Kills: removing the `before == 0` guard. Upstream has its own `col == 0` +/// break, so deleting the wrapper's copy is invisible to the cursor and +/// invisible to timing -- it only shows up as work charged for a scan over +/// zero cells that the wrapper attributed to itself. The left margin is also +/// the position both earlier sweeps of this op homed to, which is why it is +/// the position where a defect hides best. +#[test] +fn the_worst_atom_costs_nothing_at_the_left_margin() { + for columns in [80usize, 400] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: 50, + scrollback: 0, + }, + Fences::ALL, + ); + term.feed_fully(b"\x1b[3g\x1b[1;1H"); + term.reset_stats(); + term.feed_fully(b"\x1b[65535Z"); + + assert_eq!( + term.stats().completed_work, + 1, + "at column 0 there is nothing to the left to scan, so the escape \ + costs one unit and no cells", + ); + assert_eq!(term.term().grid().cursor.point.column.0, 0); + } +} + +/// The other adversary: every tabstop *set*, which maximises the number of +/// delegated single steps rather than the length of one scan. +/// +/// Kills: pricing CBT per-step-times-width. Cleared tabstops attack the +/// clamp; all-set attacks the break, forcing `columns - 1` steps of one +/// column each. The two layouts peak in different terms and neither may +/// exceed the bound, so both are here -- a suite that tested only the famous +/// one would miss the shape it chose against. +#[test] +fn the_worst_atom_is_bounded_under_the_layout_that_maximises_steps() { + for columns in [80usize, 400] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: 50, + scrollback: 100, + }, + Fences::ALL, + ); + // A tabstop in every column, then start from the right margin. + term.feed_fully(b"\x1b[3g"); + for c in 1..=columns { + term.feed_fully(format!("\u{1b}[1;{c}H\u{1b}H").as_bytes()); + } + term.feed_fully(format!("\u{1b}[1;{columns}H").as_bytes()); + term.reset_stats(); + + term.feed_fully(b"\x1b[65535Z"); + + let spent = term.stats().completed_work; + assert_eq!(term.stats().completed_units, 1); + assert!( + spent <= 2 * columns as u64, + "all-set CBT at {columns} columns charged {spent}, over 2 x columns", + ); + assert_eq!( + term.term().grid().cursor.point.column.0, + 0, + "with a stop in every column the cursor must walk all the way", + ); + } +} + +/// Stopping CBT early does not change where the cursor lands. +/// +/// The companion to the two cost tests above: they assert the work fell, +/// this asserts the behaviour did not move. Kills: stopping at something that +/// is *not* a fixed point -- `min(N, 1)`, or breaking whenever a scan fails +/// even though an earlier step still had stops to find. Cases are the ones +/// the exhaustive probe found interesting: no stops, one stop mid-row, and +/// default stops, each from the right margin with a count past the width. +#[test] +fn stopping_the_worst_atom_early_preserves_its_semantics() { + let columns = 40usize; + let cursor_column = |setup: &str| -> usize { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: 3, + scrollback: 0, + }, + Fences::ALL, + ); + term.feed_fully(setup.as_bytes()); + term.term().grid().cursor.point.column.0 + }; + + // No stops: the cursor cannot move, whatever the count. + assert_eq!(cursor_column("\u{1b}[3g\u{1b}[1;40H\u{1b}[65535Z"), 39); + assert_eq!(cursor_column("\u{1b}[3g\u{1b}[1;40H\u{1b}[40Z"), 39); + // One stop at column 20 (1-based 21): reachable once, then stuck. + let one_stop = "\u{1b}[3g\u{1b}[1;21H\u{1b}H\u{1b}[1;40H"; + assert_eq!( + cursor_column(&format!("{one_stop}\u{1b}[65535Z")), + cursor_column(&format!("{one_stop}\u{1b}[40Z")), + ); + // Default stops every 8: a large count walks all the way to column 0. + assert_eq!(cursor_column("\u{1b}[1;40H\u{1b}[65535Z"), 0); +} diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs index 5ad8a62c9..a7305b52f 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/snapshot.rs @@ -49,7 +49,7 @@ fn visible_text(frame: &buzz_terminal::damage::Frame) -> Vec { #[test] fn a_late_render_shows_only_the_next_change_but_a_snapshot_shows_the_screen() { let (shared, _actions) = terminal(20, 4); - shared.feed(b"first\r\nsecond\r\nthird"); + shared.feed_fully(b"first\r\nsecond\r\nthird"); // The incumbent consumes the damage from that output. let mut incumbent = Encoder::new(); @@ -101,12 +101,12 @@ fn a_snapshot_does_not_steal_the_incumbents_damage() { // content is shorter than its replacement so the rewrite below covers it // completely and no tail of it survives. let mut incumbent = Encoder::new(); - shared.feed(b"old"); + shared.feed_fully(b"old"); let _ = shared.render(&mut incumbent); // Rewrite row 0, then park the cursor on row 3. The incumbent is now owed // row 0, which is not the row the cursor will re-damage for free. - shared.feed(b"\x1b[1;1HAFTER\x1b[4;1H"); + shared.feed_fully(b"\x1b[1;1HAFTER\x1b[4;1H"); // A second subscriber attaches and snapshots first. let mut attaching = Encoder::new(); @@ -138,7 +138,7 @@ fn a_snapshot_does_not_steal_the_incumbents_damage() { #[test] fn a_snapshot_realigns_a_reused_encoders_dedup_state() { let (shared, _actions) = terminal(20, 4); - shared.feed(b"wide enough line"); + shared.feed_fully(b"wide enough line"); let mut encoder = Encoder::new(); let before = shared.snapshot(&mut encoder); @@ -200,7 +200,7 @@ fn a_snapshot_carries_every_row_and_the_true_cursor() { let (shared, _actions) = terminal(20, 4); // Write the bottom row of the screen, then park the cursor at line 4, // column 6 (1-based) -- row 3, column 5 to us. - shared.feed(b"\x1b[4;1Hbottom\x1b[4;6H"); + shared.feed_fully(b"\x1b[4;1Hbottom\x1b[4;6H"); let mut attaching = Encoder::new(); let frame = shared.snapshot(&mut attaching); @@ -226,7 +226,7 @@ fn a_snapshot_carries_every_row_and_the_true_cursor() { // ...and a hidden cursor is reported hidden, so `visible` tracks the mode // rather than being a constant that happens to match the default. - shared.feed(b"\x1b[?25l"); + shared.feed_fully(b"\x1b[?25l"); let mut second = Encoder::new(); assert!( !shared.snapshot(&mut second).cursor.visible, @@ -245,7 +245,7 @@ fn a_snapshot_carries_every_row_and_the_true_cursor() { #[test] fn a_snapshot_is_billed_to_the_renderer_plane() { let (shared, _actions) = terminal(20, 4); - shared.feed(b"content"); + shared.feed_fully(b"content"); shared.reader_acquire().reset(); shared.renderer_acquire().reset(); @@ -271,7 +271,7 @@ fn a_snapshot_is_billed_to_the_renderer_plane() { #[test] fn snapshots_are_repeatable() { let (shared, _actions) = terminal(20, 4); - shared.feed(b"persistent"); + shared.feed_fully(b"persistent"); let mut first = Encoder::new(); let mut second = Encoder::new(); From 04e9d0dc7edb8be2bbf488e8881814a881d618a9 Mon Sep 17 00:00:00 2001 From: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Date: Sun, 2 Aug 2026 01:00:39 -0400 Subject: [PATCH 05/15] fix(terminal): raw-drain while child exits Co-authored-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Signed-off-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> --- .../crates/buzz-terminal/src/lifecycle.rs | 14 ++++- .../buzz-terminal/src/lifecycle_tests.rs | 25 +++++++- .../crates/buzz-terminal/src/shared.rs | 57 ++++++++++++++++++- desktop/src-tauri/src/terminal_runtime.rs | 52 ++++++++++++++--- desktop/src-tauri/src/terminal_transport.rs | 5 ++ 5 files changed, 138 insertions(+), 15 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle.rs b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle.rs index b5fdc1268..dbd76f56e 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle.rs @@ -232,7 +232,13 @@ pub const MAX_LIVE_SESSIONS: usize = 20; /// drained. Join the reader first and every tab close pays that, on the arm /// where the user is already waiting. pub trait DrainingReader { - /// Stops consuming and releases the reader. Called *after* reap. + /// Detach parser work and enter raw-drain mode before child termination. + fn begin_closing(&self); + + /// Wake a reader that remains blocked after the child has been reaped. + fn stop(&self); + + /// Releases the reader thread. Called only after [`DrainingReader::stop`]. fn join(self: Box); } @@ -249,11 +255,13 @@ pub fn shutdown_draining( child: &mut Box, reader: Box, ) -> io::Result { + reader.begin_closing(); // Terminate and reap with the reader still running, so the child never // blocks in a tty write while we are waiting on it. let outcome = shutdown(child); - // Unconditional: the reader must be released whether or not the shutdown - // reported an error, or a failed close leaks the thread and its fd. + // Unconditional: wake and release the reader whether or not shutdown + // reported an error. EOF may already have ended it; stop is idempotent. + reader.stop(); reader.join(); outcome } diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs index 17068f21a..c2a5e2eda 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs @@ -430,7 +430,8 @@ fn reader_drains_through_termination_and_reap() { let after_close = std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)); let closing = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); - let reader = RecordingReader::spawn(&pair, after_close.clone(), closing.clone()); + let order = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let reader = RecordingReader::spawn(&pair, after_close.clone(), closing.clone(), order.clone()); // Let the child get well ahead of the reader before we touch anything. assert!( @@ -461,6 +462,11 @@ fn reader_drains_through_termination_and_reap() { period: the child was blocked writing to an undrained master rather \ than exiting on SIGTERM" ); + assert_eq!( + *order.lock().unwrap(), + ["begin_closing", "stop", "join"], + "reader close must begin before termination and stop/join only after reap" + ); } /// Spawns a child that floods the PTY without pause. @@ -478,6 +484,7 @@ fn spawn_noisy(pair: &PtyPair) -> Box { struct RecordingReader { total: std::sync::Arc, handle: std::thread::JoinHandle<()>, + order: std::sync::Arc>>, } impl RecordingReader { @@ -485,6 +492,7 @@ impl RecordingReader { pair: &PtyPair, after_close: std::sync::Arc, closing: std::sync::Arc, + order: std::sync::Arc>>, ) -> Self { let mut reader = pair.master.try_clone_reader().expect("reader"); let total = std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)); @@ -502,7 +510,11 @@ impl RecordingReader { } } }); - Self { total, handle } + Self { + total, + handle, + order, + } } fn total_bytes(&self) -> u64 { @@ -511,7 +523,16 @@ impl RecordingReader { } impl DrainingReader for RecordingReader { + fn begin_closing(&self) { + self.order.lock().unwrap().push("begin_closing"); + } + + fn stop(&self) { + self.order.lock().unwrap().push("stop"); + } + fn join(self: Box) { + self.order.lock().unwrap().push("join"); // Bounded, and that is the whole point. The read loop ends when the // master reports EOF, which only happens once the reaped child has // released the slave -- so joining *before* termination blocks diff --git a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs index 58bf0a76e..17c9f413b 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/shared.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/shared.rs @@ -14,7 +14,7 @@ //! metered separately: pooling them would let the reader's millions of fast //! acquires dilute the renderer's tail into a false pass. -use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::time::Instant; use alacritty_terminal::sync::FairMutex; @@ -126,6 +126,7 @@ pub struct SharedTerminal { term: FairMutex, reader: AcquireMeter, renderer: AcquireMeter, + closing: AtomicBool, } impl SharedTerminal { @@ -134,6 +135,7 @@ impl SharedTerminal { term: FairMutex::new(term), reader: AcquireMeter::default(), renderer: AcquireMeter::default(), + closing: AtomicBool::new(false), } } @@ -156,13 +158,23 @@ impl SharedTerminal { /// the whole buffer under one acquisition is what an unbounded hold *is*, /// so it is not offered here. pub fn feed(&self, bytes: &[u8]) -> bool { - self.acquire(&self.reader).feed(bytes) + let mut term = self.acquire(&self.reader); + if self.closing.load(Ordering::Acquire) { + false + } else { + term.feed(bytes) + } } /// Parse more of the pending tail under a fresh acquisition. Reader /// plane. Returns whether any remains. pub fn drain(&self) -> bool { - self.acquire(&self.reader).drain() + let mut term = self.acquire(&self.reader); + if self.closing.load(Ordering::Acquire) { + false + } else { + term.drain() + } } /// Feed and pump to completion, re-acquiring between slices. @@ -173,6 +185,17 @@ impl SharedTerminal { } } + /// Atomically enter close mode and discard parser work. Subsequent PTY + /// bytes are raw-drained by the embedder and never reach callbacks. + pub fn begin_closing(&self) -> usize { + self.closing.store(true, Ordering::Release); + self.acquire(&self.reader).abandon_tail() + } + + pub fn is_closing(&self) -> bool { + self.closing.load(Ordering::Acquire) + } + /// Sample damage and encode a frame. Renderer plane. /// /// The lock covers the copy only; `encode` -- hashing, span grouping, @@ -236,3 +259,31 @@ impl SharedTerminal { guard } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Fences, Size}; + + #[test] + fn closing_abandons_tail_and_permanently_refuses_parser_callbacks() { + let (terminal, _actions) = Terminal::new(Size::default(), Fences::ALL); + let shared = SharedTerminal::new(terminal); + let payload = b"\x1b#8".repeat(10_000); + + assert!(shared.feed(&payload), "fixture must create parser tail"); + let before = shared.lock().stats(); + let abandoned = shared.begin_closing(); + assert!(abandoned > 0, "close must abandon without draining first"); + assert!(shared.is_closing()); + + assert!(!shared.feed(b"parser callback after close")); + assert!(!shared.drain()); + let after = shared.lock().stats(); + assert_eq!(after.completed_units, before.completed_units); + assert_eq!( + after.abandoned_bytes, + before.abandoned_bytes + abandoned as u64 + ); + } +} diff --git a/desktop/src-tauri/src/terminal_runtime.rs b/desktop/src-tauri/src/terminal_runtime.rs index dcf861e23..30d8df6d1 100644 --- a/desktop/src-tauri/src/terminal_runtime.rs +++ b/desktop/src-tauri/src/terminal_runtime.rs @@ -1,6 +1,7 @@ //! Rust-owned PTY sessions and the typed Tauri transport for Buzz Substrate. use std::io::{Read, Write}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::JoinHandle; @@ -202,10 +203,23 @@ fn wire_publication(publication: Publication) -> Result { }) } -struct ReaderThread(Option>); +struct ReaderThread { + handle: Option>, + terminal: Arc, + stopping: Arc, +} + impl buzz_terminal::lifecycle::DrainingReader for ReaderThread { + fn begin_closing(&self) { + self.terminal.begin_closing(); + } + + fn stop(&self) { + self.stopping.store(true, Ordering::Release); + } + fn join(mut self: Box) { - if let Some(handle) = self.0.take() { + if let Some(handle) = self.handle.take() { let _ = handle.join(); } } @@ -253,7 +267,12 @@ impl Session { if let Ok(mut channel) = self.channel.lock() { *channel = None; } - // The slave closes on child reap; the reader continues draining until then. + // Publication is detached before the reader enters close mode; the + // lifecycle helper then abandons parser work and keeps raw-draining + // through child termination and reap. + if let Ok(mut publisher) = self.publisher.lock() { + publisher.close(); + } if let Some(reader) = self.reader.take() { #[cfg(unix)] { @@ -261,12 +280,13 @@ impl Session { } #[cfg(not(unix))] { + reader.begin_closing(); let _ = self.child.kill(); let _ = self.child.wait(); + reader.stop(); reader.join(); } } - self.master.take(); } } @@ -465,6 +485,8 @@ pub(crate) fn terminal_attach( let reader_terminal = Arc::clone(&terminal); let reader_publisher = Arc::clone(&publisher); let reader_channel = Arc::clone(&channel); + let reader_stopping = Arc::new(AtomicBool::new(false)); + let thread_stopping = Arc::clone(&reader_stopping); let reader_handle = std::thread::spawn(move || { let mut buffer = [0u8; 16 * 1024]; let mut encoder = buzz_terminal::damage::Encoder::new(); @@ -474,7 +496,19 @@ pub(crate) fn terminal_attach( Ok(0) | Err(_) => break, Ok(count) => count, }; - reader_terminal.feed(&buffer[..count]); + if thread_stopping.load(Ordering::Acquire) { + break; + } + if reader_terminal.is_closing() { + continue; + } + let mut more = reader_terminal.feed(&buffer[..count]); + while more && !reader_terminal.is_closing() { + more = reader_terminal.drain(); + } + if reader_terminal.is_closing() { + continue; + } let needs_snapshot = reader_publisher .lock() .map(|publisher| publisher.requires_snapshot()) @@ -527,12 +561,16 @@ pub(crate) fn terminal_attach( .map_err(|_| "terminal snapshot rejected".to_string())?; let session = Session { id, - terminal, + terminal: Arc::clone(&terminal), master: Some(pair.master), writer, pty_size: current_pty_size, child, - reader: Some(Box::new(ReaderThread(Some(reader_handle)))), + reader: Some(Box::new(ReaderThread { + handle: Some(reader_handle), + terminal: Arc::clone(&terminal), + stopping: reader_stopping, + })), publisher, channel, }; diff --git a/desktop/src-tauri/src/terminal_transport.rs b/desktop/src-tauri/src/terminal_transport.rs index 68666aaa7..c00a1c006 100644 --- a/desktop/src-tauri/src/terminal_transport.rs +++ b/desktop/src-tauri/src/terminal_transport.rs @@ -186,6 +186,11 @@ impl FramePublisher { } } + /// Permanently detach publication as the session begins closing. + pub(crate) fn close(&mut self) { + self.subscription = None; + } + fn require_current_snapshot(&self, frame: &Frame) -> Result<(), OfferError> { if frame.viewport == self.applied && frame.full { Ok(()) From 1a2c86ebedb739701b172b061cae45a0ddcadf3a Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:26:26 -0400 Subject: [PATCH 06/15] fix(terminal): repair three defects the gate found in the work-bounded seam All three were pre-registered predictions that the fixtures at a957d25b0 did not cover, and each was independently reproduced before being fixed. **Geometry ownership.** `Terminal::resize` took a whole `Size` and forwarded two of its three fields to the feeder, so a scrollback change updated the `Term` while the feeder kept charging its construction depth. That is most of RIS's price -- 3,840 charged against 803,840 honest at 80x24 -- and because `slice_bytes` is derived from the densest atom, it also left the *scheduler* sized for the wrong grid: a stale 130-byte slice admits 65 RIS atoms where the honest 4-byte slice admits two. The split was permanent; later resizes tracked their own fields and never re-derived the third. `Feeder::resize` now takes the whole `Size`, so there is one ownership boundary and no way to update two of three. A bound derived at construction from a value that can change afterwards is a bound with an expiry date. **Post-atom overrun.** Slices were sized against the whole budget, so a slice could hold an oversized atom *and* the callbacks behind it: `ESC c` followed by `Xmore` ran three callbacks where the law permits one. Width is now a function of what *remains* of the budget, which is a single byte once an atom worth more than the budget is in play. Two refinements the fixtures forced: plain runs are metered by the plain-byte cost and stop at the next `ESC` rather than being priced as though every byte might be RIS -- pricing them that way is correct and costs 181 MB/s -> 69 MB/s for a bound on something that cannot happen -- and the escape scan is capped at `MAX_SLICE`, because searching the whole tail per slice is quadratic and measured as a 7x regression, a bound costing more than the thing it bounds. A `mid_escape` flag keeps a sequence split across slices on escape metering; without it the byte after a lone `ESC` looks like text and the atom rides into a wide slice. **Saturating arithmetic.** `columns * lines` was computed in `usize` before the cast, and `Size` is unclamped with no caller bounding it. Debug panicked inside the accounting path -- reachable from `drain`, which is the pump a paused reader must keep calling -- and release wrapped. The wrap is the worse half: it does not degrade the fence, it inverts it, because `slice_bytes` divides the budget by the atom cost, so an undercharged atom yields a *wider* slice exactly when the atom is most expensive. Convert first, saturate throughout. The fixtures are where most of the work went, because three of them had to be rewritten after the obvious version passed on the broken tree: * Resize is asserted by equality with a terminal *constructed* at the target depth, across charge and acquisition count -- stronger than any threshold and immune to the constants moving, since both arms move together. A sanity arm proves the comparison is deterministic first. * Shrinking is asserted *directionally* after establishing a debt (shallow -> deep -> shallow). Equality would forbid conservative retention, and resizing straight down leaves a feeder indistinguishable from one built shallow -- which is also the signature of a feeder that never updated. * `completed_units` is asserted but is explicitly *not* the discriminator: it reads the same in both arms, because the same callbacks run either way. * Saturation asserts the direction -- widest atom, narrowest slice -- plus monotonicity across nine decades. "Does not panic" is satisfied by the wrapping version, which is the version that inverts the fence. * Preconditions are asserted rather than assumed: the geometry must let the scheduling fields separate at all, and the payload must be RIS, the only escape reaching the only weight with a scrollback term. 24 mutants dead in debug and release, including one per repair. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/fences.rs | 63 ++- .../src-tauri/crates/buzz-terminal/src/lib.rs | 2 +- .../crates/buzz-terminal/src/reader.rs | 100 +++- .../crates/buzz-terminal/src/units.rs | 23 +- .../crates/buzz-terminal/tests/slicing.rs | 429 +++++++++++++++++- 5 files changed, 562 insertions(+), 55 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index ed0bc1a1d..7ee9f30e2 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -75,6 +75,45 @@ pub fn slice_bytes(columns: usize, lines: usize, scrollback: usize) -> usize { ((WORK_BUDGET / densest) as usize).clamp(MIN_SLICE, MAX_SLICE) } +/// Bytes to hand the parser when `spent` of the budget is already gone. +/// +/// The scheduling rule in one place so the fixtures can assert on it rather +/// than on a copy of the arithmetic: a slice of `N` bytes holds at most +/// `N / atom_bytes` atoms, so `remaining / densest` bytes cannot carry a +/// drain past the budget. +/// +/// `next_escape` is how far the next `ESC` is from the front of the tail. +/// This is the difference between a correct bound and an unusable one. Only +/// an escape can buy grid-sized work in two bytes; a run of ordinary +/// characters costs at most `columns` per byte (a wrapping line feed that +/// scrolls), which is four orders of magnitude cheaper than RIS. Pricing +/// plain text as though every byte might be RIS drops throughput from +/// 181 MB/s to 69 MB/s at the default scrollback -- measured -- while +/// bounding something that cannot happen. So a plain run is sliced against +/// the plain-byte cost and only the escape itself is metered against the +/// worst atom. +pub fn slice_bytes_remaining( + columns: usize, + lines: usize, + scrollback: usize, + spent: u64, + next_escape: usize, +) -> usize { + let remaining = WORK_BUDGET.saturating_sub(spent); + if next_escape > 0 { + // A plain run, and it stops at the escape: an escape sharing a slice + // with the text in front of it is how a callback runs *after* the one + // that crossed the budget, which is the overrun this bound exists to + // prevent. Worst case per plain byte is a line feed that scrolls, + // which resets one row: `columns`. + let per_byte = (columns as u64).max(1); + return ((remaining / per_byte) as usize).clamp(1, next_escape.min(MAX_SLICE)); + } + // An escape starts here. `ESC c` is the densest at two bytes. + let densest = (max_atom_work(columns, lines, scrollback) / 2).max(1); + ((remaining / densest) as usize).clamp(1, MAX_SLICE) +} + /// Work the single worst uninterruptible callback can cost on this grid. /// /// This is the irreducible overrun past [`WORK_BUDGET`]: no scheduler outside @@ -95,8 +134,17 @@ pub fn max_atom_work(columns: usize, lines: usize, scrollback: usize) -> u64 { // default 10k depth -- and it is genuinely indivisible, so it is stated // rather than smoothed. CBT, once terminated at its fixed point, is // `columns` and never competes. - let ris = 2 * (columns * lines) as u64 + (scrollback * columns) as u64; - ris.max(columns as u64) + // + // Saturating, and widened to u64 *before* multiplying. `Size` fields are + // unclamped `usize` with no caller bounding them, so the products here + // are reachable overflows: in debug that is a panic in the accounting + // path, and in release it wraps to a small number, which understates the + // bound -- an overflow that reports the parser as cheap is the worst of + // the three outcomes. + let (columns, lines, scrollback) = (columns as u64, lines as u64, scrollback as u64); + let both_grids = columns.saturating_mul(lines).saturating_mul(2); + let history = scrollback.saturating_mul(columns); + both_grids.saturating_add(history).max(columns) } /// Upper bound on the work a single [`crate::reader::Feeder::drain`] can do. @@ -115,12 +163,11 @@ pub fn max_atom_work(columns: usize, lines: usize, scrollback: usize) -> u64 { /// is why this is a function callers can read rather than an assumption /// they inherit. pub fn max_drain_work(columns: usize, lines: usize, scrollback: usize) -> u64 { - let atom = max_atom_work(columns, lines, scrollback); - // A slice of N bytes holds at most N/2 of the densest atoms, and the - // budget is only consulted between slices, so a whole slice is the - // overshoot. Where the slice derivation is unclamped this is one budget; - // where MIN_SLICE binds, it is this. - WORK_BUDGET + (slice_bytes(columns, lines, scrollback) as u64 / 2).max(1) * atom + // One atom, not one slice: [`crate::reader::Feeder::drain`] sizes every + // slice against the *remaining* budget, so it cannot start a slice able + // to hold more work than is left. What it cannot do is preempt a callback + // that has begun, which is where this term comes from. + WORK_BUDGET.saturating_add(max_atom_work(columns, lines, scrollback)) } /// Max bytes that may sit unparsed before the reader must stop reading the diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs index 9a26816e2..5959d79d9 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs @@ -221,7 +221,7 @@ impl Terminal { return self.viewport(); } self.term.resize(size); - self.feeder.resize(size.columns, size.screen_lines); + self.feeder.resize(size); self.size = size; self.generation += 1; self.viewport() diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index dd4cf4e2d..90ee8aded 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -7,8 +7,8 @@ use alacritty_terminal::vte::ansi::{Handler, Processor, StdSyncHandler}; use crate::fences::{ - slice_bytes, FenceStats, Fences, MAX_SLICE, OSC_BUDGET, SYNC_CAP, TAIL_CAP, TAIL_RESUME, - WORK_BUDGET, + slice_bytes_remaining, FenceStats, Fences, MAX_SLICE, OSC_BUDGET, SYNC_CAP, TAIL_CAP, + TAIL_RESUME, WORK_BUDGET, }; use crate::units::{Counting, CursorColumn}; @@ -33,8 +33,13 @@ pub struct Feeder { /// callback for as long as it is wrong. columns: usize, lines: usize, - /// Configured scrollback depth. Fixed for the life of the feeder: it is a - /// config value, not grid state, and `resize` does not change it. + /// Whether the parser is part-way through an escape sequence that has not + /// yet dispatched. Governs how the next slice is metered -- see + /// [`crate::fences::slice_bytes_remaining`]. + mid_escape: bool, + /// Configured scrollback depth. Updated by [`Feeder::resize`] with the + /// rest of the geometry: it is most of RIS's charge, so a stale value + /// misprices the densest atom and the slice derived from it. scrollback: usize, } @@ -47,16 +52,26 @@ impl Feeder { since_reset: 0, pending: Vec::new(), pending_at: 0, + mid_escape: false, columns, lines, scrollback, } } - /// Track a viewport change, so the cost weights describe the current grid. - pub fn resize(&mut self, columns: usize, lines: usize) { - self.columns = columns; - self.lines = lines; + /// Track a geometry change, so the cost weights describe the current grid. + /// + /// Takes the whole [`crate::Size`] rather than a column/line pair on + /// purpose. Scrollback is as load-bearing as the other two -- it is most + /// of RIS's price and therefore most of the slice derivation -- and a + /// signature that accepted only the dimensions let a caller change the + /// depth on the `Term` while the feeder kept charging the construction + /// value. One argument, one ownership boundary, no way to update two of + /// three. + pub fn resize(&mut self, size: crate::Size) { + self.columns = size.columns; + self.lines = size.screen_lines; + self.scrollback = size.scrollback; } pub fn stats(&self) -> FenceStats { @@ -158,14 +173,60 @@ impl Feeder { // self. A stack buffer keeps that from allocating; the copy is a // memcpy against a parse two orders of magnitude more expensive. let mut buf = [0u8; MAX_SLICE]; - let width = slice_bytes(self.columns, self.lines, self.scrollback); - let mut spent = 0; + let mut spent: u64 = 0; while self.pending_at < self.pending.len() { + // Size each slice against what is *left* of the budget, and + // against what is actually in front of the parser. A slice can + // only be as expensive as the callbacks it contains, and only an + // escape can buy grid-sized work in two bytes -- so a plain run + // is sliced against the plain-byte cost and stops at the next + // `ESC`, which then gets a slice metered against the worst atom. + // The drain therefore returns on the atom that crosses the + // budget, not at the end of a slice that ran several more. + // + // Where one atom is worth more than the entire budget -- RIS at + // any real scrollback depth -- that escape gets a one-byte slice. + // That is the honest consequence of the law: nothing wider can + // promise to stop after the crossing atom when a single atom + // always crosses. + // A slice is never wider than MAX_SLICE, so the scan for the next + // escape stops there too: searching the whole tail would be + // O(tail) per slice and O(tail^2) per drain, which measured as a + // 7x throughput *regression* on plain text -- a bound that costs + // more than the thing it bounds. + let horizon = (self.pending_at + MAX_SLICE).min(self.pending.len()); + let next_escape = if self.mid_escape { + // Already inside a sequence whose callback has not fired. Its + // remaining bytes are *not* plain text -- `ESC` then `c` is a + // grid reset -- so they keep the escape's metering. Without + // this the byte after a lone `ESC` is priced as a character + // and the atom rides into a wide slice with whatever follows + // it, which is the post-atom overrun by another door. + 0 + } else { + self.pending[self.pending_at..horizon] + .iter() + .position(|&b| b == 0x1b) + .unwrap_or(horizon - self.pending_at) + }; + let width = slice_bytes_remaining( + self.columns, + self.lines, + self.scrollback, + spent, + next_escape, + ); let end = (self.pending_at + width).min(self.pending.len()); let len = end - self.pending_at; buf[..len].copy_from_slice(&self.pending[self.pending_at..end]); self.pending_at = end; - spent += self.advance_slice(handler, &buf[..len]); + let cost = self.advance_slice(handler, &buf[..len]); + // A slice that contained an escape but dispatched nothing left the + // parser mid-sequence. Work is the signal because it is the thing + // being budgeted: a sequence that has not yet cost anything has + // not yet run. + self.mid_escape = (self.mid_escape || buf[..len].contains(&0x1b)) && cost == 0; + spent = spent.saturating_add(cost); if spent >= WORK_BUDGET { break; } @@ -184,14 +245,15 @@ impl Feeder { /// Parse one slice, applying both fences to it. Returns the work it cost. fn advance_slice(&mut self, handler: &mut H, bytes: &[u8]) -> u64 { - let mut spent = 0; + let mut spent: u64 = 0; let sync_before = self.parser.sync_bytes_count(); { let mut counting = Counting::new(handler, self.columns, self.lines, self.scrollback); self.parser.advance(&mut counting, bytes); - self.stats.completed_units += counting.units(); - self.stats.completed_work += counting.work(); - spent += counting.work(); + self.stats.completed_units = + self.stats.completed_units.saturating_add(counting.units()); + self.stats.completed_work = self.stats.completed_work.saturating_add(counting.work()); + spent = spent.saturating_add(counting.work()); } let sync_after = self.parser.sync_bytes_count(); @@ -229,9 +291,11 @@ impl Feeder { let mut counting = Counting::new(handler, self.columns, self.lines, self.scrollback); self.parser.stop_sync(&mut counting); - self.stats.completed_units += counting.units(); - self.stats.completed_work += counting.work(); - spent += counting.work(); + self.stats.completed_units = + self.stats.completed_units.saturating_add(counting.units()); + self.stats.completed_work = + self.stats.completed_work.saturating_add(counting.work()); + spent = spent.saturating_add(counting.work()); } self.stats.sync_aborts += 1; self.note_release(released); diff --git a/desktop/src-tauri/crates/buzz-terminal/src/units.rs b/desktop/src-tauri/crates/buzz-terminal/src/units.rs index b30533a65..7d2a9f032 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/units.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/units.rs @@ -113,9 +113,12 @@ impl<'a, H: Handler + CursorColumn> Counting<'a, H> { self.work } + /// Cells in the grid. Saturating: `Size` is unclamped `usize`, so this + /// product is reachable, and a wrapped weight prices the most expensive + /// callbacks as the cheapest. #[inline] fn cells(&self) -> u64 { - self.columns * self.lines + self.columns.saturating_mul(self.lines) } /// A parameter charged at its clamped value. @@ -132,8 +135,8 @@ impl<'a, H: Handler + CursorColumn> Counting<'a, H> { #[inline] fn charge(&mut self, weight: u64) { - self.units += 1; - self.work += weight; + self.units = self.units.saturating_add(1); + self.work = self.work.saturating_add(weight); } } @@ -216,7 +219,8 @@ macro_rules! counting_handler { // charging that one zero would leave a loop that spins // without ever paying, which is precisely the mutant this // pricing has to make visible. - self.work += if after == before { before } else { before - after } as u64; + let scanned = if after == before { before } else { before - after }; + self.work = self.work.saturating_add(scanned as u64); if after == before { // A fixed point is permanent: the scan depends only // on the cursor, and the cursor did not move. @@ -340,10 +344,10 @@ counting_handler! { // recorded here instead of being priced into every scroll -- charging // 1000x on 999 calls out of 1000 to cover the thousandth would make // ordinary scrolling the slow path. - scroll_up(n: usize) => |this| this.clamp(n, this.lines) * this.columns; - delete_lines(n: usize) => |this| this.clamp(n, this.lines) * this.columns; - scroll_down(n: usize) => |this| this.clamp(n, this.lines) * this.columns; - insert_blank_lines(n: usize) => |this| this.clamp(n, this.lines) * this.columns; + scroll_up(n: usize) => |this| this.clamp(n, this.lines).saturating_mul(this.columns); + delete_lines(n: usize) => |this| this.clamp(n, this.lines).saturating_mul(this.columns); + scroll_down(n: usize) => |this| this.clamp(n, this.lines).saturating_mul(this.columns); + insert_blank_lines(n: usize) => |this| this.clamp(n, this.lines).saturating_mul(this.columns); // O(columns): one row, `damage_line(line, 0, columns - 1)`. clear_line(mode: LineClearMode) => |this| this.columns; // O(cells): 18.7us at 200x50, doubling on both axes. @@ -372,6 +376,7 @@ counting_handler! { // on its own -- 16x at the default 10k scrollback -- which is correct: // it is a genuinely oversized uninterruptible atom, and a budget that // hid that would be lying about what one drain can cost. - reset_state() => |this| 2 * this.cells() + this.scrollback * this.columns; + reset_state() => |this| this.cells().saturating_mul(2) + .saturating_add(this.scrollback.saturating_mul(this.columns)); } } diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index d597ece7e..d473a5ebb 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -10,8 +10,8 @@ //! `> 0` is satisfied by a seam that executed exactly one unit. use buzz_terminal::fences::{ - max_atom_work, max_drain_work, slice_bytes, Fences, MAX_SLICE, MIN_SLICE, SYNC_CAP, TAIL_CAP, - WORK_BUDGET, + max_atom_work, max_drain_work, slice_bytes, slice_bytes_remaining, Fences, MAX_SLICE, + MIN_SLICE, SYNC_CAP, TAIL_CAP, WORK_BUDGET, }; use buzz_terminal::{Size, Terminal}; @@ -535,16 +535,21 @@ fn every_amplifiable_escape_is_priced_exactly() { /// RIS is priced with its history axis, not just its cells. /// -/// Kills: charging `cells`, or charging the *active* grid's history. RIS -/// resets both grids and walks the primary's scrollback, and `history_size()` -/// observes only the active grid -- so a filled primary followed by -/// `ESC[?1049h` reads as empty while the work is still paid. Configured depth -/// is the only stateless quantity that bounds both, and this asserts the -/// charge tracks it. +/// Kills: charging `cells`, or dropping the history term. +/// +/// On the alt-screen arm, honestly labelled: the active-`history_size()` +/// mispricing it was written against is **unrepresentable in this design**, +/// not merely untested. `Counting` holds `scrollback` as a scalar copied at +/// construction and has no path to a live grid, so there is no way to write +/// the mutant. The arm is kept as a regression witness -- if a `Term` +/// reference is ever wired into the wrapper it becomes load-bearing the same +/// day -- and both arms are evaluated before either can report, so the +/// primary cannot short-circuit the alt. #[test] fn ris_is_priced_for_both_grids_and_the_scrollback_it_walks() { let (columns, lines) = (80usize, 24usize); let cells = (columns * lines) as u64; + let mut observed = vec![]; for scrollback in [0usize, 100, 10_000] { for (label, prefix) in [("primary", ""), ("alt screen", "\u{1b}[?1049h")] { let (mut term, _a) = Terminal::new( @@ -558,15 +563,17 @@ fn ris_is_priced_for_both_grids_and_the_scrollback_it_walks() { term.feed_fully(prefix.as_bytes()); term.reset_stats(); term.feed_fully(b"\x1bc"); - - assert_eq!( - term.stats().completed_work, - 2 * cells + (scrollback * columns) as u64, - "{label}, scrollback {scrollback}: RIS must be priced on \ - configured depth, which is the same on both grids", - ); + observed.push((label, scrollback, term.stats().completed_work)); } } + for (label, scrollback, work) in observed { + assert_eq!( + work, + 2 * cells + (scrollback * columns) as u64, + "{label}, scrollback {scrollback}: RIS must be priced on \ + configured depth, which is the same on both grids", + ); + } } /// CBT is charged for exactly the cells it scans -- an equality, in both @@ -708,11 +715,15 @@ fn the_worst_atom_is_bounded_under_the_layout_that_maximises_steps() { term.feed_fully(b"\x1b[65535Z"); - let spent = term.stats().completed_work; assert_eq!(term.stats().completed_units, 1); - assert!( - spent <= 2 * columns as u64, - "all-set CBT at {columns} columns charged {spent}, over 2 x columns", + assert_eq!( + term.stats().completed_work, + 1 + (columns as u64 - 1), + "with a stop in every column the walk crosses each of them once, \ + so the charge is exact: one unit for the escape plus one per \ + column crossed. An inequality here would not catch a 2x \ + overcharge -- which lands on 159, not 160, because the escape's \ + own unit is charged separately and is not doubled", ); assert_eq!( term.term().grid().cursor.point.column.0, @@ -758,3 +769,383 @@ fn stopping_the_worst_atom_early_preserves_its_semantics() { // Default stops every 8: a large count walks all the way to column 0. assert_eq!(cursor_column("\u{1b}[1;40H\u{1b}[65535Z"), 0); } + +/// A stream of atoms each worth more than the whole budget still drains, and +/// every drain makes progress. +/// +/// The liveness half of the bound. `max_drain_work` says how much one drain +/// may cost; it says nothing about whether the loop terminates, and an +/// oversized atom is exactly where a work-denominated scheduler could refuse +/// to start one -- spending its budget checking, never advancing, and hanging +/// the terminal with a full tail. RIS on a 10k-scrollback grid is ~16x the +/// budget, so this is not hypothetical. +/// +/// Kills: any yield that can decline to start work -- a `width` that reaches +/// 0, a `remaining`-scaled slice that underflows to nothing, a guard that +/// skips a slice deemed too expensive for what is left of the budget. Each of +/// those is a plausible thing to reach for when an atom costs more than the +/// whole budget, and each hangs a terminal on legitimate input. +/// +/// Note on a mutant it does *not* kill: moving the budget check from after +/// the slice to before it is **equivalent**, not a defect -- `spent` is zero +/// at entry, so the first slice runs either way. Recorded because I wrote +/// this test believing it caught that, ran the mutant, and it lived. +#[test] +fn atoms_larger_than_the_budget_still_make_progress() { + for (columns, lines, scrollback) in [(80usize, 24usize, 10_000usize), (200, 50, 10_000)] { + let (mut term, _a) = Terminal::new( + Size { + columns, + screen_lines: lines, + scrollback, + }, + Fences::ALL, + ); + let atoms = 200usize; + let bound = max_drain_work(columns, lines, scrollback); + assert!( + bound > WORK_BUDGET * 4, + "this arm is only meaningful where one atom dwarfs the budget", + ); + + let mut more = term.feed(&b"\x1bc".repeat(atoms)); + // `feed` already drained once; seed the baseline with its work or the + // first delta measured below silently doubles. + let mut previous = term.stats().completed_work; + let mut worst = previous; + let mut calls = 1; + while more { + let before = term.pending_bytes(); + more = term.drain(); + assert!( + term.pending_bytes() < before, + "no progress: the tail stuck at {before} bytes", + ); + let now = term.stats().completed_work; + worst = worst.max(now - previous); + previous = now; + calls += 1; + assert!(calls < 10_000, "drain did not terminate"); + } + + assert_eq!(term.stats().completed_units, atoms as u64, "lost units"); + assert_eq!(term.pending_bytes(), 0); + assert!( + worst <= bound, + "{columns}x{lines}: worst drain spent {worst}, over the stated \ + bound {bound}", + ); + } +} + +/// A scrollback change reprices RIS *and* the slicing derived from it. +/// +/// Kills: updating the feeder's columns and lines on resize but not its +/// scrollback -- and, separately, a repair that reprices the charge while +/// leaving slice width stale. Those are different failures and neither +/// observable sees the other: fix only the charge and the drain count stays +/// wrong; fix only the derivation and the charge stays wrong. +/// +/// Two properties, because one is not enough: +/// +/// * The exact RIS charge at the new depth. Direct, and it is what a +/// pricing-only repair passes. +/// * Equality with a terminal *constructed* at the new depth, across work +/// and drain count. A resized feeder that is genuinely repaired is +/// indistinguishable from one that was born there. This is stronger than a +/// hand-picked threshold and immune to `WORK_BUDGET`/`MIN_SLICE` moving, +/// since both arms move together -- and the sanity arm proves the +/// comparison is deterministic before it is used to judge anything. +/// +/// `completed_units` is deliberately *not* the discriminator here: it reads +/// 200 in both arms, because the same callbacks run either way and only their +/// cost and slicing differ. It is asserted anyway as the invariant that must +/// hold -- no unit lost or duplicated across a resize -- while carrying none +/// of the discrimination. +#[test] +fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { + let shallow = Size { + columns: 200, + screen_lines: 50, + scrollback: 100, + }; + let deep = Size { + scrollback: 10_000, + ..shallow + }; + let cells = (deep.columns * deep.screen_lines) as u64; + + // Preconditions, asserted rather than assumed, because both are easy to + // break by "generalising" this fixture later: + // + // * The geometry must let the *scheduling* fields separate. They only do + // when the two depths land on different slice widths, and the deep side + // is always floored -- so the shallow side must not be. At 1600x50 the + // visible grid alone floors every depth from 0 upward, and three of the + // four observables below go silently inert. + // * The payload must be RIS. It is the only escape reaching the only + // weight carrying a scrollback term (`units::reset_state`); DECALN and + // every other atom are priced on cells or columns and are blind to + // depth, so a conforming repair would show work identical to the + // control and the assertions here would invert into false failures. + assert!( + slice_bytes(shallow.columns, shallow.screen_lines, shallow.scrollback) > MIN_SLICE, + "geometry cannot discriminate: the shallow arm is already floored", + ); + assert_eq!( + slice_bytes(deep.columns, deep.screen_lines, deep.scrollback), + MIN_SLICE, + ); + + // How a terminal at `size` retires 200 RIS: work, and how many + // acquisitions it took. Both are feeder behaviour, not helper output. + let run = |size: Size, resize_from: Option| { + let (mut term, _a) = Terminal::new(resize_from.unwrap_or(size), Fences::ALL); + if resize_from.is_some() { + term.resize(size); + } + term.reset_stats(); + let mut drains = 1; + let mut more = term.feed(&b"c".repeat(200)); + while more { + more = term.drain(); + drains += 1; + } + ( + term.stats().completed_units, + term.stats().completed_work, + drains, + ) + }; + + let control = run(deep, None); + let sanity = run(deep, None); + assert_eq!( + control, sanity, + "two terminals built the same way must agree before this comparison can judge anything", + ); + + let resized = run(deep, Some(shallow)); + assert_eq!(resized.0, 200, "no unit may be lost or duplicated"); + assert_eq!( + resized, control, + "a feeder resized to a depth must be indistinguishable from one constructed at it -- in charge and in how many acquisitions it took", + ); + + // The exact charge, stated rather than inferred from the equality: a + // repair that made both arms equally *wrong* would pass the comparison. + let (mut term, _a) = Terminal::new(shallow, Fences::ALL); + term.resize(deep); + term.reset_stats(); + term.feed_fully(b"c"); + assert_eq!( + term.stats().completed_work, + 2 * cells + (deep.scrollback * deep.columns) as u64, + ); + + // Shrinking is asserted *directionally*, never by equality. A decrease + // leaves the feeder charging the old deep price -- a 50x over-charge, + // which is the safe direction -- and demanding equality with a shallow + // control would forbid that conservative behaviour and force an exact + // new-depth claim this seam does not make. Red for an over-charge and red + // for an under-charge look identical to an equality; they are opposites + // to the product. + // Establish a debt first: shallow -> deep -> shallow. Resizing straight + // down from the constructor value would leave a feeder indistinguishable + // from one built shallow, which is also the signature of a feeder that + // never updated at all -- so that arm cannot tell a repair from the bug. + let (mut shrunk_term, _a) = Terminal::new(shallow, Fences::ALL); + shrunk_term.resize(deep); + shrunk_term.resize(shallow); + shrunk_term.reset_stats(); + shrunk_term.feed_fully(b"\x1bc"); + let shrunk_work = shrunk_term.stats().completed_work; + + let (mut fresh, _a) = Terminal::new(shallow, Fences::ALL); + fresh.reset_stats(); + fresh.feed_fully(b"\x1bc"); + + // Directional, never equality. Shrinking may leave the feeder pricing at + // the depth it once had -- an over-charge, the safe direction -- and + // demanding equality with a shallow control would forbid that and force + // an exactness claim this seam does not make. The two failures look + // identical to an equality and are opposites to the product. + assert!( + shrunk_work >= fresh.stats().completed_work, + "a shrunk feeder may be conservative but never cheaper than one \ + built shallow: {shrunk_work} against {}", + fresh.stats().completed_work, + ); + + // A later resize on a different axis must not disturb the third one -- + // the split was permanent, with columns and lines tracking correctly + // while a stale depth persisted forever. + term.resize(Size { + columns: deep.columns * 2, + ..deep + }); + term.reset_stats(); + term.feed_fully(b"c"); + assert_eq!( + term.stats().completed_work, + 2 * (deep.columns * 2 * deep.screen_lines) as u64 + + (deep.scrollback * deep.columns * 2) as u64, + "a columns resize must keep the scrollback it was already given", + ); +} + +/// One oversized atom per drain -- no callback runs after the one that +/// crosses the budget. +/// +/// Kills: sizing slices from the *whole* budget rather than what remains of +/// it. RIS at any real scrollback depth is worth more than an entire budget, +/// so a slice wide enough for several callbacks runs several: measured +/// `completed_units == 3` for `ESC c` followed by `Xmore`, where the law +/// permits exactly one. The fix makes slice width a function of `remaining`, +/// which is a single byte once an atom this size is in play. +/// +/// Also asserts the tail survives it: yielding after the crossing atom is +/// only correct if what follows is still parsed, exactly once. +#[test] +fn an_oversized_atom_yields_before_the_next_callback() { + let size = Size { + columns: 400, + screen_lines: 100, + scrollback: 10_000, + }; + let (mut term, _a) = Terminal::new(size, Fences::ALL); + let ris_work = + 2 * (size.columns * size.screen_lines) as u64 + (size.scrollback * size.columns) as u64; + assert!( + ris_work > WORK_BUDGET, + "this arm needs an atom bigger than the whole budget", + ); + + let more = term.feed(b"\x1bcXmore"); + + assert!(more, "the drain must yield with a tail"); + assert_eq!( + term.stats().completed_units, + 1, + "exactly the crossing atom ran: a callback after it is post-atom \ + overrun, which is the thing the budget cannot preempt and therefore \ + must not start", + ); + assert_eq!(term.stats().completed_work, ris_work); + + while term.drain() {} + assert_eq!( + term.stats().completed_units, + 1 + 5, + "the five characters after it must still be parsed, exactly once", + ); + assert_eq!(term.pending_bytes(), 0); +} + +/// Extreme dimensions saturate rather than wrapping or panicking. +/// +/// Kills: `columns * lines` in `usize` before the cast. `Size` is unclamped +/// and reaches the weight path from a caller, so this product is a reachable +/// overflow -- a debug panic inside the accounting path, or a release wrap +/// that reports the most expensive callback in the emulator as one of the +/// cheapest. Saturating is the only one of the three that fails safe. +#[test] +fn extreme_dimensions_saturate_instead_of_wrapping() { + let huge = usize::MAX / 2; + assert_eq!(max_atom_work(huge, huge, huge), u64::MAX); + assert_eq!(max_drain_work(huge, huge, huge), u64::MAX); + + // The *direction* is the assertion, not merely the absence of a panic. + // A wrapping build does not produce a slightly-wrong bound, it produces a + // tiny one -- and `slice_bytes` divides the budget by it, so an + // undercharged atom yields an *oversized* slice exactly when the atom is + // most expensive. Wrapping inverts the fence. So: the widest possible + // atom must give the narrowest possible slice. + assert_eq!( + slice_bytes(huge, huge, huge), + MIN_SLICE, + "an overflowing grid must clamp to the smallest slice; a wrapped \ + `max_atom_work` would hand back a generous one", + ); + assert_eq!( + slice_bytes_remaining(huge, huge, huge, 0, 0), + 1, + "and the escape at the front of such a grid gets a single byte", + ); + + // The property behind those endpoints, and the stronger statement: a + // grid that costs more may never buy a wider slice. Endpoints pin the + // ends; only a sweep catches a non-monotone middle, and a wrap *is* a + // non-monotone middle -- it makes the worst grid look cheap and hands it + // the widest slice of all. + let mut previous = usize::MAX; + for exponent in 0..60 { + let scrollback = 1usize << exponent; + let width = slice_bytes(200, 50, scrollback); + assert!( + width <= previous, + "slice widened from {previous} to {width} at scrollback \ + 2^{exponent}: more expensive grid, more generous slice", + ); + assert!(width >= MIN_SLICE); + previous = width; + } + + // Just past 32 bits on one axis: large enough that a narrowing cast + // shows (`1 << 32` truncates to 0 in `u32`, pricing an enormous grid at + // nothing), small enough that the honest answer is exact rather than + // saturated. Neither the extreme endpoints above nor the ordinary grids + // below can see this -- the endpoints saturate either way and the + // ordinary ones fit in 32 bits. + assert_eq!(max_atom_work(1 << 32, 1, 0), 2 * (1u64 << 32)); + assert_eq!(max_atom_work(1, 1 << 32, 0), 2 * (1u64 << 32)); + assert_eq!(max_atom_work(1, 1, 1 << 32), 2 + (1u64 << 32)); + + // Ordinary grids are untouched by the saturation: exact, not clamped. + assert_eq!(max_atom_work(80, 24, 0), 2 * 80 * 24); + assert_eq!(max_atom_work(80, 24, 100), 2 * 80 * 24 + 100 * 80); +} + +/// An escape split across slices keeps its escape metering. +/// +/// Kills: deciding "plain run or escape?" by looking only at the bytes ahead. +/// After a slice ending on a lone `ESC`, the next byte is `c` -- which looks +/// like ordinary text and is in fact a full grid reset. Meter it as text and +/// the oversized atom rides into a wide slice with whatever follows, which is +/// the post-atom overrun arriving through a different door. Found by the +/// oversized-atom fixture failing after I "optimised" the plain path, which +/// is the argument for keeping both. +#[test] +fn an_escape_split_across_slices_keeps_its_metering() { + let size = Size { + columns: 400, + screen_lines: 100, + scrollback: 10_000, + }; + let ris_work = + 2 * (size.columns * size.screen_lines) as u64 + (size.scrollback * size.columns) as u64; + + // Deliver the escape one byte at a time, so the parser is left mid- + // sequence with a tail that begins on the continuation byte. + let (mut term, _a) = Terminal::new(size, Fences::ALL); + term.feed(b"\x1b"); + assert_eq!( + term.stats().completed_units, + 0, + "ESC alone dispatches nothing" + ); + + let more = term.feed(b"cXmore"); + + assert!(more, "the completed RIS must still yield with a tail"); + assert_eq!( + term.stats().completed_units, + 1, + "the continuation byte completed a grid reset; nothing may run after it", + ); + assert_eq!(term.stats().completed_work, ris_work); + + while term.drain() {} + assert_eq!(term.stats().completed_units, 1 + 5); + assert_eq!(term.pending_bytes(), 0); +} From e1500806d3046107e18b77a7a038d2f444499772 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:30:26 -0400 Subject: [PATCH 07/15] fix(terminal): retain the scrollback debt a shrink does not immediately repay The previous commit lowered the feeder's scrollback on a decrease, which reads as correct and is not: upstream frees history lazily. `Storage::shrink_lines` truncates only once the buffer exceeds the new length by `MAX_CACHE_SIZE`, so immediately after a shrink the grid still owns rows a reset has to walk. Pricing at the new depth charges for a grid that does not exist yet. So the feeder tracks a high-water mark instead of the current depth. It never falls, needs no clearing transition, and cannot go stale in the unsafe direction. The cost is real and stated: a session that shrinks its scrollback keeps paying the deep price for the rest of its life. The alternative is a bound that is wrong immediately after every shrink. The fixture that permitted this is the more important half. It asserted `shrunk_work >= fresh_shallow`, which a feeder that dropped the debt satisfies *by equality* -- "never had a debt" and "dropped the debt" are the same state, so any predicate anchored to a fresh control blesses both. Strictness on the pricing field is what separates them, and the pricing field is the only one that can: the scheduling observables separate only when the two depths land on different slice widths, and both are usually floored. The arm now asserts `completed_work > fresh_shallow` strictly, with the scheduling fields carrying their per-field directions -- `first_units` inverts, because a narrower slice retires fewer atoms per un-preemptable drain, which is the fence working. Also here, both from the same gate: * Monotonicity is swept per axis. A truncating `columns * lines` is a non-monotone middle on the columns axis alone, and a scrollback-only sweep cannot see it. * Seventeen more rows in the pricing table -- `scroll_down`, `insert_lines`, `put_tab`, forward tabs, both `clear_line` modes, both `clear_screen` modes, and the parameter extremes for each. The four that were missing are the four whose weights share a shape with ones already covered, which is exactly the argument for covering them separately rather than by analogy. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/reader.rs | 20 ++- .../crates/buzz-terminal/tests/slicing.rs | 140 ++++++++++++------ 2 files changed, 114 insertions(+), 46 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 90ee8aded..82324f460 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -37,9 +37,19 @@ pub struct Feeder { /// yet dispatched. Governs how the next slice is metered -- see /// [`crate::fences::slice_bytes_remaining`]. mid_escape: bool, - /// Configured scrollback depth. Updated by [`Feeder::resize`] with the - /// rest of the geometry: it is most of RIS's charge, so a stale value - /// misprices the densest atom and the slice derived from it. + /// Deepest scrollback this feeder has ever been configured for. + /// + /// A high-water mark rather than the current depth, and the difference is + /// not conservatism for its own sake -- the rows are still there. Upstream + /// frees history lazily: `Storage::shrink_lines` truncates only once the + /// buffer exceeds the new length by `MAX_CACHE_SIZE`, so immediately after + /// a decrease the grid still owns rows that a reset must walk. Pricing at + /// the new depth would charge for a grid that does not exist yet. + /// + /// Never lowered, so it needs no clearing transition and cannot go stale + /// in the unsafe direction. The cost is that a session which shrinks its + /// scrollback keeps paying the deep price for the rest of its life; the + /// alternative is a bound that is wrong immediately after every shrink. scrollback: usize, } @@ -71,7 +81,9 @@ impl Feeder { pub fn resize(&mut self, size: crate::Size) { self.columns = size.columns; self.lines = size.screen_lines; - self.scrollback = size.scrollback; + // Grows only. See the field: a decrease does not immediately free the + // rows a reset has to walk. + self.scrollback = self.scrollback.max(size.scrollback); } pub fn stats(&self) -> FenceStats { diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index d473a5ebb..3007e23ee 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -505,7 +505,34 @@ fn every_amplifiable_escape_is_priced_exactly() { ("scroll_up N=1", "\u{1b}[1S".into(), c), ("scroll_up N=5", "\u{1b}[5S".into(), 5 * c), ("scroll_up N=huge", "\u{1b}[65535S".into(), lines as u64 * c), + ("scroll_down N=1", "\u{1b}[1T".into(), c), + ("scroll_down N=4", "\u{1b}[4T".into(), 4 * c), + ( + "scroll_down N=huge", + "\u{1b}[65535T".into(), + lines as u64 * c, + ), ("delete_lines N=3", "\u{1b}[3M".into(), 3 * c), + ( + "delete_lines N=huge", + "\u{1b}[65535M".into(), + lines as u64 * c, + ), + ("insert_lines N=1", "\u{1b}[1L".into(), c), + ("insert_lines N=6", "\u{1b}[6L".into(), 6 * c), + ( + "insert_lines N=huge", + "\u{1b}[65535L".into(), + lines as u64 * c, + ), + ("put_tab N=1", "\t".into(), c), + ("fwd_tabs N=1", "\u{1b}[1I".into(), c), + ("fwd_tabs N=huge", "\u{1b}[65535I".into(), c), + ("insert_blank N=huge", "\u{1b}[65535@".into(), c), + ("clear_line ESC[0K", "\u{1b}[0K".into(), c), + ("clear_line ESC[1K", "\u{1b}[1K".into(), c), + ("clear_screen ESC[0J", "\u{1b}[0J".into(), cells), + ("clear_screen ESC[1J", "\u{1b}[1J".into(), cells), ("sgr", "\u{1b}[m".into(), 1), ("goto", "\u{1b}[1;1H".into(), 1), ]; @@ -943,41 +970,59 @@ fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { 2 * cells + (deep.scrollback * deep.columns) as u64, ); - // Shrinking is asserted *directionally*, never by equality. A decrease - // leaves the feeder charging the old deep price -- a 50x over-charge, - // which is the safe direction -- and demanding equality with a shallow - // control would forbid that conservative behaviour and force an exact - // new-depth claim this seam does not make. Red for an over-charge and red - // for an under-charge look identical to an equality; they are opposites - // to the product. - // Establish a debt first: shallow -> deep -> shallow. Resizing straight - // down from the constructor value would leave a feeder indistinguishable - // from one built shallow, which is also the signature of a feeder that - // never updated at all -- so that arm cannot tell a repair from the bug. - let (mut shrunk_term, _a) = Terminal::new(shallow, Fences::ALL); - shrunk_term.resize(deep); - shrunk_term.resize(shallow); - shrunk_term.reset_stats(); - shrunk_term.feed_fully(b"\x1bc"); - let shrunk_work = shrunk_term.stats().completed_work; + // Shrinking retains the debt, and the fixture proves retention rather + // than merely permitting it. + // + // `>= fresh` alone is the predicate three of us proposed and all three + // withdrew: a feeder that dropped the debt reads *exactly* equal to a + // fresh shallow one, so `>=` passes on the unrepaired state. Strictness + // on the pricing field is what rejects it. The scheduling fields are + // asserted directionally with per-field signs -- `first_units` inverts, + // because a narrower slice retires fewer atoms per un-preemptable drain, + // which is the fence working -- but none of them is the discriminator: + // they separate only when the two depths straddle the slice floor, and + // `completed_work` separates at every positive depth gap. + let debt = |from: Size, to: Size| { + let (mut term, _a) = Terminal::new(from, Fences::ALL); + term.resize(deep); + term.resize(to); + term.reset_stats(); + let mut drains = 1; + let mut more = term.feed(&b"\x1bc".repeat(200)); + let first_units = term.stats().completed_units; + let first_pending = term.pending_bytes(); + while more { + more = term.drain(); + drains += 1; + } + ( + first_units, + first_pending, + drains, + term.stats().completed_units, + term.stats().completed_work, + ) + }; + let shrunk = debt(shallow, shallow); + let fresh = run(shallow, None); - let (mut fresh, _a) = Terminal::new(shallow, Fences::ALL); - fresh.reset_stats(); - fresh.feed_fully(b"\x1bc"); - - // Directional, never equality. Shrinking may leave the feeder pricing at - // the depth it once had -- an over-charge, the safe direction -- and - // demanding equality with a shallow control would forbid that and force - // an exactness claim this seam does not make. The two failures look - // identical to an equality and are opposites to the product. + assert_eq!(shrunk.3, 200, "no unit may be lost on the way down either"); assert!( - shrunk_work >= fresh.stats().completed_work, - "a shrunk feeder may be conservative but never cheaper than one \ - built shallow: {shrunk_work} against {}", - fresh.stats().completed_work, + shrunk.4 > fresh.1, + "a feeder that has been deep must still price deep after shrinking: \ + {} against a fresh shallow {}. Equality here is the signature of a \ + feeder that dropped the debt, which is indistinguishable from one \ + that never had it", + shrunk.4, + fresh.1, ); + assert!( + shrunk.0 <= 12, + "narrower slices retire fewer atoms per drain" + ); + assert!(shrunk.2 >= fresh.2, "and take more drains to do it"); - // A later resize on a different axis must not disturb the third one -- + // A later resize on a different axis must not disturb the third one -- // A later resize on a different axis must not disturb the third one -- // the split was permanent, with columns and lines tracking correctly // while a stale depth persisted forever. term.resize(Size { @@ -1078,17 +1123,28 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { // ends; only a sweep catches a non-monotone middle, and a wrap *is* a // non-monotone middle -- it makes the worst grid look cheap and hands it // the widest slice of all. - let mut previous = usize::MAX; - for exponent in 0..60 { - let scrollback = 1usize << exponent; - let width = slice_bytes(200, 50, scrollback); - assert!( - width <= previous, - "slice widened from {previous} to {width} at scrollback \ - 2^{exponent}: more expensive grid, more generous slice", - ); - assert!(width >= MIN_SLICE); - previous = width; + // Every axis independently: a wrap on any one of the three products is a + // non-monotone middle on that axis alone, and sweeping only scrollback + // would miss a truncating `columns * lines`. + for (axis, at) in [ + ( + "scrollback", + (|n| slice_bytes(200, 50, n)) as fn(usize) -> usize, + ), + ("columns", |n| slice_bytes(n.max(1), 50, 0)), + ("lines", |n| slice_bytes(200, n.max(1), 0)), + ] { + let mut previous = usize::MAX; + for exponent in 0..60 { + let width = at(1usize << exponent); + assert!( + width <= previous, + "slice widened from {previous} to {width} at {axis} \ + 2^{exponent}: more expensive grid, more generous slice", + ); + assert!(width >= MIN_SLICE); + previous = width; + } } // Just past 32 bits on one axis: large enough that a narrowing cast From 84b274d3c5da05257df95953b3f5115ead5420dc Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:37:44 -0400 Subject: [PATCH 08/15] test(terminal): make the decrease and RIS arms assert what they claim Test-only. Five gaps, four of them found independently by two reviewers, which is the part worth noting: they are the kind that pass. The decrease arm captured `first_pending` and never asserted it, compared `first_units` to the literal `12` rather than the control's own field, and ran its geometry-persistence check on a *different* terminal -- one that had only ever grown. That last one is the sharp one. It would pass an implementation that retained the debt on shrink and dropped it on the next geometry resize, which is a real shape: `Feeder::resize` sees every resize, and only the scrollback branch is conditional. Verified by writing that mutant: the arm now fails `left: 80000, right: 4040000`, and it did not before. Two things had to be true for it to bite, and it was inert without either. It runs on the terminal that actually went shallow -> deep -> shallow, and the resize carries the *shallow* depth -- handing the debt's own value back in makes `max(debt, new)` and a plain assignment agree, so the arm cannot tell them apart. I found that second one by writing the mutant, watching it survive, and looking again. The RIS table collected all six arms before asserting but then asserted them in a loop, so a failure still printed one. Collecting stops an arm being skipped; comparing the vectors is what stops the failure being truncated. With the history term deleted the failure now shows all six rows and which ones moved, rather than the first mismatch. Every comparison in the decrease arm is now against the control's own field. A constant or geometry change has to move both sides together, or the fixture quietly starts asserting the arithmetic of the day it was written. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/tests/slicing.rs | 106 +++++++++++++----- 1 file changed, 75 insertions(+), 31 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index 3007e23ee..32da2ecdc 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -593,14 +593,21 @@ fn ris_is_priced_for_both_grids_and_the_scrollback_it_walks() { observed.push((label, scrollback, term.stats().completed_work)); } } - for (label, scrollback, work) in observed { - assert_eq!( - work, - 2 * cells + (scrollback * columns) as u64, - "{label}, scrollback {scrollback}: RIS must be priced on \ - configured depth, which is the same on both grids", - ); - } + // One comparison over the whole vector, not a loop of comparisons. + // Collecting first stops an arm from being *skipped*; asserting the + // vectors is what stops a failure from being *truncated* to the first + // mismatch. Otherwise the alt-screen receipt still never prints, which + // was the point of collecting. + let expected: Vec<_> = observed + .iter() + .map(|&(label, scrollback, _)| { + (label, scrollback, 2 * cells + (scrollback * columns) as u64) + }) + .collect(); + assert_eq!( + observed, expected, + "RIS must be priced on configured depth, identically on both grids", + ); } /// CBT is charged for exactly the cells it scans -- an equality, in both @@ -982,10 +989,12 @@ fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { // which is the fence working -- but none of them is the discriminator: // they separate only when the two depths straddle the slice floor, and // `completed_work` separates at every positive depth gap. - let debt = |from: Size, to: Size| { - let (mut term, _a) = Terminal::new(from, Fences::ALL); - term.resize(deep); - term.resize(to); + // + // Every comparison is against the fresh control's own field, never a + // literal: a constant or geometry change must move both sides together, + // or the fixture starts asserting the arithmetic of the day it was + // written. + let measure = |term: &mut Terminal| { term.reset_stats(); let mut drains = 1; let mut more = term.feed(&b"\x1bc".repeat(200)); @@ -1003,39 +1012,74 @@ fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { term.stats().completed_work, ) }; - let shrunk = debt(shallow, shallow); - let fresh = run(shallow, None); - assert_eq!(shrunk.3, 200, "no unit may be lost on the way down either"); + // The terminal under test stays alive past its measurement, so the + // geometry arm below runs on the feeder that actually shrank rather than + // on a lookalike that only ever grew. + let (mut shrunk_term, _a) = Terminal::new(shallow, Fences::ALL); + shrunk_term.resize(deep); + shrunk_term.resize(shallow); + let shrunk = measure(&mut shrunk_term); + + let (mut fresh_term, _a) = Terminal::new(shallow, Fences::ALL); + let fresh = measure(&mut fresh_term); + + assert_eq!( + shrunk.3, fresh.3, + "no unit may be lost on the way down either" + ); assert!( - shrunk.4 > fresh.1, + shrunk.4 > fresh.4, "a feeder that has been deep must still price deep after shrinking: \ {} against a fresh shallow {}. Equality here is the signature of a \ feeder that dropped the debt, which is indistinguishable from one \ that never had it", shrunk.4, + fresh.4, + ); + assert!( + shrunk.0 <= fresh.0, + "narrower slices retire fewer atoms per drain: {} against {}", + shrunk.0, + fresh.0, + ); + assert!( + shrunk.1 >= fresh.1, + "and leave more pending after the first call: {} against {}", + shrunk.1, fresh.1, ); assert!( - shrunk.0 <= 12, - "narrower slices retire fewer atoms per drain" + shrunk.2 >= fresh.2, + "and take more drains to finish: {} against {}", + shrunk.2, + fresh.2, ); - assert!(shrunk.2 >= fresh.2, "and take more drains to do it"); - // A later resize on a different axis must not disturb the third one -- // A later resize on a different axis must not disturb the third one -- - // the split was permanent, with columns and lines tracking correctly - // while a stale depth persisted forever. - term.resize(Size { - columns: deep.columns * 2, - ..deep + // The debt survives a later resize on a different axis. Two things make + // this arm bite, and it was inert without either: + // + // * It runs on the terminal that actually went shallow -> deep -> + // shallow. A lookalike that only ever grew passes it while an + // implementation that retains on shrink and drops on the next geometry + // change fails. + // * The resize carries the *shallow* depth. Passing the debt's own value + // back in means `max(debt, new)` and a plain assignment agree, so the + // arm cannot tell them apart -- which is how it survived a mutant that + // retained only when columns and lines were unchanged. + shrunk_term.resize(Size { + columns: shallow.columns * 2, + screen_lines: shallow.screen_lines, + scrollback: shallow.scrollback, }); - term.reset_stats(); - term.feed_fully(b"c"); + shrunk_term.reset_stats(); + shrunk_term.feed_fully(b"\x1bc"); assert_eq!( - term.stats().completed_work, - 2 * (deep.columns * 2 * deep.screen_lines) as u64 - + (deep.scrollback * deep.columns * 2) as u64, - "a columns resize must keep the scrollback it was already given", + shrunk_term.stats().completed_work, + 2 * (shallow.columns * 2 * shallow.screen_lines) as u64 + + (deep.scrollback * shallow.columns * 2) as u64, + "a columns resize must keep the deep scrollback debt, not fall back \ + to the current shallow depth", ); } From d67b99a3dd611f0c239d9bce13552417acaba622 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:42:44 -0400 Subject: [PATCH 09/15] refactor(terminal): delete the slice-sizing function nothing calls `slice_bytes(columns, lines, scrollback)` stopped being the scheduler's function when slices became remaining-aware, and the fixtures went on asserting against it. The two disagreed exactly where the old floor bound -- reporting 4 bytes where `drain` used 1 -- so the preconditions guarding the decrease arm were describing behaviour that no longer existed. Not wrong at today's geometries, and no test would have noticed when it became wrong. It had zero callers outside the tests it misled. Deleted, with every assertion moved to `slice_bytes_remaining`, which is what the engine calls. One function, one answer. `MIN_SLICE` goes with it: the floor is 1 and lives in the function, because on a grid whose worst atom exceeds the whole budget no wider slice can promise to stop after the callback that crosses. A named constant that only appeared in a doc comment is a second authority waiting to disagree with the first. 69 tests, debug and release. Test-only in effect; the deleted item was unreachable from the engine. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/fences.rs | 60 ++++++------------- .../crates/buzz-terminal/tests/slicing.rs | 42 +++++++------ 2 files changed, 42 insertions(+), 60 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index 7ee9f30e2..ab5d3234d 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -34,53 +34,29 @@ pub const OSC_BUDGET: usize = 256 << 10; /// spent to the last unit. pub const WORK_BUDGET: u64 = 250_000; -/// Bounds on how many bytes are handed to the parser at once. +/// Widest slice handed to the parser at once. /// -/// The budget is checked *between* slices, so the slice is what actually -/// bounds one lock hold, and a fixed byte count cannot do it: `ESC#8` is -/// three bytes and costs a full grid, so 256 bytes of it is 1.6 ms at 200x50 -/// and 14 ms at 1600x50. [`slice_bytes`] therefore derives the size from the -/// grid, and these two clamp it -- `MAX` at the throughput plateau (measured: -/// plain-char parsing saturates by 64 bytes and is flat to 64 KiB), `MIN` -/// where a slice stops being able to hold a whole escape sequence: 4 bytes -/// covers `ESC#8` and `ESC c` intact, so the floor never splits the densest -/// atoms across slices for no benefit. Measured throughput at the floor is -/// ~74% of the plateau on plain text and ~77% on SGR, which is the price of -/// bounding a grid whose worst atom exceeds the budget outright. -pub const MIN_SLICE: usize = 4; +/// The floor is 1 byte and lives in [`slice_bytes_remaining`] rather than +/// here: on a grid whose worst atom exceeds the whole budget -- RIS at any +/// real scrollback depth -- no wider slice can promise to stop after the +/// callback that crosses. This cap is the other end, set at the throughput +/// plateau: plain-char parsing saturates by 64 bytes and is flat to 64 KiB +/// measured, so nothing above it buys anything and a larger value only +/// coarsens the cut. pub const MAX_SLICE: usize = 256; -/// Bytes to hand the parser at once on a `columns x lines` grid. +/// Bytes to hand the parser next. /// -/// Sized against the **densest atom the grid admits**, so the bound holds on -/// the first byte of a cold feeder for any payload: two bytes of `ESC c` buy -/// [`max_atom_work`], which is the most work per byte upstream offers, and -/// `budget / (atom / 2)` is the widest slice that cannot exceed one budget. +/// The **only** slice-sizing function, deliberately: an earlier version of +/// this module also exported a `slice_bytes(columns, lines, scrollback)` that +/// the scheduler stopped calling when slices became remaining-aware, and the +/// fixtures went on asserting against it. The two disagreed exactly where the +/// floor bound -- reporting 4 where the engine used 1 -- so the preconditions +/// were describing a function no longer in the path. One function, one +/// answer, and every test asserts on what `drain` actually calls. /// -/// Deliberately *derived rather than learned*. An earlier version sized -/// slices from the density of preceding slices, which is strictly worse where -/// it matters: a fresh feeder has observed nothing, so its first slice is -/// wide, and a first wide slice of RIS spends many budgets before anything -/// looks. A bound that has to be taught is not a bound on the lesson. -/// -/// [`MIN_SLICE`] floors it, and on any grid with real scrollback the floor is -/// what binds -- RIS at the default 10k depth is worth more than the entire -/// budget on its own, so no slice size can keep a drain inside the budget and -/// the floor stops the arithmetic from asking for fractions of a byte. That -/// residual is not hidden: it is exactly [`max_drain_work`], and it is the -/// honest cost of an indivisible callback that upstream can be asked to make -/// smaller only by not calling it. -pub fn slice_bytes(columns: usize, lines: usize, scrollback: usize) -> usize { - let densest = (max_atom_work(columns, lines, scrollback) / 2).max(1); - ((WORK_BUDGET / densest) as usize).clamp(MIN_SLICE, MAX_SLICE) -} - -/// Bytes to hand the parser when `spent` of the budget is already gone. -/// -/// The scheduling rule in one place so the fixtures can assert on it rather -/// than on a copy of the arithmetic: a slice of `N` bytes holds at most -/// `N / atom_bytes` atoms, so `remaining / densest` bytes cannot carry a -/// drain past the budget. +/// The rule: a slice of `N` bytes holds at most `N / atom_bytes` atoms, so +/// `remaining / densest` bytes cannot carry a drain past the budget. /// /// `next_escape` is how far the next `ESC` is from the front of the tail. /// This is the difference between a correct bound and an unusable one. Only diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index 32da2ecdc..f4b87a0d8 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -10,8 +10,8 @@ //! `> 0` is satisfied by a seam that executed exactly one unit. use buzz_terminal::fences::{ - max_atom_work, max_drain_work, slice_bytes, slice_bytes_remaining, Fences, MAX_SLICE, - MIN_SLICE, SYNC_CAP, TAIL_CAP, WORK_BUDGET, + max_atom_work, max_drain_work, slice_bytes_remaining, Fences, MAX_SLICE, SYNC_CAP, TAIL_CAP, + WORK_BUDGET, }; use buzz_terminal::{Size, Terminal}; @@ -339,26 +339,26 @@ fn a_resize_mid_tail_reprices_the_remainder() { /// serves both. #[test] fn slice_size_shrinks_as_the_worst_atom_grows() { - let small = slice_bytes(80, 24, 0); - let large = slice_bytes(1600, 50, 0); + let small = slice_bytes_remaining(80, 24, 0, 0, 0); + let large = slice_bytes_remaining(1600, 50, 0, 0, 0); assert!( small > large, "a bigger grid makes each byte more expensive, so slices must shrink: \ 80x24 -> {small}, 1600x50 -> {large}", ); assert!( - slice_bytes(200, 50, 10_000) <= slice_bytes(200, 50, 0), + slice_bytes_remaining(200, 50, 10_000, 0, 0) <= slice_bytes_remaining(200, 50, 0, 0, 0), "scrollback makes RIS more expensive, so it may only shrink slices", ); for (columns, lines, scrollback) in [(80, 24, 0), (200, 50, 0), (400, 100, 0), (1600, 50, 0)] { - assert!((MIN_SLICE..=MAX_SLICE).contains(&slice_bytes(columns, lines, scrollback))); + assert!((1..=MAX_SLICE).contains(&slice_bytes_remaining(columns, lines, scrollback, 0, 0))); // One slice holds at most N/2 of the densest atom. Either that fits a // budget, or the floor binds -- and then the overshoot is stated by // `max_drain_work` rather than being an accident. - let worst = (slice_bytes(columns, lines, scrollback) as u64 / 2) - * max_atom_work(columns, lines, scrollback); + let width = slice_bytes_remaining(columns, lines, scrollback, 0, 0); + let worst = (width as u64 / 2) * max_atom_work(columns, lines, scrollback); assert!( - worst <= WORK_BUDGET || slice_bytes(columns, lines, scrollback) == MIN_SLICE, + worst <= WORK_BUDGET || width == 1, "{columns}x{lines}: a slice buys {worst} work against a \ {WORK_BUDGET} budget without the MIN clamp to excuse it", ); @@ -923,12 +923,18 @@ fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { // depth, so a conforming repair would show work identical to the // control and the assertions here would invert into false failures. assert!( - slice_bytes(shallow.columns, shallow.screen_lines, shallow.scrollback) > MIN_SLICE, + slice_bytes_remaining( + shallow.columns, + shallow.screen_lines, + shallow.scrollback, + 0, + 0 + ) > 1, "geometry cannot discriminate: the shallow arm is already floored", ); assert_eq!( - slice_bytes(deep.columns, deep.screen_lines, deep.scrollback), - MIN_SLICE, + slice_bytes_remaining(deep.columns, deep.screen_lines, deep.scrollback, 0, 0), + 1, ); // How a terminal at `size` retires 200 RIS: work, and how many @@ -1151,8 +1157,8 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { // most expensive. Wrapping inverts the fence. So: the widest possible // atom must give the narrowest possible slice. assert_eq!( - slice_bytes(huge, huge, huge), - MIN_SLICE, + slice_bytes_remaining(huge, huge, huge, 0, 0), + 1, "an overflowing grid must clamp to the smallest slice; a wrapped \ `max_atom_work` would hand back a generous one", ); @@ -1173,10 +1179,10 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { for (axis, at) in [ ( "scrollback", - (|n| slice_bytes(200, 50, n)) as fn(usize) -> usize, + (|n| slice_bytes_remaining(200, 50, n, 0, 0)) as fn(usize) -> usize, ), - ("columns", |n| slice_bytes(n.max(1), 50, 0)), - ("lines", |n| slice_bytes(200, n.max(1), 0)), + ("columns", |n| slice_bytes_remaining(n.max(1), 50, 0, 0, 0)), + ("lines", |n| slice_bytes_remaining(200, n.max(1), 0, 0, 0)), ] { let mut previous = usize::MAX; for exponent in 0..60 { @@ -1186,7 +1192,7 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { "slice widened from {previous} to {width} at {axis} \ 2^{exponent}: more expensive grid, more generous slice", ); - assert!(width >= MIN_SLICE); + assert!(width >= 1); previous = width; } } From fe7d291f484184e7353be7358a694f2feaad9406 Mon Sep 17 00:00:00 2001 From: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Date: Sun, 2 Aug 2026 01:42:10 -0400 Subject: [PATCH 10/15] test(terminal): reject stop before child reap Co-authored-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Signed-off-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> --- .../buzz-terminal/src/lifecycle_tests.rs | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs index c2a5e2eda..7f29cb5cc 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lifecycle_tests.rs @@ -409,10 +409,10 @@ fn tempdir(name: &str) -> std::path::PathBuf { /// Mari's noisy-child discriminator: the reader must still be draining /// **while** the child is being terminated and reaped. /// -/// The property is an ordering, and an ordering cannot be observed from the -/// outcome -- both orders end with a dead child and a stopped reader. So the -/// reader records *when* it read, relative to the moment close begins, and the -/// assertion is on bytes drained after that moment. +/// The portable contract is structural: `stop` must not be requested until +/// the child has been reaped. The recording reader checks the child PID at the +/// `stop` call, while the continuously noisy PTY makes the test exercise a +/// reader that is genuinely active rather than a quiet no-op. /// /// The child is deliberately noisy: it floods the PTY continuously, so a /// master that stops being read fills its kernel buffer within milliseconds @@ -427,20 +427,18 @@ fn reader_drains_through_termination_and_reap() { // process dies, so there is always more output pending than the buffer // holds. let mut child = spawn_noisy(&pair); + let pid = child.process_id().expect("pid") as i32; - let after_close = std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)); - let closing = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let order = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let reader = RecordingReader::spawn(&pair, after_close.clone(), closing.clone(), order.clone()); + let reader = RecordingReader::spawn(&pair, pid, order.clone()); - // Let the child get well ahead of the reader before we touch anything. + // Establish that this is a live draining reader, not a quiet fixture. assert!( - poll_until(|| after_close.load(Ordering::Relaxed) == 0 && reader.total_bytes() > 4096), + poll_until(|| reader.total_bytes() > 4096), "test setup: the child is not producing enough output to fill the pty \ buffer, so this cannot distinguish drain order" ); - closing.store(true, Ordering::Relaxed); let started = Instant::now(); let outcome = shutdown_draining(&mut child, Box::new(reader)).expect("shutdown"); let elapsed = started.elapsed(); @@ -451,11 +449,6 @@ fn reader_drains_through_termination_and_reap() { "a `yes` pipeline dies on SIGTERM; SIGKILL here means it was wedged in \ a tty write against an undrained master" ); - assert!( - after_close.load(Ordering::Relaxed) > 0, - "the reader drained nothing after close began -- it was joined before \ - termination, which is the ordering the drain law forbids" - ); assert!( elapsed < TERM_GRACE, "shutdown took {elapsed:?}, at or beyond the {TERM_GRACE:?} grace \ @@ -482,6 +475,7 @@ fn spawn_noisy(pair: &PtyPair) -> Box { /// A [`DrainingReader`] that records how much it read after close began. struct RecordingReader { + pid: i32, total: std::sync::Arc, handle: std::thread::JoinHandle<()>, order: std::sync::Arc>>, @@ -490,8 +484,7 @@ struct RecordingReader { impl RecordingReader { fn spawn( pair: &PtyPair, - after_close: std::sync::Arc, - closing: std::sync::Arc, + pid: i32, order: std::sync::Arc>>, ) -> Self { let mut reader = pair.master.try_clone_reader().expect("reader"); @@ -505,12 +498,10 @@ impl RecordingReader { break; } counter.fetch_add(n as u64, Ordering::Relaxed); - if closing.load(Ordering::Relaxed) { - after_close.fetch_add(n as u64, Ordering::Relaxed); - } } }); Self { + pid, total, handle, order, @@ -528,6 +519,10 @@ impl DrainingReader for RecordingReader { } fn stop(&self) { + assert!( + !pid_alive(self.pid), + "reader stop must not be requested before the child is reaped" + ); self.order.lock().unwrap().push("stop"); } From 651ef710b43bb78707d9194eb7f2304012e1bf44 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:48:49 -0400 Subject: [PATCH 11/15] docs(terminal): repoint the links the slice_bytes deletion broke Two intra-doc links still resolved to the deleted function, so `cargo doc` emitted unresolved-link warnings. Neither `cargo test` nor clippy sees those, which is why they survived a green gate -- the deletion was verified by the checks that run in CI and not by the one that reads the docs. The single remaining mention is deliberate: the paragraph in `fences.rs` explaining why the old function existed and why it is gone. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- desktop/src-tauri/crates/buzz-terminal/src/fences.rs | 2 +- desktop/src-tauri/crates/buzz-terminal/src/reader.rs | 2 +- desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index ab5d3234d..17a67f490 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -130,7 +130,7 @@ pub fn max_atom_work(columns: usize, lines: usize, scrollback: usize) -> u64 { /// fixtures caught it: /// /// * The budget is checked *between* slices, so a drain overshoots by up to -/// one whole slice -- not one atom. [`slice_bytes`] keeps that under one +/// one whole slice -- not one atom. [`slice_bytes_remaining`] keeps that under one /// budget wherever its derivation is unclamped. /// * A callback already running cannot be preempted. RIS at the default 10k /// scrollback is worth 16x the whole budget on its own, so on such a grid diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 82324f460..18ce98d41 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -172,7 +172,7 @@ impl Feeder { /// per count when it finds no stop. /// /// What *is* bounded is the number of atoms per slice, and that bound - /// holds from the first byte of a cold feeder: [`slice_bytes`] is derived + /// holds from the first byte of a cold feeder: [`slice_bytes_remaining`] is derived /// from the densest work-per-byte upstream can produce on this grid, so /// no slice can contain more than one budget's worth of callbacks no /// matter what the payload is or what the feeder has seen before. diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index f4b87a0d8..26097430a 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -333,7 +333,7 @@ fn a_resize_mid_tail_reprices_the_remainder() { /// byte count cannot bound a lock hold: `ESC c` is two bytes and resets both /// grids plus scrollback. /// -/// Kills: replacing `slice_bytes` with a constant, or deriving it from +/// Kills: replacing `slice_bytes_remaining` with a constant, or deriving it from /// `cells` while the worst atom is larger than `cells`. Measured: 256 bytes /// of DECALN is 1.6 ms at 200x50 and ~14 ms at 1600x50, so no one constant /// serves both. @@ -1152,7 +1152,7 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { // The *direction* is the assertion, not merely the absence of a panic. // A wrapping build does not produce a slightly-wrong bound, it produces a - // tiny one -- and `slice_bytes` divides the budget by it, so an + // tiny one -- and `slice_bytes_remaining` divides the budget by it, so an // undercharged atom yields an *oversized* slice exactly when the atom is // most expensive. Wrapping inverts the fence. So: the widest possible // atom must give the narrowest possible slice. From 9ca661ee8e684d720724673b69c8f8a110fb72b2 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:52:08 -0400 Subject: [PATCH 12/15] docs(terminal): say that the tail-depth signals have no consumer yet `tail_full` and `tail_drained` are the queue bound, and nothing outside the tests calls them: the runtime reader pumps `drain` to completion after every read, so the tail cannot reach the cap and nobody needs to ask. That is fine and it is the design -- the signal was built for the reader that stops pumping -- but an exported, documented predicate with an empty call graph reads as wired to anyone who doesn't go looking. Stated in the doc comment instead, because an unused signal that looks connected is worse than one that says it isn't. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- desktop/src-tauri/crates/buzz-terminal/src/lib.rs | 1 + desktop/src-tauri/crates/buzz-terminal/src/reader.rs | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs index 5959d79d9..9665ed07d 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs @@ -158,6 +158,7 @@ impl Terminal { } /// Whether the tail is at its cap and the reader must stop reading. + /// Not yet consumed in production -- see [`reader::Feeder::tail_full`]. pub fn tail_full(&self) -> bool { self.feeder.tail_full() } diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 18ce98d41..4dc4aa46c 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -111,6 +111,15 @@ impl Feeder { /// latch is a state the fence owns and could fail to clear, which is /// exactly how a paused reader strands a child mid-teardown; a reader that /// simply stops asking resumes by default. + /// + /// **Not yet consumed in production.** The runtime reader pumps + /// [`Feeder::drain`] to completion after every read, so the tail cannot + /// currently grow to the cap and nothing needs to ask. This signal exists + /// for the reader that stops pumping -- it is the queue bound, and the + /// pump loop is the only reason the queue bound is not load-bearing + /// today. Stated rather than left to be inferred from an empty + /// call-graph: an unused signal that looks wired is worse than one that + /// says it isn't. pub fn tail_full(&self) -> bool { self.pending_bytes() >= TAIL_CAP } From 5f416f7d25b079fe264c4a473a9bb13619237d27 Mon Sep 17 00:00:00 2001 From: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Date: Sun, 2 Aug 2026 01:53:12 -0400 Subject: [PATCH 13/15] test(terminal): require the runtime to pump deferred work Co-authored-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Signed-off-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> --- .../crates/buzz-terminal/src/fences.rs | 4 +-- .../src-tauri/crates/buzz-terminal/src/lib.rs | 5 ++++ .../crates/buzz-terminal/src/reader.rs | 5 ++-- desktop/src-tauri/src/terminal_runtime.rs | 30 ++++++++++++++++--- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index ab5d3234d..2797fa401 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -130,8 +130,8 @@ pub fn max_atom_work(columns: usize, lines: usize, scrollback: usize) -> u64 { /// fixtures caught it: /// /// * The budget is checked *between* slices, so a drain overshoots by up to -/// one whole slice -- not one atom. [`slice_bytes`] keeps that under one -/// budget wherever its derivation is unclamped. +/// one whole slice -- not one atom. [`slice_bytes_remaining`] keeps that +/// under one budget wherever its derivation is unclamped. /// * A callback already running cannot be preempted. RIS at the default 10k /// scrollback is worth 16x the whole budget on its own, so on such a grid /// the floor binds and the overshoot is a few of those atoms. No scheduler diff --git a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs index ff8796870..72edac1af 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/lib.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/lib.rs @@ -161,6 +161,11 @@ impl Terminal { } /// Whether the tail is at its cap and the reader must stop reading. + /// + /// There is no production consumer today, deliberately: the desktop + /// runtime pumps `drain()` to completion after every read, so the tail is + /// empty between iterations. A future reader that defers pumping must + /// consult this signal before accepting more PTY bytes. pub fn tail_full(&self) -> bool { self.feeder.tail_full() } diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 82324f460..0a09bc160 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -172,8 +172,9 @@ impl Feeder { /// per count when it finds no stop. /// /// What *is* bounded is the number of atoms per slice, and that bound - /// holds from the first byte of a cold feeder: [`slice_bytes`] is derived - /// from the densest work-per-byte upstream can produce on this grid, so + /// holds from the first byte of a cold feeder: + /// [`slice_bytes_remaining`] is derived from the densest work-per-byte + /// upstream can produce on this grid, so /// no slice can contain more than one budget's worth of callbacks no /// matter what the payload is or what the feeder has seen before. /// diff --git a/desktop/src-tauri/src/terminal_runtime.rs b/desktop/src-tauri/src/terminal_runtime.rs index 30d8df6d1..9be896e9b 100644 --- a/desktop/src-tauri/src/terminal_runtime.rs +++ b/desktop/src-tauri/src/terminal_runtime.rs @@ -203,6 +203,15 @@ fn wire_publication(publication: Publication) -> Result { }) } +fn feed_and_drain(terminal: &SharedTerminal, bytes: &[u8]) -> bool { + let mut more = terminal.feed(bytes); + let deferred = more; + while more && !terminal.is_closing() { + more = terminal.drain(); + } + deferred +} + struct ReaderThread { handle: Option>, terminal: Arc, @@ -502,10 +511,7 @@ pub(crate) fn terminal_attach( if reader_terminal.is_closing() { continue; } - let mut more = reader_terminal.feed(&buffer[..count]); - while more && !reader_terminal.is_closing() { - more = reader_terminal.drain(); - } + let _ = feed_and_drain(&reader_terminal, &buffer[..count]); if reader_terminal.is_closing() { continue; } @@ -815,6 +821,22 @@ mod tests { assert!(successor.frame.full); } + #[test] + fn reader_pumps_a_deferred_tail_without_an_external_event() { + let (terminal, _actions) = Terminal::new(Size::default(), Fences::ALL); + let terminal = SharedTerminal::new(terminal); + let payload = "\u{1b}c".repeat(2_102_714); + + assert!( + feed_and_drain(&terminal, payload.as_bytes()), + "fixture must defer parser work before the runtime pumps it" + ); + + let terminal = terminal.lock(); + assert_eq!(terminal.pending_bytes(), 0); + assert_eq!(terminal.stats().completed_units, 2_102_714); + } + #[test] fn initial_attach_retains_output_captured_after_its_bootstrap_snapshot() { let viewport = marker_frame(0, true).viewport; From 232f56b67219613ddb1fd6628a73ae31bd1156a5 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:56:59 -0400 Subject: [PATCH 14/15] docs(terminal): say why tail_full cannot fire, not just that nothing calls it The previous wording said the signal has no consumer. That is the weaker half of the fact and the half that ages badly: a reader who deletes the pump loop learns the rest by wedging a terminal. Both measurements are now in the comment. With the loop, 1 MiB of pure RIS through 16 KiB reads leaves a tail high-water of exactly 0 -- the tail is empty between iterations, so the predicate is unreachable by construction. Without it, the same stream reaches TAIL_CAP at read 257. Reproduced here before quoting: 0 and 257 both hold on this tree. Wording is Sami's, who measured it first and made the argument that the signal and the loop are one fact from two sides. Delete the loop and this predicate stops being unreachable in the same instant it starts being needed. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/reader.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 4dc4aa46c..438267627 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -112,14 +112,19 @@ impl Feeder { /// exactly how a paused reader strands a child mid-teardown; a reader that /// simply stops asking resumes by default. /// - /// **Not yet consumed in production.** The runtime reader pumps - /// [`Feeder::drain`] to completion after every read, so the tail cannot - /// currently grow to the cap and nothing needs to ask. This signal exists - /// for the reader that stops pumping -- it is the queue bound, and the - /// pump loop is the only reason the queue bound is not load-bearing - /// today. Stated rather than left to be inferred from an empty - /// call-graph: an unused signal that looks wired is worse than one that - /// says it isn't. + /// **No production consumer today, and not an oversight.** The runtime + /// reader pumps [`Feeder::drain`] to completion after every read + /// (`terminal_runtime.rs`), so the tail is empty between iterations and + /// this can never go true -- measured 0 bytes high-water against 1 MiB of + /// pure RIS, the densest atom there is. It exists for a future reader + /// that defers pumping, and such a reader **must** consult it: without + /// the pump loop the same stream reaches [`TAIL_CAP`] in 257 reads of + /// 16 KiB. + /// + /// The numbers are here rather than "nothing calls this" because the + /// signal and the loop are one fact from two sides. Delete the loop and + /// this predicate stops being unreachable in the same instant it starts + /// being needed. pub fn tail_full(&self) -> bool { self.pending_bytes() >= TAIL_CAP } From e1eaf88b0d7c9b2f9922ffea6a2cbd47682e746d Mon Sep 17 00:00:00 2001 From: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Date: Sun, 2 Aug 2026 02:01:55 -0400 Subject: [PATCH 15/15] docs(terminal): cite the discriminating tail payload Co-authored-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> Signed-off-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz> --- desktop/src-tauri/crates/buzz-terminal/src/reader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs index 438267627..0218dcda9 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/reader.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/reader.rs @@ -115,7 +115,7 @@ impl Feeder { /// **No production consumer today, and not an oversight.** The runtime /// reader pumps [`Feeder::drain`] to completion after every read /// (`terminal_runtime.rs`), so the tail is empty between iterations and - /// this can never go true -- measured 0 bytes high-water against 1 MiB of + /// this can never go true -- measured 0 bytes high-water against 8 MiB of /// pure RIS, the densest atom there is. It exists for a future reader /// that defers pumping, and such a reader **must** consult it: without /// the pump loop the same stream reaches [`TAIL_CAP`] in 257 reads of