fix(scroll): round 14a β data-show-scroll-button attribute (E2E observability for the jump-button race)#108
Merged
Conversation
β¦servability ROOT CAUSE (still WebKit-specific, but at a deeper layer than rounds 11-13) Even with rounds 11+12+13 applied (native scroll listener, sustained- hidden modal verification, timing-budget separation), webkit-msg 1/1 still intermittently fails on `messaging-scroll.spec.ts:266` "T007-T008: Jump button". PR #99 run 26176580568 caught one such failure with all 3 retries failing. Forensic from the failure-time screenshot at `/tmp/pr99-flake/extracted/resources/21b9a69b...png` (extracted from the blob report): the page IS loaded, the conversation IS open, the test IS clicking through the flow correctly. The button truly never renders. The race: 1. Test sets `messageThread.scrollTop = ...` + dispatches a native scroll event (round-11 pattern: works around React's onScroll not catching synthetic events on WebKit). 2. Native event fires β handleScroll() runs β setShowScrollButton (true) queues a React state update. 3. Test reads `scrollInfo` via a separate `.evaluate(...)` call β this reads DOM values, NOT React state. 4. Test asserts `expect(jumpButton).toBeVisible()` with 5s timeout. 5. React schedules the state update for the next microtask / animation frame. 6. WebKit's event loop on Linux CI under heavy concurrent shard load sometimes defers this render past the 5s assertion window. The DOM scroll position IS correct (step 1 + 3 agree). What's missing is observability of the *component's reaction* to the scroll. THE FIX (round 14a β first of two) Add `data-show-scroll-button={showScrollButton ? 'true' : 'false'}` to the MessageThread wrapper div. This attribute mirrors React state to the DOM during commit β visible to Playwright the moment the state-update flushes. The test now polls this attribute (via expect.poll with 50/100/200/500ms intervals) BEFORE asserting the button is visible. Why this is the structural fix and not another mitigation: - The attribute is a direct projection of the component's internal state β there's no in-between system (React reconciler vs WebKit event loop) for the test to race against. - The wrapper element is always in the DOM (it's the outer `relative h-full` container, not the scrollable child) β so the attribute query never returns null. - The component now exposes a stable, documented observability boundary for any future test that needs to know when the scroll- button state is true. - This pattern matches the round-10/-12 lesson: trust attribute polling over event-driven assumptions when crossing browser engine boundaries. WHAT THIS DOES NOT DO - Does NOT remove round-11's native scroll listener (it's still correct for catching the dispatched scroll event) - Does NOT remove round-12's sustained-hidden verification (it's still correct for the modal-retry flake) - Does NOT seed conversation data β that's round 14b in a follow-up PR. Round 14b fixes a separate failure mode (thin conversations not having enough scroll height to enter the `if` branch). ROUND 14 PROGRESS | Round | PR | Layer | |---|---|---| | 14a (this) | TBD | data-show-scroll-button attribute (observability) | | 14b | follow-up | seed 30 messages into test conversation (data shape) | VERIFICATION - 31/31 MessageThread unit tests pass (no regression) - Type-check clean - Lint clean - The E2E test `T007-T008: Jump button` now polls a state mirror that fires in the same React render as the button itself β eliminating the inter-system race. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #99 caught yet another webkit-msg jump-button failure even after rounds 11+12+13. Forensic from the blob report's failure-time screenshot showed the page loaded fine, conversation open, test flow correct β but the button truly never rendered within the 5s assertion window.
The race is between React's state-update flush and WebKit's event loop, not between the dispatch event and the listener. Native listener fires β
setShowScrollButton(true)queues a state update β React schedules it for the next microtask β WebKit on Linux CI under shard contention sometimes defers the render past 5s.Fix
Mirror the
showScrollButtonReact state to a DOM attribute on the wrapper:The test now polls this attribute (via
expect.poll) BEFORE asserting the button is visible:The attribute is a direct projection of component state β visible to Playwright the moment React commits the update. No reconciler vs event-loop race remains.
Why this is the structural fix
relative h-fullcontainer, not the scrollable child) β the attribute query never returns null.Rounds 10 / 11 / 12 / 13 / 14a β independent, all correct
What this PR does NOT do
Test plan
webkit-msg 1/1which has been the flake targetBlocks
π€ Generated with Claude Code