diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index 9054e4e..f85c171 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 `b7b8d09`, -and past the reviewed `cadfba0` it is eight rounds, each described in +**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 `docs/LAYOUT_LOG.md`: - **The repair**, `add6774` and `84dad21` -- collapsed-share placement, @@ -32,10 +32,16 @@ and past the reviewed `cadfba0` it is eight rounds, each described in two constants the shader and the CPU both count in written twice, and a `Scroll` positioning content the framework positions, which cost a redraw at the default alignment. +- **A sweep over the rigs, `Fixed`, and `b7b8d09` itself**, `f8aa0c5` -- the + other half of that same `Scroll` test, which could never decide it; a grid + conversion and its helper that only their own test called; `Len` + 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. -The settled design of the last three is in `docs/LAYOUT.md` under "Three -names, and the one argument that says them". Bryan settled the API over -2026-09-17 to 19; it is current, not frozen. +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 +over 2026-09-17 to 19; it is current, not frozen. The core design remains sound. Round-to-nearest is still unchanged. @@ -105,16 +111,22 @@ a compile error, which reads exactly like a fuzzer failure. 1. **Bryan's review of #19.** Fixes are themselves unreviewed code: repeat `pre-submit-review` over each round's changes, and apply the gate above to whatever each one touched. The ordinary oracle does not replace absolute - geometry and retained-primitive expectations. + 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. 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 - written, so all of that code went in unreviewed and none of the five - sweeps above covered more than the layout branch. It wants a pass of its - own (Bryan, 2026-09-20). The surface-texture defect in `02048ea` is the + written, so all of that code went in unreviewed and none of the sweeps + above covered more than the layout branch. It wants a pass of its own + (Bryan, 2026-09-20). The surface-texture defect in `02048ea` is the argument: nothing about that arm was hard, and it was written wrong anyway, which is what a first reader catches and a later sweep of some - other subject does not. + other subject does not. Three things it already has waiting, left out of + #19 because they are outside its diff: `Align::tuple` and both + `partial_align`s have no callers, `Vec2::align`/`partial_align` are + `UiVec2`'s with a conversion in front, and `impl_op!` carries four + grammars of which `core/src/util/vec2.rs` uses two, one line apart. 3. **Integrate the app's Iris capabilities before changing its pin.** `32f6ad8` has 45 commits not reachable from the review branch; shared UI ownership, richer masks, Android support, and app-side performance work diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index ccdcc48..b26fa29 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,117 @@ 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`. +## 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/`, +`core/src/fixed.rs`, `scripts/run-headless.sh` -- and once more over +`b7b8d09`, which was itself unreviewed because it was the sixth sweep's own +fix. Six findings, all in `f8aa0c5`. The cold dump over 400 depth-5 trees is +byte-identical to `b7b8d09` across all 34,492 boxes, and all three seed scans +pass (400 at depth 5 in 62.75s, 1,000 at depth 6 in 162.37s, 2,000 at depth 4 +in 305.25s). + +**A scroll still asking a question it has already answered.** The sixth +sweep found `anchor != Px::ZERO` in `Scroll`'s content test and removed it; +the operand beside it is the same defect and survived. `self.content_len` is +`answer_px.max(container_len)`, so content that fits has nothing to scroll +through, and `update_amt` on the line above clamps `amt` to +`content_len - container_len`, which is then zero. So in + + let content = match self.amt != Px::ZERO || self.content_len != self.container_len { + +the first disjunct can never decide the match: `amt != ZERO` implies +`content_len != container_len`, which the second already tests. It is now +`match self.content_len > self.container_len`, the exact complement of the +`content_len <= container_len` that guards the contract fifteen lines above. +Verified by asserting the implication in place and running the whole suite, +the scrolling tests included; it held. This is the lesson of "the fixes a +review produces are themselves unreviewed code" arriving on schedule -- one +round's fix left the same mistake in the expression it was editing. + +**Things nothing reads, in the new arithmetic.** `Fixed::to_scale` converts a +value between two fixed-point grids, and `shift_round` exists only to serve +it. Both arrived on this branch; the only caller either has ever had is +`a_coarser_grid_rounds_and_a_finer_one_does_not`, the test written for them. +Every other `Fixed` method has real callers (checked one by one), and nothing +needs a grid conversion: `ratio`, `mul` and `div` already cross grids where +layout has to. All three deleted. `to_scale` was also the one operation in +the file that could overflow silently without documenting it -- going to a +finer grid is `self.0 << (TO - SHIFT)` and the test only ever went coarse, +fine, coarse with a value small enough to survive the trip. + +**`Len` arithmetic written a component at a time.** `Len::align` built a +`Len` whose `px` is always `Px::ZERO`, then added to and subtracted from both +of its components by hand: + + start: Len::from_parts(at.rel.sub(self.rel.mul(rel)), at.px.sub(self.px.mul(rel))), + end: Len::from_parts(at.rel.add(self.rel.mul(rest)), at.px.add(self.px.mul(rest))), + +`Len::scale` is "both parts by the same fraction" and `Len` has `Add` and +`Sub`, so the whole rule is `at - self.scale(rel)` and +`at + self.scale(Rel::ONE.sub(rel))` -- the point the alignment names, less +the part of the length before it. Identical arithmetic, which the dump +confirms. It is the only place in the codebase that expanded a `Len` +operation like this; `LayoutLen::apply_leftover` touches `rel` alone, which +is genuinely one component. + +**A question asked through a value, one line from its sibling.** `445287c` +moved every method that answers a question about a value to `&self`. +`LayoutLen::without_leftover` was missed, and its own doc comment calls +`apply_leftover` -- which takes `&self` -- "the opposite reading of the same +value". Now `&self` too. Every other by-value method in the workspace that +does not return `Self` was checked: the rest are conversions on numbers +(`Fixed::raw`, `to_f32`) or builders. + +**A rig that scales a gesture against a mode the output no longer has.** +`run-headless.sh --mode` sets `out_w`/`out_h` beside the `swaymsg output ... +mode`, because those two numbers are the extent `replay-touch` passes to +`zwlr_virtual_pointer_v1::motion_absolute` -- the recording's coordinates are +a fraction of them. `--resize`, added in `b7b8d09`'s round, changed the mode +and left the extent alone, so `--resize 800x600@60Hz --replay flick.touch` +replayed every sample at `x * 800 / 1920` and finished looking like a run +that worked. Both are one `set_mode` function now, so a third mode change +cannot get it wrong. Found by reading rather than by running: this VM has no +recorded gesture that also resizes, which is exactly why it went unnoticed. + +**A comment the plan/build split stranded.** `src/random.rs`'s "a row takes +the height it is given rather than its tallest child" sat above +`let gap = self.rng.below(3) as i32 * 4`, telling a reader that the line +consumes no randomness. It describes the `set_size_rules` that `98d4e98` +moved into `Build::kind`, and the line it was left above is one of the two +draws in the function. Moved to the rule it is about, and the +seed-stability half dropped: building a plan consumes no randomness at all +now, so there is nothing left for that clause to say. + +### Tripped a rule and left as it stands + +- `Align::tuple`, `UiVec2::partial_align` and `Vec2::partial_align` have no + callers anywhere. All three pre-date this PR and are outside its diff, so + they belong to the pre-review-gate sweep rather than to this branch. +- `Vec2::align`/`partial_align` are `UiVec2`'s two functions again with + `UiVec2::from(*self)` in front. Also pre-existing, and merging them means + deciding whether `Vec2` should have them at all. +- `impl_op!` has four grammars for one macro, and `core/src/util/vec2.rs` + spells two of them one line apart -- `impl_op!(impl Add for Vec2: add x y)` + beside `impl_op!(Vec2 Sub sub; x y)`. The `impl ... for ...:` arm has that + one caller. Pre-dates this PR; the two arms this PR added (`same ...`) are + a real distinction, since a type mixing a fraction and an offset has no + meaning for a bare `f32`. +- `AxisAlign`, `RegionAlign` and `Align` lost `Eq` when `AxisAlign` became a + `Rel` wrapper, which `Rel` derives. Nothing needs it, so it was left rather + than adding a derive with no reader. +- `Holds::through` special-cases a fully unbounded range before inverting a + fraction, which the general path would also answer correctly through + `narrow`. Left: it is the one place a `Px::MIN`-to-`Px::MAX` interval is + shifted and divided, and the early return says that no fraction can narrow + "every length". +- `Scroll`'s `content_len <= container_len` can only be equality, given the + `max` that built it. Left: `<=` reads as "the content fits", which is what + the branch means, and the mirror `>` now reads as "it overflows". +- `scenario::Case::name`, `window` and `Shuffle::of` take `self` on `Copy` + enums. They are test-rig code the `&self` pass did not cover, and an enum + with no fields is the one place taking a value costs a caller nothing. + ## Sixth sweep: the shader boundary and the position widgets (2026-09-20) Over what the five earlier rounds did not name -- the WGSL prelude and how