docs(issues): attribute the mobile CLS breach, land its measurement harness, and escalate #150 - #1505
Conversation
… trip #147 asked which elements shift. Driving Chromium against the same offline production build with a PerformanceObserver on layout-shift (Lighthouse mobile emulation, reading entry.sources[].node) gives one dominant cause on all four breaching routes: the entire main content region moves down 128px and straight back up 128px within 15-60ms. Both moves score, so it is pure cost with zero net movement — 100% of /documents/search's 0.220 and about 75% of /dsm's. The shifting element is the max-sm:pt-[var(--phone-overlay-chrome-h)] wrapper around <main>. A MutationObserver timeline on the root style attribute pins the mechanism rather than inferring it: the property goes CSS seed -> 200px -> 72px, and the 200px is written when the header stack ALREADY measures 72px (t=1552ms reserve=200px stack=72, corrected at t=1612ms). usePhoneOverlayChromeReserve reads stack.offsetHeight while the stack is transiently tall, publishes a value that is stale by the time it lands, and its ResizeObserver then corrects it. The CSS seed at globals.css:375 is correct for the settled stack, which corrects the mechanism recorded on the now-archived #130 — that framed the defect as the seed under-reserving by 0-8px. Measured, the driver is a 128px transient over-reserve written by the hook, not the seed. / is the control: it never writes the property and is the one clean route. Variance is stated rather than smoothed: /dsm measured 0.363 and 0.219 across two runs, and this harness has no network throttling so /forms and /therapy-compass run high locally. Only /dsm, /documents/search and / reproduced the live dispatch exactly. Also recorded: attaching a MutationObserver to document.documentElement inside a Playwright addInitScript throws before the document element exists, silently killing the CLS observer and reporting a uniform CLS=0.000 — a false clean bill that voided one run of this harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Temporarily paused during the owner-authorized ordered merge sweep so |
|
@cursoragent this PR will be re-opened soon. Please resolve any failing CI, run bugbot and resolve any bugbot comments against current repo and fix any merge conflicts and also fix any errors and address and fix all comments or suggestions that are likely to improve the PR and Codebase. Please implement smallest required fix according to best code practices. Check your work. Leave this PR closed for now but still make all these changes and run the review. I want it ready to go for when I re-open it. |
|
PR #1505 is ready to reopen and still closed. Branch tip Status
Pushed commits
Local gates:   |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…round trip Collapse-motion routes never apply max-sm:pt-[var(--phone-overlay-chrome-h)], so the 200px→72px reserve publish cannot move /therapy-compass. Keep its residual CLS as a separate cause after the overlay-route fix. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…head Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Reopen-ready prep (PR left closed)Branch tip pushed: Done
Not done (by design)
|
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Resolves the fifth conflict on docs/outstanding-issues.md by taking main's side on both hunks, after verifying that was correct rather than convenient: main renumbered the queue to fix a duplicate rank 18 this branch introduced, and it archived #138 and #146 rather than dropping them (confirmed present in the archive table). The resulting id set is byte-identical to main's. #150 escalated. That row records CodeRabbit reviewing none of a full day's PRs and rests explicitly on the Codex connector being the surviving reviewer that "found three real defects that had survived local gates". On PR #1505 Codex posted its own usage-limit notice alongside CodeRabbit's spending-cap notice, so that PR received zero automated review and so will anything opened while both caps hold. This is the same issue with its fallback removed, not a new one — which matters because the single Codex finding on #1459 was correct and caught a verification that had matched the wrong component, closing #105 on bad evidence. Local gates did not catch it. The attribution harness lands as scripts/measure-cls-attribution.mjs. #147's next step needs a before/after CLS pair, and #118 will want element attribution again; without this, both mean re-deriving it. Build and serve mirror run-lighthouse-budget.mjs so the numbers sit beside the Lighthouse reports. Two traps that cost real time are encoded rather than left to be rediscovered: CHROME_PATH must be set where the browser is outside a standard location, and an init script attaching a MutationObserver to document.documentElement before <html> exists throws and silently takes the CLS observer with it — so the script now fails loudly on an all-zero result instead of reporting a false clean bill. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF
BigSimmo
changed the title
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@cursoragent fix merge conflict |
|
Merge conflict resolved and pushed. GitHub’s
|
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
3 participants
Summary
Three changes: the
#147CLS attribution (already partly onmainvia the merge sweep), the harness that produced it, and an escalation of#150. One is a new script; the rest is ledger prose.1.
#147— the mobile CLS breach is attributed, and one cause dominatesDriving Chromium against an offline production build under Lighthouse mobile emulation (412×823, DPR 1.75, 4× CPU throttle) with a
PerformanceObserveronlayout-shiftreadingentry.sources[].node: on/dsm,/documents/search,/formsand/therapy-compassthe entire main content region moves down 128px and straight back up 128px within 15–60ms. Both moves score, so it is pure cost with zero net movement — 100% of/documents/search(0.110 + 0.110 = 0.220) and roughly 75% of/dsm. The shifting element is themax-sm:pt-[var(--phone-overlay-chrome-h)]wrapper around<main>.The mechanism is pinned, not inferred. A
MutationObserveron the root style attribute records the property going CSS seed →200px→72px, and the decisive line is that the 200px is written when the header stack already measures 72px:t=1552ms reserve=200px stack=72, corrected att=1612ms.usePhoneOverlayChromeReservereadsstack.offsetHeightin itsuseLayoutEffectwhile the stack is transiently tall, publishes a value already stale by the time it lands, and itsResizeObservercorrects it.This corrects the mechanism recorded on the now-archived
#130, which framed the defect as the CSS seed under-reserving by 0–8px. Measured, the seed atglobals.css:375is correct for the settled stack; the driver is a 128px transient over-reserve from the hook./is the control — it never writes the property and is the one clean route at 0.023.2.
scripts/measure-cls-attribution.mjs— the harness, landed#147's next step needs a before/after CLS pair, and#118will want element attribution again. Without this, both mean re-deriving roughly 200 lines. Build and serve mirrorscripts/run-lighthouse-budget.mjs(offline env, demo corpus, inert loopback Supabase, isolated.next-playwright/<run-id>/dist), so its numbers sit beside the Lighthouse reports.Two traps that cost real time are encoded rather than left to be rediscovered:
CHROME_PATH. Where the browser lives outside a standard location, resolution fails on every route. The script honoursCHROME_PATH/PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHand otherwise lets Playwright resolve its own browser.MutationObservertodocument.documentElementbefore<html>exists throws, and because that precedes thePerformanceObserverit silently takes the CLS measurement down — reporting a uniformCLS=0.000on every route. That voided a run of this harness. The script now fails loudly on an all-zero result instead of reporting it as clean.Additive and wired into no gate, so it cannot break an existing check.
3.
#150escalated — both automated reviewers are now capped#150records CodeRabbit reviewing none of a full day's PRs, and rests explicitly on the Codex connector being the surviving reviewer that "found three real defects that had survived local gates". On PR #1505 the Codex connector posted its own usage-limit notice alongside CodeRabbit's spending-cap notice, so that PR received zero automated review — and so will anything opened while both caps hold. This is the same issue with its fallback removed, not a new one.It is worth more than a status note: the single Codex finding on #1459 was correct and changed the outcome — it showed a claimed
LoadingPanelverification had matchedModeHomePageSkeletoninstead, which had closed#105on wrong evidence. Local gates did not catch it; the review did.Verification
npm run verify:cheap— exit 0.Test Files 445 passed (445),Tests 4659 passed | 4 skipped (4663). Run because this adds a source file, not just prose.npm run check:outstanding-issues—Outstanding-issues guard passed: 157 rows (44 open, 113 archived), unique ids, next-id=160 above the highest, no merge driver, no ids deleted from base b2122c7882c8.npm run check:branch-review-ledger—299 live table records + 1206 archived … no conflict markers, mojibake, heading records, or duplicates.npm run docs:check-inventory/docs:check-scripts/docs:check-links— inventory current at196 script files, 209 npm scripts;412 npm-run reference(s)and1427 repo path referencesresolve. The pre-commit hook caught a stale 195 count and refused the first commit until the regeneration was staged.npx prettier --check .—All matched files use Prettier code style!Whole-tree, against the committed state.evaluatePullRequestPolicyagainst the real file set —clinicalRisk: false,operationalRisk: false,ragRanking: false,errors: [].Variance stated rather than smoothed over.
/dsmmeasured 0.363 (3 shift entries) and 0.219 (2 entries) across runs — the round trip is the stable core. The harness applies no network throttling, so/forms(0.310 local vs 0.212 live) and/therapy-compass(0.240 vs 0.142) run high locally. Only/dsm,/documents/searchand/reproduced the live dispatch exactly; only those three are production-equivalent. LCP does not reproduce locally at all and must not be read off this harness.UI verification not run: no UI, routing, styling, reduced-motion or forced-colors code changed. The
#147fix will neednpm run verify:phone-chromeplus a before/after pair — that is the recorded next step, deliberately not done here.Risk and rollout
#147loses its attribution and the#130correction,#150loses the escalation, and the harness goes with it.Notes
docs/outstanding-issues.md, which has no merge driver by design (#133). Main's side was taken on both hunks, but only after checking it was right rather than convenient: main renumbered the queue to fix a duplicate rank 18 this branch introduced, and archived#138/#146rather than dropping them — both confirmed present in the archive table. The resulting id set is byte-identical to main's.src/lib/rag/**or any retrieval, ranking, or clinical surface is touched, so noRAG impact:line and no Clinical Governance Preflight applies.#147fix is deliberately not here. Stopping the hook publishing a value it is about to revise is a phone-chrome change governed bydocs/search-chrome-behaviour.md, on a surface with a documented flake history, and it deserves its own before/after evidence.#147. It is already correct for the settled stack.🤖 Generated with Claude Code
https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF