Repository navigation
Home: the hero-to-grid dive reports as document motion - #188
Conversation
The third review finding on #187, and the largest reveal the application performs. `snap` travels 0 -> 1, so the spring's own velocity is a couple of FRACTIONS per second; `layout_grid` turns that same fraction into `GRID_TOP_Y - PEEK_Y` = 617 px of vertical travel for every shelf on screen. Both the grid's `scroll_y` and every shelf's `scroll_x` can hold perfectly still through the dive, so `scrolling_fast()` stayed false right across it and admitted the poster work the gate exists to defer. Reporting `snap.vel` would not have fixed it -- the units are wrong, and it reads as a still document. What is reported is the displacement the layout actually realises, which is also why `grid_origin` is now a function: the motion report and `layout_grid` read one expression, so a later change to the dive's geometry cannot leave the report describing the old one. Test observed red first: the spring's own velocity is asserted under 10 (it measures ~2) in the same frame the realised dive has to read as fast, so a future attempt to report the raw velocity fails here rather than silently disarming the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2327eb645
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P1 on #188. Sharing one expression between `layout_grid` and the motion report reassociated `(top + flow) - scroll * snap` into `(top - scroll * snap) + flow`. Those are not bit-identical in f32 -- 26% of realistic (snap, scroll, row) triples differ -- and `base_y` is serialised into `LogicalState` through `CardRow::write_motion`, so every committed replay recording that scrolls the Home grid would have hash-diverged. Nothing in `make check` or CI replays those recordings: `test_harness.py` uses the fixture NAMES against a fake simulator in a temp dir and never loads `1-boot-home-chip-grid` itself. So the green gates on the previous commit were blind to this, and it would have surfaced only when someone re-recorded. `layout_grid` is byte-identical to `main` again. `grid_origin` stays, for the motion report alone, and the two are now pinned by a test rather than by being the same line: at row 0 `flow` is 0.0, so the two forms ARE bit-identical, and the guard asserts that on `to_bits()` across five mid-dive positions. That keeps the drift protection the shared expression was meant to give without touching the arithmetic the recordings hash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4863c652e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 on #188. `Grid::eff_scroll` is `scroll_x * snap`, so the dive scales a shelf's RETAINED horizontal offset by the same fraction it scales the vertical travel. Returning to the hero from the far right of a row sweeps that row's cards the full offset across the screen while the shelf's own spring sits perfectly still and reports nothing. The two axes do not fall under the threshold together: at snap 0.9 the vertical term measures 88 px/s -- already settled as far as the gate is concerned -- while a 4000 px offset swept by the same step is ~570 px/s. So the late dive was a fast reveal reading as a still document, which is the same failure the gate was built to prevent, one axis over. Both axes are now reported and `note_scroll` keeps the larger, so the gate answers from the document's fastest axis rather than from whichever one happens to be plumbed. The test is an A/B on the horizontal term alone: identical frame, identical snap position, differing only in whether a shelf carries an offset, with the no-offset half asserting the vertical term really is settled so the test cannot pass for the wrong reason. Observed red first. Note for the next reader: `restore_scroll` clamps to the item count the CALLER passes, and the Home fixture's rows hold three elements, which fit on screen. The first draft of this test therefore clamped its offset to 0 and proved nothing while passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third review finding on #187, landed separately because #187 had already merged.
snaptravels 0 → 1, so the spring's own velocity is a couple of fractions per second.layout_gridturns that same fraction intoGRID_TOP_Y - PEEK_Y= 617 px of verticaltravel for every shelf on screen. The grid's
scroll_yand every shelf'sscroll_xcan holdperfectly still through the dive, so
scrolling_fast()stayed false right across the largestreveal of poster art the application performs — and admitted exactly the work the gate exists
to defer.
Reporting
snap.velwould not have fixed it: the units are wrong and it reads as a stilldocument. What gets reported is the displacement the layout actually realises. That is also
why
grid_originis now a function — the motion report andlayout_gridread one expression,so a later change to the dive's geometry cannot leave the report describing the old one.
Verification
but the dive it realises is hundreds of px/s and must read as fast)realised dive must read as fast, so a future attempt to report the raw velocity fails here
instead of silently disarming the gate
cargo test --lib4034 passed; 0 failed; 1 ignoredcargo check --lib --no-default-featuresclean,make lintclean🤖 Generated with Claude Code