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

album-dropdown 'can be scrolled' still races: the guard and the write are separate round trips #151

New Issue
Closed
opened 2026-08-20 02:57:23 +00:00 by logan · 1 comment
logan commented 2026-08-20 02:57:23 +00:00
Collaborator
Copy Link
Copy Source

Report

album-dropdown.spec.ts — the grid it opens in can be scrolled —
still fails intermittently with the exact signature #133 was filed and
closed for:

Expected: 80
Received: 10

Observed on branch fix/43-queue-panel-selection, whose only changes
are a new spec file that runs after this one and a comment: green on
the first full-suite run, red on the second against the same app, and
green again when album-dropdown.spec.ts was run alone on that same
app immediately afterwards. So it is not leftover state — it is timing
inside the run.

Findings

#133's fix was to strengthen the guard, and the guard is now right:
scrollRange() reports room as scrollHeight - clientHeight >= SCROLL_TARGET, i.e. the poll waits for the exact 80 the assertion is
about to ask for. That closes the "41–79 satisfies the guard" hole it
was written for.

It cannot close the remaining one, because a guard and the action it
guards are two different moments. The sequence is:

  1. expect.poll(scrollRange) — a CDP round trip that observes range ≥ 80
  2. app.evaluate(sc.scrollTop = 80) — a second round trip
  3. scrollTop(app) — a third

The grid recomputes its columns after the resize rather than during it
(#133's own analysis, and the extra ResizeObserver pass #132 added at
this width), so the range passes through ≥ 80 and comes back down. Step
1 can be true and step 2 can still land on a container with 10px of
range. Polling harder cannot fix a sequence whose steps are separately
scheduled — only fusing them can.

Direction

Poll the outcome, not the precondition: one evaluate that sets
scrollTop and returns what it reads back, retried until it equals the
target (or until it stops changing). That is a single moment in the
page, so there is no window between deciding and doing.

The same shape is worth checking wherever a spec resizes and then
measures — a guard followed by a separate write is the pattern, not this
one grid. #133's last line says the same thing about thresholds; this is
the other half.

**Report** `album-dropdown.spec.ts` — *the grid it opens in can be scrolled* — still fails intermittently with the exact signature #133 was filed and closed for: ``` Expected: 80 Received: 10 ``` Observed on branch `fix/43-queue-panel-selection`, whose only changes are a new spec file that runs *after* this one and a comment: green on the first full-suite run, red on the second against the same app, and green again when `album-dropdown.spec.ts` was run **alone** on that same app immediately afterwards. So it is not leftover state — it is timing inside the run. **Findings** #133's fix was to strengthen the guard, and the guard is now right: `scrollRange()` reports `room` as `scrollHeight - clientHeight >= SCROLL_TARGET`, i.e. the poll waits for the exact 80 the assertion is about to ask for. That closes the "41–79 satisfies the guard" hole it was written for. **It cannot close the remaining one, because a guard and the action it guards are two different moments.** The sequence is: 1. `expect.poll(scrollRange)` — a CDP round trip that observes range ≥ 80 2. `app.evaluate(sc.scrollTop = 80)` — a *second* round trip 3. `scrollTop(app)` — a third The grid recomputes its columns after the resize rather than during it (#133's own analysis, and the extra ResizeObserver pass #132 added at this width), so the range passes through ≥ 80 and comes back down. Step 1 can be true and step 2 can still land on a container with 10px of range. Polling harder cannot fix a sequence whose steps are separately scheduled — only fusing them can. **Direction** Poll the **outcome**, not the precondition: one `evaluate` that sets `scrollTop` and returns what it reads back, retried until it equals the target (or until it stops changing). That is a single moment in the page, so there is no window between deciding and doing. The same shape is worth checking wherever a spec resizes and then measures — a guard followed by a separate write is the pattern, not this one grid. #133's last line says the same thing about thresholds; this is the other half.
logan added the Kind/Testing
Priority
Medium
3
Reviewed
Confirmed
1
Area/Library-UI labels 2026-08-20 02:57:23 +00:00
logan self-assigned this 2026-08-20 03:29:47 +00:00
logan added the
Status
In Progress
 label 2026-08-20 03:29:48 +00:00
logan commented 2026-08-20 03:29:48 +00:00
Author
Collaborator
Copy Link
Copy Source

Claiming this. Branch: fix/151-fuse-the-scroll-guard-and-the-write.

Approach. The Direction as filed: one evaluate that sets
scrollTop and returns what it reads back, polled until it equals the
target — so deciding and doing are the same moment in the page and there
is no window between them for the grid to settle out of.

Then the second half of #133's own last line, which is what left this
open: sweep the other viewport-shrinking specs for the same shape. A
guard in one round trip followed by a write in another is the pattern,
not this one grid.

Verified by running the spec file repeatedly rather than once — #133
reproduced at 2 failures in 9 runs, so a single green run says nothing.

Claiming this. Branch: `fix/151-fuse-the-scroll-guard-and-the-write`. **Approach.** The Direction as filed: one `evaluate` that sets `scrollTop` and returns what it reads back, polled until it equals the target — so deciding and doing are the same moment in the page and there is no window between them for the grid to settle out of. Then the second half of #133's own last line, which is what left this open: sweep the other viewport-shrinking specs for the same shape. A guard in one round trip followed by a write in another is the pattern, not this one grid. Verified by running the spec file repeatedly rather than once — #133 reproduced at 2 failures in 9 runs, so a single green run says nothing.
logan closed this issue 2026-08-20 03:58:04 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-20 03:58:16 +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 logan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#151