From 1a2c86ebedb739701b172b061cae45a0ddcadf3a Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:26:26 -0400 Subject: [PATCH 1/2] 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 2/2] 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