Record the twelfth sweep, over the request arena

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
iris-aiandClaude Opus 5 committed 2026-09-20 21:35:36 -04:00
1 parent 338dd00f76
commit b788cd1382
2 files changed
+104 -1

No files matched your search

+25 -1
View File
@@ -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 the three deferred corpora, 206 and 210 tests, and the cold dump byte-identical
to `2ac0843` across all 34,986 boxes. 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 ## 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` 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 - **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 and the expression the pass allocates were two shapes of one thing, with
the fold written twice and an `Arc` where nothing shares. 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 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 "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 **Always**, because they cost nothing: format, workspace clippy under
`-D warnings` with and without `layout-diagnostics`, the workspace tests, and `-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 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 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 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 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 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 time, put the check in the shared helper, which both closes the class and
tests the instances that round said were already fine. 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.** 2. **A review of everything written before the review gate existed.**
`pre-submit-review` and the rule that nothing is submitted unreviewed `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 arrived on 2026-09-13, well after the Rust port and most of Iris were
+79
View File
@@ -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 it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
the current plan is in `docs/HANDOFF.md`. 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) ## One node type for a request, and no refcount (2026-09-20)
`05e6ced`. `SizeRequest` was a second expression shape beside the one the `05e6ced`. `SizeRequest` was a second expression shape beside the one the