From b788cd1382e5ecd43278cee4ba81faa58ebca220 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sun, 20 Sep 2026 21:35:36 -0400 Subject: [PATCH] Record the twelfth sweep, over the request arena Co-Authored-By: Claude Opus 5 --- docs/HANDOFF.md | 26 ++++++++++++++- docs/LAYOUT_LOG.md | 79 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 598a754..3ab20d4 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -53,6 +53,23 @@ 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 quality sweep of the request arena is submitted on PR #19 + +The twelfth sweep landed at `f48e04e`. It is the first review of `05e6ced` -- +one node type for a request, an arena per owner, and `Widgets::edit_bound`. +Four findings, the largest being that the one path copying a request into +another arena was run by nothing: all 206 tests passed with a `panic!` in it, +because every expression in the corpora and in the cases compares an +expression against a plain length. A fixture with absolute geometry and a +fourth arm in the deferred corpus cover it now, and the path was already +correct. The other three: `Debug` on a request still printed the derived +arena its own new `Display` was written to avoid, a method asked nothing of +its receiver, and a closure parameter shadowed the rule it was called on. +`docs/LAYOUT_LOG.md` has all four, with the four rules it tripped and left. +Verified: format, clippy both ways, 207 and 211 tests, the cold dump +byte-identical to `05e6ced` across all 34,986 boxes, and 400 depth-5 trees in +each of the three deferred corpora in 200.95s. + ## A quality sweep of the bounds work is submitted on PR #19 The eleventh sweep landed at `ea1f836`. It is the first review of `2ac0843` @@ -184,6 +201,8 @@ comparison work above, its review rounds past `cadfba0` are described in - **One node type for a request**, `05e6ced` -- the expression a rule holds and the expression the pass allocates were two shapes of one thing, with 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. 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 @@ -239,7 +258,7 @@ the name it is reachable by. **Always**, because they cost nothing: format, workspace clippy under `-D warnings` with and without `layout-diagnostics`, the workspace tests, and the **cold dump**. `layout_dump` over 400 depth-5 trees is **34,986** boxes -since `76aaf06` grew bounds in the trees, and is unchanged through `05e6ced`. +since `76aaf06` grew bounds in the trees, and is unchanged through `f48e04e`. It was 34,571 from `2dba90b`, which grew the images, and 34,488 before those; the fourth through eighth sweeps all repeated 34,492, which is a `wc -l` of the whole run rather than of its box lines, so count the lines that are a box (`grep -cE '^[0-9]+ [0-9]+ '`). It is the only thing that catches two @@ -271,6 +290,11 @@ a compile error, which reads exactly like a fuzzer failure. adds a third: where a round fixed a class of defect one instance at a time, put the check in the shared helper, which both closes the class and tests the instances that round said were already fine. + The twelfth adds a fourth, about coverage rather than about a defect: a + branch only a combination of the public API reaches can be reached by + 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. 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 5a49ff0..a5729dd 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,85 @@ 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`. +## Twelfth sweep: the request arena (2026-09-20) + +Over `05e6ced`, which no earlier round reviewed -- one node type for a +request, an arena per owner, and `Widgets::edit_bound`. 316 inserted lines +over four files. Four findings, all in `f48e04e`. + +- **The one path that copies a request into another was run by nothing.** + `SizeRequest::join` grafts the other side's nodes into this side's arena, + which happens only where both sides of a comparison are expressions -- + `a.min(b).max(c.min(d))`, or a `clamp` whose ends are expressions. + `random.rs` builds only lengths and bounds, and every expression in the + three deferred corpora and in `cases/deferred.rs` compares an expression + against a plain length, so nothing reached it: measured, all 206 tests pass + with a `panic!` in that arm. It is where a missed renumbering would be + silent, because an operand copied without remapping still names a node that + exists -- just the wrong one. There is now a fixture at two window widths + with absolute geometry (`min(1 leftover, 40)` against + `max(min(2 leftover, 70), 10)`, which is 60 of 90 and 70 of 300), and a + fourth arm in `deferred_requests_agree_warm_and_cold` that puts an + expression on both sides across the whole corpus. Both were checked to + reach the arm by instrumenting it again. The path is correct as written: + this is a guard, not a fix. +- **The derived `Debug` the commit rejected was still what every `{:?}` + printed.** `SizeRequest` grew a `Display` because "a derived `Debug` of an + arena is not something a tree can be rebuilt from", and `describe` moved + onto it -- but `Debug` stayed derived, so the four `assert_eq!`s in + `cases/deferred.rs`, the only place a request is compared, print the arena + on failure, which is the case a reader has. Measured, `leftover(1).min(40)` + as derived `Debug` is `Expr(Expr { nodes: Nodes([Node { op: Min, a: + Linear(LayoutLen { px: 0, rel: 0, leftover: 1 }), b: Linear(LayoutLen { px: + 40, rel: 0, leftover: 0 }), leftover: true }]), root: 0 })` -- 179 + characters for what `Display` writes as `min(1 leftover;, 40 px;)`. That is + the class the eleventh sweep found one commit earlier and fixed at the call + site in hand. `Debug` forwards to `Display`, so the reason given for one + now governs both. +- **A method that asked nothing of its receiver.** `Nodes::linear(&self, at: + Operand)` never touched the arena, and `Nodes::leftover` beside it does, so + the pair reads as though both answers depend on it. It is `Operand`'s + question, the way `RequestedLen::linear` is `RequestedLen`'s; `combine` + opens `if let (Some(x), Some(y)) = (a.linear(), b.linear())`. +- **A closure parameter shadowing what it was called on.** `describe`'s + `|r| format!("{r}")` sits inside `let rule = |r: &SizeRule|`, so `r` means + two things four lines apart. It is `ToString::to_string`. + +Four things the sweep **looked at and left**: + +- **`RequestedLen`'s `leftover` repeats `Node`'s.** The same bit is stored on + the node and copied onto every handle made from it. Left because the handle + is the only form that leaves the arena and `Span` asks it where it has no + arena to ask -- `lens.iter().any(|len| len.has_leftover())` in + `position/span.rs` is widget code holding a slice of handles. Removing the + copy would push arena access into every widget that asks. +- **Nodes an arena can never reach.** `graft` copies a subtree and then + `combine` may fold the result away, which would leave the copy unreachable. + Worked through and it cannot happen: a fold discards an operand only where + both are `Operand::Linear` or the two are equal, and a grafted operand is + either a length or a fresh number that no existing node has, so it never + equals the side it is being combined with. Recorded so the next reader does + not derive it again. +- **`RequestArena::reset` reaches into `self.nodes.0`**, the one raw field + access outside `impl Nodes`. Left: a `clear` method is three lines to save + a `.0`, and `reset` must clear rather than replace, because the pass's + arena keeps its capacity across frames -- which is what the allocation rig + checks. +- **Three "compare, write, mark redraw" bodies in `Widgets`.** `set_len` and + `edit_bound` join `set_size_rule`, `set_alignment` and `set_region_node` in + writing that shape out. Unifying the first two needs a before-and-after + comparison of the whole rule, which is the clone this commit removed; the + duplication is the price of not having it. + +Verified at `f48e04e`: format, workspace clippy under `-D warnings` with and +without `layout-diagnostics`, 207 ordinary and 211 diagnostic tests (206 and +210 before, plus the new fixture), the cold dump byte-identical to `05e6ced` +across all **34,986** boxes, and 400 depth-5 trees in each of the three +deferred corpora in 200.95s. The corpora were run because one of them +changed; the three seed scans were not, because nothing here can move a box +-- a `Debug` impl, a method moved between two types, and tests. `bounds_cost` +still prints `rule_bytes=40`. + ## One node type for a request, and no refcount (2026-09-20) `05e6ced`. `SizeRequest` was a second expression shape beside the one the