diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index f85c171..334bd3a 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -6,8 +6,8 @@ found is in `docs/LAYOUT_LOG.md`. ## The Iris layout repair is submitted -**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `f8aa0c5`, -and past the reviewed `cadfba0` it is nine rounds, each described in +**Iris PR #19** (`layout/one-ask`) replaces closed #18. The tip is `77ed7a2`, +and past the reviewed `cadfba0` it is ten rounds, each described in `docs/LAYOUT_LOG.md`: - **The repair**, `add6774` and `84dad21` -- collapsed-share placement, @@ -38,6 +38,12 @@ and past the reviewed `cadfba0` it is nine rounds, each described in arithmetic written a component at a time; a question asked through a value one line from its `&self` sibling; and `run-headless.sh --resize` leaving a replayed gesture scaled against the mode the output used to have. +- **A sweep over the tests and the seventh sweep's own fix**, `77ed7a2` -- + four shrunk fixtures naming one widget under three names and counting + each in the list of boxes the case compares, seven copies of a helper + sitting at the top of the same file, the GPU rigs' adapter probe written + twice, a mask resolved three times, a field nothing reads, a shrinker + claim its own assertion does not make, and three stale numbers. 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 @@ -93,9 +99,12 @@ 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,492 boxes, and -it is the only thing that catches two same-typed values being swapped, which -is the failure mode of a rename or a move. The repair moved 650 of those +the **cold dump**. `layout_dump` over 400 depth-5 trees is **34,488** boxes +-- counted again on 2026-09-20, the fourth through eighth sweeps having all +repeated 34,492, which is a `wc -l` of the whole run rather than of its box +lines -- and it is the only thing that catches two same-typed values being +swapped, which is the failure mode of a rename or a move. Count the lines +that are a box (`grep -cE '^[0-9]+ [0-9]+ '`), not the output. The repair moved 650 of those boxes, all from the collapsed-share correction; every commit since has been byte-identical to `84dad21`. @@ -114,6 +123,10 @@ a compile error, which reads exactly like a fuzzer failure. geometry and retained-primitive expectations. The seventh sweep is the evidence: reading `b7b8d09`, the sixth sweep's own fix, found that it had removed one dead operand from a test and left the one beside it. + The eighth adds a second lesson, about the tests themselves: a fixture a + fuzzer shrank is a regression test, so a round that rewrites one has to + prove the tree is the same rather than that the case still passes, since + a fixture that quietly changed still passes and covers nothing. 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 b26fa29..d921d99 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,125 @@ 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`. +## Eighth sweep: the tests, and the seventh sweep's fix (2026-09-20) + +Over the part no earlier round named -- the 6,300 lines under `tests/`, which +is more than half of what #19 adds -- and once more over `f8aa0c5`, which was +the seventh sweep's own fix and so unreviewed. Seven findings, all in +`77ed7a2`. No library code changed: the cold dump over 400 depth-5 trees is +byte-identical to `f8aa0c5` across all **34,488** boxes, and the seed scans +have nothing to find, since nothing that decides a layout moved. + +That count is not the 34,492 the four rounds before this one recorded, nor +the 34,490 `77ed7a2`'s own message gives. The dump prints eight lines that +are not a box -- cargo's two, libtest's four, and two blanks -- so a `wc -l` +of the run comes to 34,496 and any partial filter lands somewhere between. +The boxes are the lines matching `^[0-9]+ [0-9]+ `, and there are 34,488 of +them at both `f8aa0c5` and `77ed7a2`. What every round actually established +is still true, since each diffed two dumps rather than trusting a count. + +**A shrunk fixture that names one widget three times.** `width`, `sized` and +`align` set a rule on the widget they are handed and give its own id back; +only `pad` and `wrapper` make a new widget. So in + + let wrapped = wtext("Wrapping shapes").size(16).wrap(true).add(&mut h.rsc); + let sized = wrapped.width(76).add(&mut h.rsc); + let aligned = sized; + +all three names are one text, and all three went into the vector of ids the +case compares warm against cold. Measured: `plant` and `plant_fixed` list six +ids and hold four widgets, `plant_pair` lists four and holds three, +`plant_scrolled` lists eight and holds seven; the other nine fixtures are +honest. So four cases check fewer boxes than they say, and the doc comment of +each quotes the inflated count as the size of the tree the shrinker reduced +to -- which is the number a reader uses to judge whether a case is still the +minimal one. The names are gone and the counts are what the fixtures build. +`trace_unsettled.rs` carries a copy of two of these fixtures and had the same +aliases. + +The check that mattered here is that a rebuilt fixture is the same tree: a +regression test whose fixture quietly changed still passes and no longer +covers its defect. Each was diffed against its old self -- same widget slots, +same regions, for both settings of `swapped`. + +**A helper at the top of the file, and seven copies of its body below it.** +`assert_same_regions` collects every widget whose box moved and reports them +together. Six tests call it; seven more spell the eight lines out instead, +byte for byte. They call it now, and it took `#[track_caller]` so the panic +names the case rather than the helper. + +**The GPU rigs' adapter probe, written twice.** `draw_cost` and `chain_cost` +each held a `config` (identical) and an adapter probe (identical but for the +feature it asks for and what it returns). The probe leaks its `Instance` on +purpose -- a Vulkan loader may unload the driver as a test thread exits -- +and only `draw_cost` said so, with `chain_cost` referring the reader to it. +Both come from `tests/gpu/mod.rs` now, shared through `#[path]` the way +`scenario/mod.rs` already is, with the justification on the thing it is +about. + +**The mask a widget is clipped by, resolved three times**, two of them a +byte-identical closure defined inside a loop. `mask_bounds` takes the +`MaskIdx` rather than the widget, because the third site reads the slot it +saved before the frame: that a redraw keeps the same slot is exactly what it +is checking, and a helper that looked the slot up again would have made that +assertion pass for the wrong reason. + +**A field nothing reads.** `Layered::_revision` existed to be incremented, to +mark its widget dirty. Two tests in the same file already do that with +`widgets_mut().get_dyn_mut(id)`, which is the idiom `tests/scenario/mod.rs` +uses as well. The underscore was hiding the dead-code warning that would have +said so. + +**A claim the test below it does not make.** `plan.rs` said "Every +simplification is strictly smaller, so taking them in turn reaches a fixed +point instead of circling", and then asserted `small.size() <= node.size()`. +Measured: 53 of one tree's 101 simplifications keep the widget count, because +`Plan::size` counts widgets and dropping an alignment or stepping `Wrapped -> +OneLine` does not change it. The assertion is the right one and the claim was +not. What actually rules out circling is that those steps are one-way too -- +a `Some` becomes a `None`, a kind steps down a ladder with no way back up -- +and the comment now says that. The test is `no_simplification_of_a_plan_is_larger_than_it`. + +**Numbers and lines that had gone stale.** `generated.rs` says "Eight that +have never failed" and "the ten the others check"; both said one fewer, +having been written when `SEEDS` had nine entries and not updated when +`d8ae9c3` added seed 20. The `should_panic` scroll test ended in an +`h.frame()` that cannot run, because `Harness::set_root` lays the tree out +and is where the panic comes from -- verified by deleting it. Two +`drop(tree)` calls sat at the end of their own scope. + +### Tripped a rule and left as it stands + +- `env` is written four times under `tests/` (`layout_dump`, + `layout_diagnostics`, `revision_cost`, `scenario`). `revision_cost`'s is + deliberate and documented: that file is kept in the API subset an old + worktree also has, so it can be dropped in and measured there, and a + `#[path]` module would break that. Sharing the other three means either a + new file for six lines of `std` or pulling `scenario`'s 494 lines into two + more binaries. +- `determinism.rs`'s `BranchesOnMeasurement` is `iris::random::Branch` with + the same four fields and nearly the same body, and `unsettled.rs` beside it + uses the real `Branch`. The difference is load-bearing: the copy states no + contract, so the framework must re-ask it at every width, which is the + whole point of a test about whether a re-measure branches the same way. + `Branch` also has a `size_hint` that changes how a span treats it. +- Three tests call `h.frame()` immediately after `h.set_root`, which already + frames. Unlike the `should_panic` one, these are reachable and assert that + a settled second frame adds no draws. Left; the redundancy reads as noise + but removing it removes a check. +- `primitive_bounds` and `primitive_masks` share the walk from a widget's + primitives to their instances and differ only in the field they take. + Left: naming the intermediate means exporting the instance type into the + test, which is more coupling than the four shared lines are worth. +- `Harness::replay` ignores each sample's `t_ms`, and the parser rejects + time running backwards with a comment about the wait between samples -- + which is `replay-touch`'s behaviour, not the harness's. Left: the parser + is shared by both replays and the rule is the real one, but a harness + gesture has no timing, so nothing in-process can measure a fling velocity. +- `layout_diagnostics.rs`'s `report` takes `_harness` and reads it, because + it is only used under `layout-diagnostics`. An underscore on a parameter + the body uses is backwards, but the alternative is a `cfg_attr`ed allow. + ## Seventh sweep: the rigs, `Fixed`, and the sixth sweep's fix (2026-09-20) Over what no earlier round named -- `src/random.rs` and `tests/scenario/`,