docs(issues): record the mobile-/ CLS root cause and file the hydration-gap guard - #2266
Conversation
…ws that chased it The install-card layout shift is traced, reproduced locally and fixed on main (PR #2253, commit 0cf0493), so the three open rows still hunting it get the evidence rather than another candidate: - #TYZK23 — the full trace: the shifting node, the ~2s window where #main-content is absent during streaming hydration, the first successful local reproduction (exactly 0.2230 before, 0.000 after), and the refuted first attempt, which was reverted rather than left in place with a message claiming a fix. - #50QRCF — the same cause explains the gate-reliability half. What varies between runs is timing, not the diff, which is why deleting one JSON file could flip a metric it cannot influence. - #KFRC3H — the fourth mechanism that row predicted. It is not the double-:has() shape: the ownership :has() is false during the gap whether or not a redundant second one is chained onto it, so PR #2234 is hardening rather than the cause and is still worth landing. None of the three is closed here. One green CI run on a bistable metric is weak on its own; the deterministic local before/after is the stronger half, and each row's own stop rules ask for confirmation first. Also files one new P3: nothing stops a future :has(#main-content…) rule inheriting the same hydration gap. All eleven current consumers are the install card, which 0cf0493 defends, so there is no live defect — only a missing guard against the next one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UksCoUHQMp72XUdp7RjchE
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:44 minutes Limit details: You’ve used the included review currently available. Your 90 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 (4)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:87a9014fe1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "id": "#TYZK23", | ||
| "detail": "ROOT CAUSE FOUND, REPRODUCED LOCALLY, AND FIXED — landed on main 2026-08-22 in PR #2253 (merge 66594dd), commits bc23075 (diagnostic) and 0cf0493 (fix). This row asked for the shifting node from a run where the shift actually fired; that evidence now exists. (1) ATTRIBUTION. The Lighthouse artifact could not be downloaded (Azure Blob egress blocked by this network policy), so the attribution was moved into the runner instead: scripts/run-lighthouse-budget.mjs now parses layout-shifts / layout-shift-elements / cumulative-layout-shift out of each per-cell report and prints selector, snippet, score and the raw worst item BEFORE the report directory is deleted, but only when grading already failed. CI then printed: \"mobile-root cls=0.2230 / 0.2230 body.min-h-full > div.pwa-notice-stack\", boundingRect {top:654, bottom:815, width:396, height:161}, nodeLabel \"Install Clinical KB … Install app / Not now\" — the COMPACT install card. (2) MECHANISM. #main-content briefly stops existing while Next 16 streams the route in and React hydrates it. The phone install-card geometry is chosen by body:has(#main-content[data-phone-footer-owner=\"hero\"]) …, so a card mounting inside that gap is styled by a selector that is false: it paints tall (h=401, bottom gap 92px), then is restyled compact (h=161, bottom gap 8px) when the shell returns. One discrete restyle, which is why the value recurs to three decimals instead of drifting. (3) LOCAL REPRODUCTION — the first one anyone has achieved, and the answer to this row note that it \"reproduces only in CI\". Four earlier attempts failed because beforeinstallprompt never fires in this container. Dispatching it synthetically from an init script at ~120ms WITH network throttling reproduced 0.2230 exactly, with the trace: t=4726ms #main-content present (owner=hero) -> t=7855ms #main-content GONE -> t=9083ms pwa-notice-stack mounts top=330 bottom=731 h=401 -> t=9930ms #main-content returns, stack top=654 bottom=815 h=161 -> t=9963ms SHIFT value=0.2230 div.pwa-notice-stack. So it was never CI-specific runner contention or Chromium 151 behaviour; it needed a slow network plus an early install prompt, which CI has and a fast local container does not. (4) FIX. src/components/pwa-lifecycle.tsx holds the notice stack unmounted until the app shell is present, via useSyncExternalStore over a MutationObserver on documentElement. The readyState===\"complete\" escape hatch releases the gate ONLY while the shell has never been seen (appShellHasEverMounted===false); an earlier version without that qualifier was refuted by CI returning the identical 0.2230, because load fires ~4s and the gap is at ~9s. That refuted commit was reverted rather than left in place with a message claiming a fix. Same local reproduction after the fix: CLS 0.000. CI on 0cf0493 (run 32531103787): Lighthouse budget SUCCESS, pr-required SUCCESS. (5) STOP RULES HONOURED: the cls tolerance was not widened, the baseline was not refreshed, and nothing was attributed without a run where the shift fired. NOT CLOSED HERE: one green CI run on a bistable metric is weak on its own — the deterministic local before/after is the stronger half. Close this row together with #50QRCF and #KFRC3H after the next Lighthouse budget runs on main-scoped PRs come back green.", |
There was a problem hiding this comment.
When these requests are applied by issues:reconcile, this payload updates only the detail/source, so #TYZK23 retains the summary claiming the defect “reproduces only in CI” even though this new detail proves a local reproduction and says it was never CI-specific. The sibling 50eff4b6-19e0-4992-9596-95501f63226b.json similarly retains “PR #2234 ... unconfirmed pending CI” and says that PR is “still worth landing,” although commit 9b10e59 for #2234 is already an ancestor of this commit’s parent. Because /issues surfaces these summaries for triage, the reconciled ledger will advertise obsolete work; include corrected summary fields and describe #2234 as already landed.
AGENTS.md reference: AGENTS.md:L1130-L1136
Useful? React with 👍 / 👎.
Summary
/Lighthouse CLS flake against the three ledger rows that were still hunting it. The defect itself was fixed in PR feat(dictionary): merge Search and Browse into one catalogue and rebuild the phone header #2253 (commit0cf0493), which merged before these capture requests could travel with it. This PR carries only the four immutable inbox requests; no source, no canonical ledger edit.#TYZK23gets what the row explicitly asked for — the shifting node from a run where the shift actually fired, plus the answer to its own note that the shift "reproduces only in CI". It does not. It needed a slow network and an earlybeforeinstallprompt, which CI has and a fast local container does not; dispatching the event synthetically at ~120ms under network throttling reproduced0.2230exactly. The record includes the mechanism trace (#main-contentpresent at t=4726ms, gone at t=7855ms, back at t=9930ms, shift fires at t=9963ms), the fix, and the earlier attempt that CI refuted and that was reverted rather than left in place.#50QRCFgets the explanation for the half that looked impossible: a required check flipping pass/fail on a diff that cannot influence layout. What varies between runs is timing, not the diff, so deleting one JSON file changes nothing about the page and everything about which side of the race the run lands on.#KFRC3Hgets the fourth mechanism it predicted. It is not the double-:has()shape: the ownership:has()is false during the hydration gap whether or not a redundant second one is chained onto it, which is exactly why0.223survived both of PR fix(pwa): stop the install-notice entrance animation from causing CLS #2219's fixes on run32477570217. PR fix(pwa): remove the remaining racy double-:has() install-card selectors #2234 is recorded as worthwhile hardening rather than the cause.P3recommendation is filed. On a throttled phone load#main-contentis absent from the DOM for roughly two seconds during streaming hydration, so any CSS keyed onbody:has(#main-content…)evaluates false for a window during load. All eleven current consumers are the PWA install card, which0cf0493defends, so there is no live defect — but nothing stops a twelfth consumer being added elsewhere and silently reintroducing a bistable CLS that took three ledger rows and several days to attribute. The recommendation proposes a paragraph indocs/search-chrome-behaviour.mdplus a static contract test, and carries a stop rule against deleting the:has()selectors, which are the correct way to express phone-composer ownership.Branch note:
claude/friendly-davinci-u4iwedwas restarted fromorigin/mainafter PR #2253 merged, rather than stacking these commits on already-merged history.Verification
npm run verify:pr-localAll 11 selected steps completed, none failed:
Every step ran fresh; no gate receipt was reused. The selector classified this as recognised low-risk documentation scope and skipped lint, typecheck, the full unit suite, the RAG fixture scan and the build, reporting each skip and its reason rather than passing silently. That selection was confirmed with
--dry-run --filesbefore the gate was run.The two gates that actually bite on this change both passed on real content:
npm run formatwas run against the whole tree and reported no changes beyond the four new files, which were already Prettier-clean as emitted byledger-inbox.mjs.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run: no UI, routing, styling, browser, reduced-motion or forced-colors behaviour changed. The diff is four JSON request files under
docs/outstanding-issues-inbox/and nothing else.npm run verify:releasebefore release or handoff confidence claimsVerification not run: release confidence is not claimed, and the gate is provider-backed.
npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changedVerification not run: no retrieval, ranking, selection, chunking or scoring behaviour changed. Nothing under
src/lib/rag/, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness or the golden fixture is touched. The gate is also provider-backed.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changedVerification not run: answer generation, the synthesis prompt and answer post-processing are untouched. The gates are provider-backed.
npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changedVerification not run: none of those changed. No executable code is in the diff.
npm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedVerification not run: deployment startup, hosting and rollout behaviour are unchanged.
Risk and rollout
docs/outstanding-issues.mdis untouched — these requests are applied later, deliberately serialised, bynpm run issues:reconcileon its own fresh-base branch. The worst realistic failure is a request whose prose turns out to be wrong, which is corrected by a superseding request rather than by editing anything.git revertof this PR's commit removes the four files. Nothing has been applied to the canonical ledger, so there is no reconciliation to undo and no ID allocation to reclaim.Notes
scripts/run-lighthouse-budget.mjsnow prints layout-shift attribution — selector, snippet, score and the raw worst item — before the report directory is deleted, and only when grading has already failed.#50QRCFand#KFRC3Hboth stalled on being unable to download the CI Lighthouse artifact from this network; that blocker is gone, and the next layout-shift breach of any kind will name its own element in the job log.6895c60a…,7c0f035c…, both 2026-08-21). They are untouched; the count of six pending in the gate output is those two plus these four.Generated by Claude Code