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 <noreply@anthropic.com>
This commit is contained in:
1 parent
b788cd1382
commit
e397680a19
2 files changed
+130
-1
No files matched your search
@@ -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
|
||||
|
||||
Reference in new issue
Block a user