yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

album-dropdown 'can be scrolled' is racy: its guard waits for 40px and it asserts 80 #133

New Issue
Closed
opened 2026-08-19 15:49:19 +00:00 by logan · 0 comments
logan commented 2026-08-19 15:49:19 +00:00
Collaborator
Copy Link
Copy Source

Report

album-dropdown.spec.ts:91 — "the grid it opens in can be scrolled" — fails
intermittently on chromium with:

Expected: 80
Received: 10

It failed CI on PR #112, which changes one paragraph of CLAUDE.md and
nothing else, so it cannot be the cause. WebKit passed in the same run.

Findings

Reproduced locally against a make dev-headless SEED=default app, running the
spec file repeatedly:

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

The test's precondition is weaker than its assertion. scrollRange()
reports scrollable as scrollHeight > clientHeight + 40, the poll waits for
that, and the next line asserts the container can be scrolled to 80. A grid
whose scroll range is anywhere in 41–79 satisfies the guard and cannot satisfy
the assertion, so the test proceeds on a layout that is still settling and then
reads a clamped value.

The settled range at 900×600 is 330, measured on the running app and stable
across 1.2 s of sampling — so there is no shortage of scroll range, only a race
against the grid reaching it. At the default viewport the range is 0, which is
why the test shrinks the window first; the transient between the two is what it
lands in.

Why #132 made it reachable. The queue panel's mode is now measured rather
than media-queried, so a viewport change at this width runs
ResizeObserver → Lit update → display change on the panel, and cover-grid
recomputes its columns after that rather than before it. The settled result is
identical (main panel 700px, range 330, panel correctly display: none while
closed and overlaid), so this is not a user-visible defect — it is one more
layout pass at exactly the width the spec picked, which widens a window the
spec was already racing.

That extra pass is inherent to computing the mode instead of breakpointing it,
which is deliberate and argued in plan 018 — the panel's width is user state
and a media query cannot see it. So the test is what should change.

Direction

Poll for the condition the assertion actually needs — a scroll range of at
least the 80 it is about to ask for — rather than for "scrollable at all".
Deriving the target from the measured range would also work and is less
brittle than either constant.

Worth checking the other two viewport-shrinking specs for the same shape: a
guard whose threshold is smaller than the value the assertion depends on is
not a guard.

**Report** `album-dropdown.spec.ts:91` — "the grid it opens in can be scrolled" — fails intermittently on chromium with: ``` Expected: 80 Received: 10 ``` It failed CI on PR #112, which changes **one paragraph of `CLAUDE.md`** and nothing else, so it cannot be the cause. WebKit passed in the same run. **Findings** Reproduced locally against a `make dev-headless SEED=default` app, running the spec file repeatedly: | tree | runs | failures | |---|---|---| | `3607fe4` (before #132) | 6 | 0 | | `4e3c953` (after #132) | 9 | 2 | **The test's precondition is weaker than its assertion.** `scrollRange()` reports `scrollable` as `scrollHeight > clientHeight + 40`, the poll waits for that, and the next line asserts the container can be scrolled to **80**. A grid whose scroll range is anywhere in 41–79 satisfies the guard and cannot satisfy the assertion, so the test proceeds on a layout that is still settling and then reads a clamped value. The settled range at 900×600 is **330**, measured on the running app and stable across 1.2 s of sampling — so there is no shortage of scroll range, only a race against the grid reaching it. At the default viewport the range is 0, which is why the test shrinks the window first; the transient between the two is what it lands in. **Why #132 made it reachable.** The queue panel's mode is now *measured* rather than media-queried, so a viewport change at this width runs ResizeObserver → Lit update → `display` change on the panel, and `cover-grid` recomputes its columns after that rather than before it. The settled result is identical (main panel 700px, range 330, panel correctly `display: none` while closed and overlaid), so this is not a user-visible defect — it is one more layout pass at exactly the width the spec picked, which widens a window the spec was already racing. That extra pass is inherent to computing the mode instead of breakpointing it, which is deliberate and argued in plan 018 — the panel's width is user state and a media query cannot see it. So the test is what should change. **Direction** Poll for the condition the assertion actually needs — a scroll range of at least the 80 it is about to ask for — rather than for "scrollable at all". Deriving the target from the measured range would also work and is less brittle than either constant. Worth checking the other two viewport-shrinking specs for the same shape: a guard whose threshold is smaller than the value the assertion depends on is not a guard.
logan added the Area/Library-UIKind/Testing
Priority
Medium
3
Reviewed
Confirmed
1
 labels 2026-08-19 15:49:19 +00:00
logan closed this issue 2026-08-19 16:03:17 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Area/Library-UI Kind/Testing
Priority
Medium
3
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#133