From 551c01398fe3d9b855d5f685560b99a5d34f7b97 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 7 Sep 2026 20:52:45 -0400 Subject: [PATCH] iris: the guards against silently wrong output survive into release docs/REVIEW-2026-09-07.md's R1. Every invariant guard added on 2026-09-07 was a `debug_assert!`, and every build anybody runs on this project is release -- the bench APK must be (the debug `libmain.so` is 325 MB and will not install) and Iris's phone gets release too. So a `List` drawn without a mask painted over its surroundings again, in exactly the build the fault was found in, with nothing saying so. Promoted to `assert!`, each O(1) or a handful per *draw* and each protecting against output that is wrong on screen with no other symptom: `List::draw`'s `painter.is_masked()`, `List`'s `extents`-are-on-screen check, `Painter::set_mask`'s doubled-call check (the second call replaces rather than nests, i.e. an unclipped widget), `Painter::glyphs`'s atlas generation (glyphs sampled from coordinates now holding other letters), and `List::fling`'s finiteness (one comparison per gesture; NaN propagates into `deceleration_for`'s `ln()` and the fling never settles). Left as `debug_assert!` and now saying so in a comment: `List::place`'s slot-exists precondition (once per row placed per frame, and its release failure is the `.expect` below rather than something wrong on screen) and `poly_fit_least_squares`'s two preconditions (run on every velocity query, with `MIN_SAMPLE_SIZE` and the `is_finite` check giving release a defined outcome either way). `PointerClock::sample`'s ordering assert was already annotated in 2ec0fee for the same reason. Verified: `cargo test --lib -p iris` (103) and `cargo test -p transcript-fixture` (12) pass in both debug *and* `--release`, which is what says the promoted asserts do not fire on a real replayed flick; fmt, clippy and `cargo ndk check -p iris` clean. Co-Authored-By: Claude Fable 5.1 --- iris/core/src/ui/painter.rs | 13 ++++++++++--- iris/src/sense.rs | 5 +++++ iris/src/widget/list.rs | 25 ++++++++++++++++++++----- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/iris/core/src/ui/painter.rs b/iris/core/src/ui/painter.rs index fbe8b44..0eec9a3 100644 --- a/iris/core/src/ui/painter.rs +++ b/iris/core/src/ui/painter.rs @@ -65,7 +65,12 @@ impl<'a> Painter<'a> { /// so keeps pointing at whichever slot it was drawn under. See /// `ActiveData::own_mask` for what pushing a fresh one cost. pub fn set_mask(&mut self, region: UiRegion) { - debug_assert!( + // `assert!`, not `debug_assert!`: one comparison per widget draw, + // and the second call silently *replacing* the first is a widget + // drawn unclipped -- which reaches the screen and nothing says so. + // Every build anybody runs here is release + // (docs/REVIEW-2026-09-07.md's R1). + assert!( self.own_mask == MaskIdx::NONE || self.mask != self.own_mask, "set_mask called twice while drawing one widget: the second would replace the first \ rather than nest inside it", @@ -264,8 +269,10 @@ impl<'a> Painter<'a> { // A caller re-emitting quads placed against an atlas that has since // been cleared draws every glyph from coordinates now holding // something else. Caught at the submission rather than on screen, - // where it reads as fragments of unrelated letters. - debug_assert_eq!( + // where it reads as fragments of unrelated letters. `assert_eq!` + // for R1's reason: two integers per laid-out string, not per + // glyph, and the failure is unreadable text on a release build. + assert_eq!( text.generation, self.atlas_generation(), "glyphs placed against atlas generation {} submitted against {}: the holder did not \ diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 76d6082..6d31255 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -1221,6 +1221,11 @@ impl VelocityTracker { /// points than coefficients; [`MIN_SAMPLE_SIZE`] makes that unreachable /// from the only caller, so the truncation is an assert instead of a /// branch that could never be exercised. +// Both guards stay `debug_assert!` under docs/REVIEW-2026-09-07.md's R1: +// they are preconditions of a fit run on every velocity query, and the two +// callers between them already answer 0 below `MIN_SAMPLE_SIZE` and check +// the result with `is_finite`, so a release build has a defined outcome +// rather than a silently wrong one. fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] { debug_assert_eq!(x.len(), y.len()); debug_assert!( diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index 5dfa71a..02e07bc 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -487,8 +487,12 @@ impl List { // through) would propagate silently into `deceleration_for`'s // `.ln()` -- the fling either never settles or jumps to NaN // positions with nothing on screen saying why (docs/ - // REVIEW-2026-09-06.md finding 3). - debug_assert!(velocity_px_per_s.is_finite()); + // REVIEW-2026-09-06.md finding 3). A plain `assert!` rather than a + // `debug_assert!`: it is one comparison per *gesture*, and every + // build anybody runs -- the emulator's and Iris's phone's -- is + // release, where a debug-only guard against silently wrong output + // is no guard at all (docs/REVIEW-2026-09-07.md's R1). + assert!(velocity_px_per_s.is_finite()); // Compose's two thresholds at a release, and **only** those two. // // The maximum is `ViewConfiguration.getScaledMaximumFlingVelocity()` @@ -1011,7 +1015,10 @@ impl List { // sites, not by this function, which would otherwise fail with a // bare "index out of bounds" and no context (docs/ // REVIEW-2026-09-06.md finding 2). `slot_widget`, called from - // here, is what actually indexes/`.expect`s on it. + // here, is what actually indexes/`.expect`s on it. Stays a + // `debug_assert!` under R1's rule: this runs once per row placed + // per frame, and its release failure is the `.expect` below rather + // than something silently wrong on screen. debug_assert!( self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}" @@ -1161,7 +1168,12 @@ impl Widget for List { // cannot set the mask itself, since `Painter::set_mask` allows one // mask per widget and rows of this list already use their own // (`transcript-ui`'s `row.rs`, `tool.rs`). So it checks instead. - debug_assert!( + // + // `assert!`, not `debug_assert!`: one bool per draw, and what it + // catches is a `List` painting over its surroundings with nothing + // on screen saying so -- the fault e922b73 was written to fix. + // Every build that runs is release (docs/REVIEW-2026-09-07.md's R1). + assert!( painter.is_masked(), "a `List` must be drawn inside something `.masked()`: it draws rows straddling both \ edges in full, so the parts outside its own box reach the screen otherwise", @@ -1223,7 +1235,10 @@ impl Widget for List { // what overlaps the viewport, and nothing above or below it can // be seen. The first failed silently for a whole build -- an // off-screen row draws correctly, it is just in the wrong place. - debug_assert!( + // `assert!` for R1's reason: it walks the rows *on screen*, a + // handful, once per draw, and a release build is the only build + // this fault has ever been seen in. + assert!( self.extents .values() .all(|e| self.intersects_viewport(e.top, e.bottom)),