From e397680a198d571bfbc838e5e1478c886da65305 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sun, 20 Sep 2026 22:05:10 -0400 Subject: [PATCH] Record the thirteenth sweep, a full pass over #19 Seven findings at cbccfb6, the largest being a length of zero that printed as the empty string -- which the twelfth sweep made load-bearing by pointing Debug at Display, and which scenario::describe rendered as "no rule at all". Also re-measured the hole Sow::bound's comment recorded: the seeds it named stopped naming those trees when the leaves grew images, and 600 depth-5 trees now agree with every bound a fraction. Co-Authored-By: Claude Opus 5 --- docs/HANDOFF.md | 40 +++++++++++++++++++- docs/LAYOUT_LOG.md | 91 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 3ab20d4..4400273 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -8,7 +8,7 @@ found is in `docs/LAYOUT_LOG.md`. Bryan clarified that the built-in attribute should replace the wrapper. Work is in `/home/bob/repos/iris-deferred`, on `layout/one-ask`, at -`05e6ced` (pushed). Bounds now constrain the offer in `Placing::ask`; +`cbccfb6` (pushed). Bounds now constrain the offer in `Placing::ask`; declarations are retained as resolved window lengths, so movement does not decide the bound again. `.max_width`/`.max_height` now edit the same widget, like the minimum @@ -53,6 +53,37 @@ and `revision_cost` resize -0.79% in release instruction medians. the three deferred corpora, 206 and 210 tests, and the cold dump byte-identical to `2ac0843` across all 34,986 boxes. +## A full sweep of #19 is submitted on it + +The thirteenth sweep landed at `cbccfb6`. Unlike the twelve before it, it +read the whole branch rather than one commit, and it is the first review of +`f48e04e`. Seven findings, the largest being that a length of zero printed as +the empty string: `Display for LayoutLen` leaves out each part that is zero, +and `f48e04e` had just pointed `Debug` at `Display`, so a request of zero +printed as nothing in the only place requests are compared -- and +`scenario::describe` printed a `.width(0)` rule as `-`, which is what it +prints for a widget with no rule at all. Also: `Fixed::ceil_from_f32` stepped +past the top of the grid and wrapped, so the largest measurement came back as +the most negative length; `Moves::depth` walked the move chain a second way +with its own copy of `CHAIN_LIMIT`; `Harness::set_len` claimed to set a +length "the way `.width()` sets one" and dropped any bound beside it; three +rigs each spelled "one seed, or a range of them" by hand; `diag::outside` +could count a refused reuse and explain it with nothing; and +`cases/deferred` sat outside `suite.rs`'s alphabetical list. + +It also re-measured a recorded hole. `Sow::bound` grows bounds in pixels +because two depth-5 trees once disagreed warm against cold with fractions in +them -- named by seed, and `generated.rs` says in its own comment that seeds +stopped naming those trees when the leaves grew images. 600 depth-5 trees +over all sixteen cases now agree with every bound a fraction (93.18s). The +generator still grows pixels, because `deferred_generated.rs` already varies +that dimension and growing fractions here would move every box in the cold +dump; the comment says that instead of describing an open defect. + +`docs/LAYOUT_LOG.md` has all seven, with the five things the sweep looked at +and left. Verified: format, clippy both ways, 208 and 212 tests, and the +cold dump byte-identical to `f48e04e` across all 34,986 boxes. + ## A quality sweep of the request arena is submitted on PR #19 The twelfth sweep landed at `f48e04e`. It is the first review of `05e6ced` -- @@ -203,6 +234,9 @@ comparison work above, its review rounds past `cadfba0` are described in the fold written twice and an `Arc` where nothing shares. - **The request arena's own sweep**, `f48e04e` -- the join that copies one request's nodes into another's, which nothing in the suite ran. +- **A full sweep of the branch**, `cbccfb6` -- a length of zero that printed + as nothing, a ceiling that stepped off the top of the grid, and a harness + setter that dropped the bound beside the length it set. The settled design of the vocabulary rounds is in `docs/LAYOUT.md` under "Three names, and the one argument that says them". Bryan settled the API @@ -295,6 +329,10 @@ a compile error, which reads exactly like a fuzzer failure. nothing and still look covered, since every test around it passes. Put a `panic!` in the arm and run the suite; it costs one build and it answers the question the test names cannot. + The thirteenth adds a fifth, about the record rather than the code: a + comment describing an open defect by the seed that found it stops being + true the moment the generator changes, and `generated.rs` says so in its + own comment. Re-run the measurement before repeating what one says. 2. **A review of everything written before the review gate existed.** `pre-submit-review` and the rule that nothing is submitted unreviewed arrived on 2026-09-13, well after the Rust port and most of Iris were diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index a5729dd..aebfb73 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,97 @@ nothing here is rediscovered. Each entry says who found it and when. it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and the current plan is in `docs/HANDOFF.md`. +## Thirteenth sweep: a full pass over #19 (2026-09-20) + +Over the whole branch rather than one commit, and the first review of +`f48e04e`. Seven findings, all in `cbccfb6`. + +- **A length of zero printed as nothing.** `Display for LayoutLen` leaves out + each part that is zero, so `LayoutLen::ZERO` is the empty string -- and + `f48e04e` pointed `Debug` at `Display`, so the four `assert_eq!`s in + `cases/deferred.rs` print nothing where a request of zero is. Worse in + `scenario::describe`, which prints a `.width(0)` rule as `-` -- the same + thing it prints for a widget with no rule. That function exists so a tree a + fuzzer found "can be written out by hand"; a value it cannot say is a hole + in the one thing it is for. Measured before the fix: + `format!("{}", px(0).min(px(40)))` was `""`, and `Size::ZERO` was `"(, )"`. + It is `0 px;` now, with a test over all four shapes of length. +- **A ceiling that stepped off the top of the grid.** `ceil_from_f32` takes + `next_up` of a `from_f32` that has already clamped, and `next_up` wraps, so + `Px::ceil_from_f32(1e12)` was `Px::MIN` -- the most negative length there + is, from the largest measurement. `from_f32` clamps deliberately ("a float + has further to come from"); the ceiling is the other way in from a float + and the rule governs both. The assertion goes beside the one `from_f32` + already had, which is where the class lives. +- **`Moves::depth` walked the chain a second way**, with its own copy of + `CHAIN_LIMIT` and without the debug assertion `walk` makes. It is + `self.walk(idx, |_| depth += 1)` now, so the CPU counts a move chain in one + place and the constant the shader is handed reaches both. +- **A harness setter that did not do what it said.** `Harness::set_len` said + "the way `.width()` sets one" and called `set_size_rule`, which writes the + whole rule -- so it dropped any bound beside the length, where + `Widgets::set_len` keeps one on purpose. A case that set a bound and then a + length would have passed with no bound at all. It calls `Widgets::set_len`. +- **Three spellings of "one seed, or a range of them."** `generated.rs`, + `shrink.rs` and `deferred_generated.rs` each wrote the `std::env::var(..) + .parse()` match by hand -- the class the eleventh sweep closed for *reading* + a parameter and not for this one. One `rig::seeds`, carrying an + `#[allow(dead_code)]` because the module is compiled into each rig target + and the measurement rigs choose no seeds. +- **A refusal that could go uncounted.** `diag::outside` writes out + `AxisHolds::contains`'s four clauses to say *which* one refused a reuse. A + fifth clause added there and not here would leave a refusal counted by + `ReuseOutside` and explained by nothing; a debug assertion catches that now, + which is the cheap guard rather than machinery to derive the reasons. +- **`cases/deferred` sat last** in `suite.rs`'s otherwise alphabetical list. + +**A stale record, re-measured.** `Sow::bound` grows bounds in pixels and its +comment said why: a fraction survives a `place_at` into a different rel base, +"seeds 4 (shuffle-all-but-first) and 196 (resize-size) at depth 5 are where +that showed". But `generated.rs` says in its own comment that a seed names a +tree only while the generator draws the same things in the same order, and +that adding images to the leaves moved every one of them -- which is +`2dba90b`, after that hole was recorded. Re-measured: 600 depth-5 trees over +all sixteen cases agree warm against cold with every bound a fraction +(93.18s, release). The generator still grows pixels, because +`deferred_generated.rs` already varies that dimension in its relative-bound +corpus and growing fractions here would move every box in the cold dump for +overlap; the comment now says that rather than describing an open defect. + +Five things the sweep **looked at and left**: + +- **`independent_order` compares two lengths by `leftover` where their pixels + and fractions are equal**, which is only sound while a leftover resolves at + a nonnegative ratio. Worked through: `allocate` starts at `Ratio::ZERO` and + breaks before moving when the fixed parts already exceed the room, so `at` + is never negative and a larger weight is never a shorter length. Recorded so + the next reader does not derive it again. +- **`Bound::outside`'s assertion that a floor does not sit over a cap fires + only for some boxes.** With `min` 100 and `max` 50 it fires where the box is + under 100 and passes where it is over, because the floor only binds on one + side. Left: the two ends can be a fraction and a pixel length, so which is + larger is not a question `set_min_len` can answer, and the box is where it + becomes one. +- **A bound has no remover.** `set_min_len`/`set_max_len` only ever write + `Some`, and clearing one means writing the whole rule -- which drops the + preferred length, the mirror of the defect fixed above. Left because + nothing asks for it yet and the preferred length has no remover either; the + asymmetry is uniform. +- **`Moves::clear` replaces its arena rather than clearing it**, throwing away + capacity where `RequestArena::reset` deliberately keeps it. Left: it runs + only on a full redraw, and `Arena` has no `clear`. +- **`TextEditCtx::apply_event` reads `if undo && let Some(..)` with an `else + if` after it**, so an undo with an empty history now falls into the branch + that pushes history. It is equivalent only because an undo command does not + itself change the text, so the `!=` guarding that push is false. Left as + correct; noted because the guard is somebody else's invariant. + +Verified at `cbccfb6`: format, workspace clippy under `-D warnings` with and +without `layout-diagnostics`, 208 ordinary and 212 diagnostic tests (207 and +211 before, plus the one this adds), and the cold dump byte-identical to +`f48e04e` across all **34,986** boxes. The three seed scans were not run: +nothing here can move a box, which the dump confirms. + ## Twelfth sweep: the request arena (2026-09-20) Over `05e6ced`, which no earlier round reviewed -- one node type for a