Wait for the scroll range the assertion needs #134

Merged
logan merged 1 commits from fix/133-album-dropdown-scroll-race into main 2026-08-19 16:03:17 +00:00
Collaborator

album-dropdown.spec.ts's scroll test guarded on a smaller number than it
asserted, so it proceeded on a layout that was still settling.

scrollable: sc.scrollHeight > sc.clientHeight + 40   // the guard
expect(await scrollTop(app)).toBe(80);              // the assertion

Any range in 41–79 satisfies the first and cannot satisfy the second, and the
grid passes through exactly that while it recomputes its columns after a
viewport change. The reported failure was Expected: 80, Received: 10.

Why it surfaced now

It failed CI on #112, which changes one paragraph of CLAUDE.md and nothing
else — and WebKit passed in the same run, which is what ruled the change out.

#132 made it reachable rather than broke it. The queue panel's mode is
measured rather than media-queried (deliberately — the panel's width is user
state and a media query cannot see it), so a viewport change at 900px costs one
more ResizeObserver → update → layout pass, and cover-grid settles after it
instead of before. I checked the settled state is correct and not a defect:
main panel 700px, scroll range 330, stable across 1.2 s of sampling, panel
correctly display: none while closed and overlaid.

tree runs failures
3607fe4 (before #132) 6 0
4e3c953 (after #132) 9 2
this branch 10 0

The change

The guard now waits for the range the assertion depends on, and both read one
SCROLL_TARGET constant so they cannot disagree again. It is strictly
stronger
than the old predicate — it cannot pass anywhere the old one failed
— so this waits for the right condition rather than weakening the test, and it
still guards the original defect (a .grid-scroll-container that never scrolls
never reaches the range, and the poll fails).

Verification

  • The spec file 10× against a running app — 0 failures.
  • e2e npx tsc --noEmit — clean.

Closes #133

`album-dropdown.spec.ts`'s scroll test guarded on a smaller number than it asserted, so it proceeded on a layout that was still settling. ``` scrollable: sc.scrollHeight > sc.clientHeight + 40 // the guard expect(await scrollTop(app)).toBe(80); // the assertion ``` Any range in 41–79 satisfies the first and cannot satisfy the second, and the grid passes through exactly that while it recomputes its columns after a viewport change. The reported failure was `Expected: 80, Received: 10`. ## Why it surfaced now It failed CI on **#112**, which changes one paragraph of `CLAUDE.md` and nothing else — and WebKit passed in the same run, which is what ruled the change out. **#132 made it reachable rather than broke it.** The queue panel's mode is measured rather than media-queried (deliberately — the panel's width is user state and a media query cannot see it), so a viewport change at 900px costs one more ResizeObserver → update → layout pass, and `cover-grid` settles after it instead of before. I checked the settled state is correct and not a defect: main panel 700px, scroll range **330**, stable across 1.2 s of sampling, panel correctly `display: none` while closed and overlaid. | tree | runs | failures | |---|---|---| | `3607fe4` (before #132) | 6 | 0 | | `4e3c953` (after #132) | 9 | 2 | | this branch | **10** | **0** | ## The change The guard now waits for the range the assertion depends on, and both read one `SCROLL_TARGET` constant so they cannot disagree again. It is **strictly stronger** than the old predicate — it cannot pass anywhere the old one failed — so this waits for the right condition rather than weakening the test, and it still guards the original defect (a `.grid-scroll-container` that never scrolls never reaches the range, and the poll fails). ## Verification - The spec file 10× against a running app — 0 failures. - `e2e` `npx tsc --noEmit` — clean. Closes #133
logan added 1 commit 2026-08-19 15:51:11 +00:00
test(e2e): wait for the scroll range the assertion needs
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m33s
CI / e2e (pull_request) Successful in 6m22s
90ac6e0825
The guard polled for `scrollHeight > clientHeight + 40` and the next
line asserted the container could be scrolled to 80, so any range in
41-79 satisfied the precondition and could not satisfy the assertion.
The grid passes through exactly that while it settles, because it
recomputes its columns after a viewport change rather than during it,
so the test read a clamped scrollTop and reported 10 against 80.

It failed CI on a pull request that changes one paragraph of CLAUDE.md
and nothing else, while WebKit passed in the same run. Reproduced
locally: 0 failures in 6 runs before #132, 2 in 9 after, 0 in 10 with
this change.

#132 is what made it reachable rather than what broke it. The queue
panel's mode is measured rather than media-queried, so a viewport
change at this width costs one more layout pass, and cover-grid settles
after it instead of before. The settled range is 330 and stable, the
main panel is 700px, and the panel is correctly display:none while
closed — there is no user-visible defect, only a wider window for a
race the spec already had.

A threshold below the value its caller depends on is not a guard, so
the target is one constant that both the guard and the assertion read.

Closes #133
logan merged commit e07f248cc8 into main 2026-08-19 16:03:17 +00:00
logan deleted branch fix/133-album-dropdown-scroll-race 2026-08-19 16:03:18 +00:00
Sign in to join this conversation.