docs(audit): close RB-3 with the measurement it deferred - #2723
Conversation
RB-3 was recorded by a static, code-only pass and deferred its own
verification ("Measurement needed: Chrome DevTools Performance recording
during scroll, hover, and theme toggle"). That measurement has now been
taken in Chromium, and it splits the finding three ways rather than one.
1. `html.theme-transitioning *` was real and worse than its P3 rating:
1226 concurrent animations and 546ms with no frame presented, leaving a
half-light/half-dark paint on every theme switch. Fixed in PR #2653.
2. The three stacked backdrop-filter passes are NOT a defect. Over a 4s
continuous scroll at 1440x900, three runs per condition:
three passes mean 17.06/16.81/16.83ms >32ms 4/3/3
one pass mean 16.79/16.70/16.79ms >32ms 3/3/5
no blur mean 16.82/16.75/16.76ms >32ms 3/3/3
Removing the blur entirely does not move the numbers out of run-to-run
noise, so the two extra passes cost nothing worth reclaiming. They are
not redundant either: dropping them changes 35.59% of the scrim's pixels
(max channel-sum delta 39/765). Visible design change, no measured gain.
The scrim is display:none below 640px, so it was never a phone cost.
A backdrop-root hypothesis was tested and refuted: `.edge-glass-header`
carries `isolation: isolate`, but the passes are not inert — disabling
the blur changes 47.50% of scrim pixels (max delta 321).
3. The `box-shadow` entry in the `.answer-footer-search-pill` transition is
NOT a defect. Toggling the real `-open` state every 200ms for 4s: mean
16.62/16.66/16.62ms with it versus 16.65/16.67/16.67ms without, and zero
frames over 32ms in either condition. It is not dead weight either —
:hover, :focus-within and -open each set a different box-shadow, so
removing it makes the composer's focus ring and elevation snap.
No source change accompanies this commit on purpose. Two thirds of RB-3
should not be actioned, and the remediation table previously told a reader
to action them. The stale line numbers are corrected in the same block.
Gates: npm run verify:pr-local (docs scope) — 15 checks completed, none
failed, none unreached. Build skipped: no build-affecting changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_431890b3-1fa5-4472-aff0-cabbbee016f4) |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
The measurement block asserted numbers without saying how they were taken, which this repo's evidence rule does not allow. Records that the runs were headless Chromium 1194 (this container's build against the pinned 1234, the #255 drift) driving the DEV server at 1440x900 on cloud hardware, three runs per condition, sampling every requestAnimationFrame. Absolute frame times are therefore not a production figure and no production claim is made from them. The relative comparison is what both closures actually rest on: identical page, identical hardware, one property changed between conditions. A dev build is the slower case, so it biases towards exposing a paint cost, and none appeared. Also notes that the cited line numbers were re-checked against origin/main at 0177bed. Gates: npm run verify:pr-local (docs scope) — 15 checks completed, none failed, none unreached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38411b8cf7
ℹ️ 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".
… absent Codex review on PR #2723 was right, and it cited the correct rule ("Evidence and calibration are never compressed"). The first version of this update read flat 60Hz frame times as proof of no cost. It is not. requestAnimationFrame deltas are vsync-capped at ~16.7ms, so a condition that finishes early is indistinguishable from one that finishes just inside the budget. That method can only show frames were not dropped. Re-measured with the scrim area scaled up to force the cost above the vsync floor, everything else held fixed (mean/p95 ms, two runs): height three passes one pass no blur 104px 16.90/16.94 p95 16.9 16.70/16.85 p95 16.9 16.86/16.75 416px 19.43/19.77 p95 42.4 17.01/17.08 p95 18.2 16.67/16.75 832px 75.99/77.36 p95 114.5 18.61/19.52 p95 32.8 16.83/16.88 The three-pass stack is emphatically not free: by x8 it is ~4x the frame budget and an order of magnitude worse than a single pass. The earlier claim that the extra passes "cannot be costing anything worth reclaiming" was wrong and is withdrawn. What survives is narrower and still useful: at the SHIPPED 104px the cost is below this method's resolution and drops no frames, on a renderer that turns out to be SwiftShader software rasterisation (ANGLE (Google, Vulkan 1.3.0 (SwiftShader Device (Subzero)))) — slower than any GPU a user will have. So there is no measured problem to fix at the current size, and flattening the stack on performance grounds is not justified today. No sub-frame figure is quoted for 104px because none was measured; extrapolating from x4 would be arithmetic, not evidence. The genuinely actionable finding is new: the cost scales superlinearly with area, and scrim height is max(6.5rem, safe-area-top + 5.5rem). Anyone enlarging it, raising the blur radii, or reusing the three-pass recipe on a taller surface must re-measure. The summary bullet and both tables are re-scoped to say that instead of "non-defect". The same vsync caveat is now stated for the box-shadow item. Its conclusion is unchanged: one small pill animating only on hover, so area sensitivity does not apply, and removing it is a certain interaction regression against an unmeasured gain. The benchmark harness is retained inline in the RB-3 block, per the review request, so the tables can be reproduced rather than taken on trust. Gates: npm run verify:pr-local (docs scope) — 15 checks completed, none failed, none unreached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd
|
Consolidated into #2744 at 86c6115. The complete original head 7d62215 is retained as an ancestor, including its code, tests and audit requests. Closing this duplicate review/CI stream under the authorised consolidation; source branch retained. Current verification and remaining merge prerequisites are recorded on #2744. This closure does not claim that the successor is merged or that its required CI is green. |
Summary
RB-3 in the 2026-08-02 CWV audit was recorded by a static, code-only pass and deferred its own verification ("Measurement needed: Chrome DevTools Performance recording during scroll, hover, and theme toggle"). That measurement has now been taken. No source change accompanies this PR on purpose — one third of RB-3 was real and is already fixed, and the remediation row for the other two is withdrawn.
1.
html.theme-transitioning *— real, and worse than its P3 rating. Already fixed in #2653.1226 concurrently running animations, then 546 ms with no frame presented (rAF samples at 313 ms and 859 ms, nothing between). The frame surviving that stall was painted half in each theme — light header band and light sidebar chrome over dark panels, headings still at the light-theme ink colour. That is what a user saw on every theme switch.
2. Three stacked
backdrop-filterpasses — the cost is real and scales sharply with area, but drops no frames at the shipped size.The original method was wrong, and this is the correction.
requestAnimationFramedeltas are vsync-capped at ~16.7 ms, so flat 60 Hz numbers can only show frames were not dropped — they cannot show absence of cost. The workload was re-run with the scrim area scaled up to force the cost above the vsync floor, everything else held fixed (mean/p95 ms, two runs):The stack is emphatically not free: by x8 it is ~4x the frame budget and an order of magnitude worse than a single pass. What survives is narrower — at the shipped 104 px the cost is below this method's resolution and drops no frames, on a renderer that turns out to be
ANGLE (Google, Vulkan 1.3.0 (SwiftShader Device (Subzero))), i.e. software rasterisation, slower than any GPU a user will have. No sub-frame figure is quoted for 104 px because none was measured; extrapolating from x4 would be arithmetic, not evidence.So: no measured performance problem at the current size, and flattening on performance grounds is not justified today. Flattening is also a visible design change — dropping the two pseudo-element passes alters 35.59% of the scrim's pixels (max channel-sum delta 39/765). The scrim is
display: nonebelow 640 px, so none of this was ever a phone cost.The actionable finding is the area sensitivity, which the first version missed: scrim height is
max(6.5rem, safe-area-top + 5.5rem), and the cost climbs superlinearly. Anyone enlarging it, raising the radii, or reusing the three-pass recipe on a taller surface must re-measure.A backdrop-root hypothesis was tested and refuted:
.edge-glass-headercarriesisolation: isolate, but the passes are not inert — disabling the blur changes 47.50% of scrim pixels (max delta 321). The authoring rule above the block holds as written.3.
box-shadowin the.answer-footer-search-pilltransition — no observed frame loss, and removing it costs interaction quality.Toggling the real
.answer-footer-search-pill-openstate every 200 ms for 4 s, three runs per condition: mean 16.62/16.66/16.62 ms withbox-shadowin the transition list versus 16.65/16.67/16.67 ms without, and zero frames over 32 ms in either. The same vsync caveat applies — no dropped frames, not zero cost — but this is one small pill animating only on hover/focus, so the area sensitivity above does not bite. It is not dead weight either::hover,:focus-withinand-openeach set a differentbox-shadow, so removing it makes the composer's focus ring and elevation snap. A certain interaction regression against an unmeasured and probably negligible gain.Scope limits, stated rather than implied: nothing here covers 120 Hz displays (an 8.3 ms budget this method cannot resolve at x1), WebKit, or hardware compositing. No such run exists — neither is available in this container — and that gap is written into the document as a limit.
Stale line numbers in the finding are corrected in the same block, re-checked against
origin/mainat0177bed(globals.css1344-1358 for the pill, 1382-1421 for the scrim, 5024-5062 for the theme block). The benchmark harness is retained inline in the RB-3 block, with browser build, workload, viewport, run count and GPU string, so the tables can be reproduced rather than taken on trust.Verification
npm run verify:pr-local -- --files docs/audit/performance-image-cwv-audit-2026-08-02.md— exit 0, re-run after each of the three commits. Summary line:completed: check:runtime, check:installed-lock-parity, format:changed, check:diff-integrity, sitemap:check, check:mockups, check:repo-awareness-snapshot, docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links, check:branch-review-ledger, check:outstanding-issues, check:ledger-write-discipline, check:clinical-hazard-controls/failed: (none)/not reached: (none).--dry-runfirst confirmed the risk router classifies this as recognised low-risk scope:lint, typecheck, full unit suite and RAG fixture scan skipped,build skipped: no build-affecting changes detected.npm run verify:ui— UI verification not run: no source, style, or route file is touched.plan:browserhas nothing to select on a docs-only diff.check:clinical-hazard-controlsemitsCLINICAL_HAZARD_CONTROLS_SHALLOW_CLONE ... reviewedCommit existence/ancestry checks were skipped. That is the same shallow-clone condition that makestests/clinical-hazard-controls.test.tsandtests/privacy-readiness-contract.test.tsfail in a fresh cloud container while passing in CI, which has full history. Not a repo defect and not touched here.Risk and rollout
Notes
docs/outstanding-issues.mdrow is added: nothing is open there on this finding (#016closed 2026-08-18), and the audit is the correct home for the measured outcome.🤖 Generated with Claude Code
https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd