Fuse the scroll guard and the write it guards #153

Merged
logan merged 1 commits from fix/151-fuse-the-scroll-guard-and-the-write into main 2026-08-20 03:58:04 +00:00
Collaborator

album-dropdown's the grid it opens in can be scrolled failed twice
over two sessions with Expected 80, Received 10 — both times on
branches that could not have caused it (one changed a paragraph of
CLAUDE.md; the other added a spec that runs later). #133 fixed the
guard; this fixes the shape.

The argument

#133 strengthened the guard from "scrollable at all" to "has at least
the range the assertion needs". That was necessary and cannot be
sufficient, for a structural reason rather than a threshold one: the
guard and the write are separate CDP round trips, so the page is free to
re-lay-out between them. Polling harder cannot close a window between
two moments.

Measured

Sampling scrollHeight - clientHeight every frame across the spec's own
1440×900 → 900×600 resize, three runs, identical each time:

t (ms) range
0 0
1 88
8–14 330 (settled)

88 satisfies a guard asking for 80 and is not the settled value. The
guard is therefore capable of passing on a layout one pass from done.
Under full-suite load the transient is worse — the observed failures
read 10 — which also explains where this lives: 0 failures in 10
consecutive runs of the file alone, both before and after this change.

Isolation is not where it reproduces, so isolation cannot be the
verification either.

The fix

scrollTo() sets scrollTop and returns scrollTop in one
page-side call, and expect.poll retries that. The assertion is now
about what the grid did rather than about what it was ready to do,
and there is no window between deciding and doing.

scrollRange() survives as a reporter rather than a gate — it is what
says why the target was not reached when it is not.

The sweep #133 asked for

Its last line asked for the other viewport-shrinking specs to be checked
for the same shape. Six specs resize; four assert through Playwright
locators, which retry and have actionability built in. One more had it:
layout-overflow's sidebar probe already fused its scroll and its
measurement into a single evaluate but ran that once, so it read
whatever the sidebar happened to be doing after the resize. Polled now —
safe to repeat, because scrolling to the bottom twice is scrolling to
the bottom.

Verification

  • make e2e — 178 passed, twice against one make dev-headless
  • 10 consecutive runs of album-dropdown.spec.ts: 0 failures (and 0
    before, which is the honest reading — see above)
  • npx tsc --noEmit in e2e/
  • The measurement above, taken with a throwaway probe spec that resized
    and sampled in the same test; not committed

Closes #151

`album-dropdown`'s *the grid it opens in can be scrolled* failed twice over two sessions with `Expected 80, Received 10` — both times on branches that could not have caused it (one changed a paragraph of `CLAUDE.md`; the other added a spec that runs later). #133 fixed the guard; this fixes the shape. ## The argument #133 strengthened the guard from "scrollable at all" to "has at least the range the assertion needs". That was necessary and **cannot** be sufficient, for a structural reason rather than a threshold one: the guard and the write are separate CDP round trips, so the page is free to re-lay-out between them. Polling harder cannot close a window between two moments. ## Measured Sampling `scrollHeight - clientHeight` every frame across the spec's own 1440×900 → 900×600 resize, three runs, identical each time: | t (ms) | range | |---|---| | 0 | 0 | | 1 | **88** | | 8–14 | 330 (settled) | **88 satisfies a guard asking for 80 and is not the settled value.** The guard is therefore capable of passing on a layout one pass from done. Under full-suite load the transient is worse — the observed failures read 10 — which also explains where this lives: **0 failures in 10 consecutive runs of the file alone, both before and after this change.** Isolation is not where it reproduces, so isolation cannot be the verification either. ## The fix `scrollTo()` sets `scrollTop` and returns `scrollTop` in **one** page-side call, and `expect.poll` retries that. The assertion is now about what the grid *did* rather than about what it was *ready to do*, and there is no window between deciding and doing. `scrollRange()` survives as a reporter rather than a gate — it is what says *why* the target was not reached when it is not. ## The sweep #133 asked for Its last line asked for the other viewport-shrinking specs to be checked for the same shape. Six specs resize; four assert through Playwright locators, which retry and have actionability built in. One more had it: `layout-overflow`'s sidebar probe already fused its scroll and its measurement into a single `evaluate` but ran that once, so it read whatever the sidebar happened to be doing after the resize. Polled now — safe to repeat, because scrolling to the bottom twice is scrolling to the bottom. ## Verification - `make e2e` — 178 passed, **twice** against one `make dev-headless` - 10 consecutive runs of `album-dropdown.spec.ts`: 0 failures (and 0 before, which is the honest reading — see above) - `npx tsc --noEmit` in `e2e/` - The measurement above, taken with a throwaway probe spec that resized and sampled in the same test; not committed Closes #151
logan added 1 commit 2026-08-20 03:38:06 +00:00
test(e2e): fuse the scroll guard and the write it guards
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 8m3s
ddd04623f7
`album-dropdown`'s "can be scrolled" failed twice over two sessions with
`Expected 80, Received 10`, both times on a branch that could not have
caused it. #133 strengthened the guard from "scrollable at all" to "has
the range this assertion needs", which was necessary and cannot be
sufficient: the guard and the write are separate round trips, so the
page re-lays-out between them.

Measured every frame across the resize, three runs: the range goes 0 →
**88** at 1ms → 330 settled by 8-14ms. 88 satisfies a guard asking for
80 while the grid is still a pass from done, so the guard is capable of
passing on a layout that is about to move. Under full-suite load the
transient is worse — the observed failures read 10 — which is why this
shows up on the second run of a suite and not in ten consecutive runs
of the file alone (0/10 before the change and after it; isolation is
not where this lives).

So the probe sets `scrollTop` and returns what it reads back, in one
page-side call, and the poll retries that. The assertion is now about
what the grid did rather than about what it was ready to do, and there
is no window between deciding and doing for anything to happen in.

#133's own last line asked for the other viewport-shrinking specs to be
swept for the same shape. One had it: `layout-overflow`'s sidebar probe
already fused its scroll and its measurement into one evaluate but ran
it once, so it read whatever the sidebar happened to be doing after the
resize. It is polled now — safe to repeat, because scrolling to the
bottom twice is scrolling to the bottom.

Closes #151
logan merged commit 7d348f243a into main 2026-08-20 03:58:04 +00:00
Sign in to join this conversation.