test(ui-tools): fix the diagnosis-map hydration race, and capture the bistable mobile-/ CLS finding - #2214
Conversation
…fore clicking The dialog assertion failed on CI run 32460303619 with "element(s) not found" after a full 10s wait — the click had landed but nothing opened. `gotoLauncher` waits only for `#main-content`, so the trigger is present and clickable before React attaches its `onClick`, and a click inside that window is swallowed. This is a race, not a regression, and the runs prove it: `Production UI (3)` passed on head 8506db3 and failed on c56d12d, whose only delta was 26 ledger JSON files. Same code, different outcome. The file already carries `waitForReactEventHandler` with an `onClick` default and uses it before fifteen other clicks; this one call site simply predates that pattern. No assertion is weakened and nothing is skipped or quarantined — which would not have been permitted anyway on a single observation, under the three-reproductions-on-one-SHA rule in `tests/flake-ledger.json`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HoyoNN7nCUVDLaE84k6e6c
…and the concurrency follow-up Four inbox requests, none of which edit the canonical ledger: - P2 issue — mobile-/ Lighthouse CLS is bistable at 0.016 or 0.223 and fires only in CI. Records the five-run evidence, why PR #2199 cannot be the cause, and two negative local results: the attribution harness reports zero shifts, and a full local Lighthouse run with the same mobile emulation reports 0.000. Names the next step that needs no reproduction — read the mobile-root layout-shift debugdata out of a failing run's uploaded report artifact. - P3 task — re-adopt the document-viewer Linux visual baseline once PR #2199 lands. Blocked until then: adopting earlier would commit a golden for a state main does not have. - P3 task — watch runner usage now that every main push gets its own concurrency group (PR #2209). The in-file comment accepts the cost as a prediction; nobody has measured it. - update to #JVYQEM — marks it partially stale. Its phone remedy already landed (the token reads 10.125rem, not the 6.625rem the row records); the wide reserve is what remains, and it needs a mechanism rather than a bigger constant. Also warns against reading it as the cause of the 0.223 breach. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HoyoNN7nCUVDLaE84k6e6c
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:52 minutes Limit details: You’ve used the included review currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Summary
Follow-up to #2209, working the outstanding items from that session. Two commits.
646123a— fixed theui-toolsdiagnosis-map failure. It failed on CI run32460303619withexpect(getByTestId('diagnosis-map-dialog')).toBeVisible()→element(s) not foundafter the full 10s wait: the click landed but nothing opened.gotoLauncherwaits only for#main-content, so the trigger is present and clickable before React attaches itsonClick, and a click inside that window is swallowed. The file already carrieswaitForReactEventHandlerwith anonClickdefault and uses it before fifteen other clicks — this call site simply predates the pattern. No assertion weakened, nothing skipped or quarantined.It is a race and not a regression, and the runs prove it:
Production UI (3)passed on head8506db3and failed onc56d12d, whose only delta was 26 ledger JSON files. Quarantine would not have been permitted anyway on a single observation, under the three-reproductions-on-one-SHA rule.8607102— four inbox requests, none of which edit the canonical ledger.Verification
npm run verify:pr-localnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run: no product surface changes — one Playwright spec and four ledger inbox files. The changed spec cannot be executed here (
#255: Playwright pins chromium 1234, this image ships 1194, and forcing a mismatched binary is disallowed), so its proof is delegated to CI Production UI shard 3.npm run verify:release— not run: release confidence was not requested.Risk and rollout
646123a(test wait),8607102(ledger requests).Notes
On the mobile-
/CLS item — not fixed here, and deliberately so. I could not attribute it, and a blind fix would be worse than none. What is now established:8506db3passed clean whilec56d12dbreached at0.223, and the only delta between those heads was ledger JSON.DocumentViewer.tsxanddocument-overview-landing.tsx, andDocumentVieweris imported solely bydocuments/[id]/{page,loading}.tsx, so neither is in the client bundle for/.scripts/measure-cls-attribution.mjsreportedCLS=0.000 shifts=0, and a full localverify:lighthouse -- --keep— which applies Lighthouse's own mobile emulation and throttling — reportedmobile-root cls 0.000(ungraded,evidence incomplete — browser drift, HeadlessChrome/141 vs the baseline's /151). So it depends on something CI-specific.#JVYQEM: that row's phone remedy already landed (--spacing-mode-home-composer-phonereads10.125rem, not the6.625remit records) and its scale (~0.035) is an order of magnitude below0.223. The queued update marks that row partially stale and says what genuinely remains — the wide reserve, which needs a mechanism rather than a bigger constant.The recorded next step needs no reproduction: every failing run uploads a
lighthouse-budget-<runid>artifact containing the full report; reading themobile-rootcumulative-layout-shiftdebugdata names the shifting node directly.Also still blocked: the
document-viewerLinux visual baseline re-adoption cannot be done until #2199 merges — adopting now would commit a golden for a statemaindoes not have. Queued as its own P3 rather than guessed at.Generated by Claude Code