fix(ci): validate Lighthouse evidence and retain performance audit - #2744
fix(ci): validate Lighthouse evidence and retain performance audit#2744BigSimmo wants to merge 6 commits into
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
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
… 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
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
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: Advanced 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_7d4ef37f-e4a2-4d3d-a0eb-1766f6ff3c30) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aed81e482
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch codex/chat-lighthouse-baseline-reliability-7925 at starting commit 4aed81e; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:codex/chat-lighthouse-baseline-reliability-7925, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #16436 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
BigSimmo
changed the title
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_e85aa888-50d1-4fc1-ac6f-a3c66e846327) |
2 participants
Summary
Consolidates Lighthouse baseline reliability with its performance audit. Required evidence now includes every configured tolerance metric; missing report or baseline metrics fail validation.
Consolidates #2744 (4aed81e), #2723 (7d62215). Original heads are retained as ancestors; absorbed PRs are closed only after publication is verified.
Verification
npm run test -- tests/check-lighthouse-budget.test.ts— 75 passed, including missing configured report/baseline metric regressions.npm run format— passed; formatting committed.git diff --check— passed.Verification not run: full local PR/release suites; focused integration checks above were selected, and GitHub remains the merge gate. No live provider evaluation, deployment, migration or production data access was performed.
UI verification not run: local browser matrix; applicable hosted UI checks must pass before merge.
Current-base CI limitation: Make the outstanding-issues snapshot merge-safe (v2), and record the measured assessment of two other rows #2735 run 34495136858 failed only the aged ranking fixture provenance test (32 days old). clinical(governance): resolve forms permissions, test guards, and alert tiers (Bundle 1) #2763 carries the existing refresh; these bundles must not be merged past failed required checks.
Removed the obsolete tolerance binding exposed by hosted lint; focused ESLint on
scripts/check-lighthouse-budget.mjspassed without warnings.Risk and rollout