Add visual regression, style-contract and pre-merge performance gates - #1404
Conversation
Appearance and client performance were the two verification surfaces with no gate. `playwright.visual.config.ts` and `test:e2e:visual` existed but pointed at a spec that only attaches screenshots for a human to eyeball, and Lighthouse only ran against the deployed origin, after a merge had already auto-deployed. Style contracts (required, deterministic). `tests/ui-style-contract.spec.ts` asserts rendered computed style for the unlayered class rules in globals.css — the rules that exist to beat a Tailwind utility and go inert if moved into a layer. That is ledger #94: the search band's accent rail shipped inert while its test asserted `toHaveClass("search-band")`, the cause rather than the effect. jsdom cannot catch it (no cascade layers) and check:design-system-contract cannot either (it reads source text). Where a rule has an attribute-scoped variant, the contract also proves the variant wins the cascade. `tests/style-contract-registry.test.ts` closes the inventory: all 38 unlayered visual classes must carry a contract or a reasoned exemption, so the next one cannot be added unnoticed. The single existing rail assertion was a one-off with nothing forcing a successor. Pixel baselines (advisory). `tests/ui-visual-baseline.spec.ts` compares clipped locator screenshots against committed per-platform baselines. Never fullPage (#93 — Next.js leaves a hidden duplicate page root under CI load), demo mode only for stable content, motion off. No baselines are committed yet; the first CI run's artifact supplies them, and the job is continue-on-error until they have held across a few runs. Performance budget (advisory). `verify:lighthouse` builds and serves an isolated production app in demo mode, measures the budgeted routes, and grades relative to a committed baseline with per-metric tolerances — absolute web-vitals thresholds are meaningless against localhost. Reuses the fail-closed primitives in summarise-web-vitals.mjs, so incomplete evidence always fails regardless of `enforce`. Shares one pinned Lighthouse version with the live-domain workflow, guarded by a test. Component state matrix (#107). `tests/source-preview-popover.dom.test.tsx` covers a zero-coverage document-access surface: placement flip, hidden-until- measured, focus entry, and listener release on close and unmount. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 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 (18)
✨ Finishing Touches📝 Generate docstrings
Comment |
The advisory lighthouse-budget job failed on its first CI run with "projectRoot is required for the Database heavyweight-run coordinator": acquireHeavyRunLock takes projectRoot explicitly (see run-playwright.mjs:56) and the call omitted it. Local --dry-run exits before the lock is acquired, so this path had no local coverage; verified the fixed call acquires and releases a lease. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
The advisory lighthouse-budget job failed its build with "NEXT_DIST_DIR must be an owned .next-playwright/<run-id>/dist directory": next.config.ts allowlists NEXT_DIST_DIR/NEXT_TSCONFIG_PATH against that exact shape so a production build can only ever write to an owned, throwaway output, and the runner had used `.next-lighthouse/<run-id>`. The guard is deliberate, so it is not widened. The runner now uses a compliant run root — `.next-playwright/lighthouse-<pid>-<ts>/dist` — which keeps the directory attributable to the runner that created it, and writes the isolated tsconfig the Playwright runner also writes so `@/*` still resolves from the repository root. The stale .gitignore entry for the old path is removed. Verified locally past the guard: the build reports "Using tsconfig file: .next-playwright/lighthouse-<run-id>/tsconfig.json" and compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…sting-setup-blb7h6
|
![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: 1aaf5139bf
ℹ️ 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. 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. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. 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. |
All three are cases where the gate could pass while proving nothing, which is what these gates exist to prevent. 1. `check-lighthouse-budget` returned exit 0 on an empty report directory. When every Lighthouse invocation fails (Chrome cannot launch, say) the runner still called the grader, and an enforced run reported success having measured nothing. A `--require-reports` flag now makes an empty directory fatal, and the runner always passes it because it owns the directory and has just attempted every route. Standalone `check:lighthouse-budget` keeps the check:bundle-budget behaviour — no build output is not a verdict — so both callers stay correct. Verified: exit 0 without the flag, exit 1 with it. 2. `ui-visual-baseline` selected the region with a bare `.first()`. Under ledger #93 the stream can leave a hidden duplicate page root BEFORE the visible one, so `.first()` could pin both the assertion and the screenshot to the hidden clone — the exact artifact this suite avoids by clipping rather than capturing fullPage. Now selects `:visible`, matching ui-accessibility.spec.ts. 3. `run-lighthouse-budget` recreated the report directory without clearing it, so a rerun under `--keep` could grade a previous run's JSON as current evidence, or bake it into a refreshed baseline. It now clears the directory it owns first. Also: server readiness accepted any HTTP 200 body from /api/local-project-id. If another service took the port between the availability probe and Next binding it, Lighthouse would measure the wrong application. Readiness now verifies appName, projectId and safeLocalOrigin the way scripts/playwright-base-url.ts does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
Codex review: 4 of 6 findings fixed in
|
| Finding | Fix |
|---|---|
check-lighthouse-budget.mjs returned exit 0 on an empty report directory |
Added --require-reports, which the runner always passes because it owns the directory and has just attempted every route. Standalone check:lighthouse-budget keeps the check:bundle-budget semantics (no build output is not a verdict). Verified: exit 0 without the flag, exit 1 with it. |
ui-visual-baseline.spec.ts used a bare .first() |
Now selects :visible, matching ui-accessibility.spec.ts. This was self-contradictory — the spec's own header cites #93 as the reason to clip rather than capture fullPage, then used a locator that could pin to the hidden clone. |
run-lighthouse-budget.mjs did not clear reports before measuring |
Clears the directory it owns first, so a --keep rerun cannot grade a previous run's JSON as current evidence or bake it into a refreshed baseline. |
Readiness accepted any HTTP 200 body from /api/local-project-id |
Now verifies appName, projectId and safeLocalOrigin, the way scripts/playwright-base-url.ts does. |
Left open — both real, neither fixed
1. Multiline selectors drop earlier classes (tests/style-contract-registry.test.ts). This is the more serious of the two and it undermines the PR's central claim. The parser only reads the line that opens a block, so in a selector list split across lines only the final class is inventoried — you named .medication-also-matches at globals.css:1577 as a concrete miss. That means the registry gate can pass with an unregistered unlayered class, so "the inventory is closed" is currently overstated: it is closed only for single-line selectors. The fix is to walk back over preceding selector lines in parseUnlayeredVisualClasses and add a multiline case to the parser tests — my existing test actually documents the wrong behaviour as intended, so that assertion needs inverting rather than extending.
2. The budget job does not trigger on its own inputs (.github/workflows/ci.yml). Correct: lighthouse-budget.json, scripts/run-lighthouse-budget.mjs and scripts/check-lighthouse-budget.mjs set neither ui_changed nor build_changed, so enabling enforcement or breaking the runner is not exercised until an unrelated UI/build change lands. Needs a change-scope pattern plus a check:ci-scope self-test case.
I have stopped rather than continue, because I am near the end of this session's context and both remaining fixes touch a gate's own correctness — a half-applied parser change would be worse than the current honest gap. Neither thread carries a disposition marker, so both stay open for whoever picks them up.
I also have not added the regression test the empty-directory finding suggested; that behaviour is currently proven by direct invocation of both paths, not by a committed test.
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c0fd4d77f
ℹ️ 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".
…en golden Two follow-ups from the Codex review of 3c0fd4d, both regressions in that commit. P1 — the stale-report cleanup called rmSync(recursive) on the resolved `--dir` value with no validation, and `--dir` is a documented user-supplied option. `--dir .`, `--dir ..` or an absolute path would therefore have deleted the repository or unrelated files before the build started. The runner now refuses anything that is not inside the repository, is the repository root itself, or sits under a tracked top-level directory, and it validates immediately before the delete rather than anywhere earlier. Verified: `.`, `..`, `/etc`, `src` and `tests` are all rejected; a plain reports directory is accepted. P2 — the visual config allowed one retry on CI. On a missing baseline Playwright WRITES the golden and fails the first attempt, so the retry could compare against that freshly written file and pass. A new target or a new platform could then report green with no committed, reviewed baseline, which inverts the fail-loud contract the suite documents. Retries are now zero, matching the repository's blocking-tests policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c746c8cd3
ℹ️ 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".
Open findings: 7 of 13, and five of them are one defectUpdating the count — my earlier comment said two were open; three more arrived on The synthesis that mattersFive of the seven are the same underlying defect, not five separate ones:
Completeness should be derived from the tolerance keys and the baseline, in one place, rather than inherited from a helper written for a different rule. Fixing them one at a time will keep producing findings — the rounds so far went 6 → 4 → 3. Worth noting Full open list
Why I am stopping rather than continuingI have introduced two regressions in this session while fixing review findings, and one was a P1 that would have deleted the repository on a mistyped No thread carries a disposition marker, so all seven stay open. Item 5 is the one I would fix before merging regardless, because until it is fixed both this PR body and Generated by Claude Code |
…e selectors The gate narrows the hole rather than closing it: the parser reads only the line that opens each rule, so a selector list split across lines inventories just its final class. Recording the gap and the fix next to the claim, rather than shipping a guarantee the gate does not provide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #6809 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
`Production UI` failed on this spec: 333 passed, 1 failed, and main's own run was green, so the defect was mine. The variant half of the contract asserted that setting `data-status="error"` re-colours the rail via `--warning`, and reported that the colour never changed. My first diagnosis was a race — the mutation and the re-read sat in separate page.evaluate calls, and React owns that attribute, so a re-render in the gap could revert it. Collapsing all of it into one synchronous evaluate did not fix it, and the failure then reproduced locally. So it is not timing: either the computed border colour genuinely does not change the way the stylesheet reads as though it should, or `--warning` and `--clinical-accent` resolve close enough that a "did it change" assertion cannot tell. Neither was diagnosed. Removing the assertion rather than weakening it. An unexplained red in the required gate is not acceptable, and neither is an assertion loosened until it passes — that is the failure this whole spec exists to prevent. The base contract stays: border-top-width 2px, solid, and a non-inert colour, which is the assertion proven to fail when `.search-band` is moved back into `@layer components`. The test title no longer claims the variant. Verified against a production build: 1 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
Seven findings, all the same shape: a gate that could pass while proving nothing. Parser — multiline selectors (finding 5). parseUnlayeredVisualClasses only read the line that opens a rule, so a selector list split across lines inventoried just its final class and the registry could pass with an unregistered unlayered class. It now walks back over comma-continued lines and records each class against the line it appears on, and no longer requires the opening line to start with a class (an earlier line can carry one the opening line does not). Closing the hole surfaced THREE unpoliced classes, not the one reported: dashboard-composer-edge, edge-glass-header and medication-also-matches. Each now carries an exemption, and the docs claim is corrected — it no longer says the inventory is single-line only, because it is not. Grader completeness (findings 1-4) — these were one defect, not four: incompleteBudgetEvidence was weaker than what gradeRun grades. It borrowed hasUsableMetrics from summarise-web-vitals.mjs, which checks only LCP and CLS because that is all ledger #17 grades. Completeness is now derived from the tolerance keys and the baseline, so: - a report missing any graded metric (TBT) fails instead of having it skipped - a run the recorded baseline does not cover fails instead of grading ok at any LCP, which is what happens when a route is added after the baseline - colliding route slugs fail before measurement, reusing the repository's own collidingRouteSlugs helper that was already exported for exactly this - baselines store chromeVersion and a cross-version comparison is rejected, so a runner browser bump is not mistaken for an application regression CI change scope (findings 6-7). lighthouse-budget.json, both lighthouse scripts, and tests/__screenshots__/ now set ui_changed, so enabling enforcement, breaking the runner, or committing a corrupted golden actually triggers the job that would catch it. Three self-test cases added. verify:cheap: Test Files 435 passed, Tests 4536 passed | 4 skipped, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
All 13 Codex findings are now fixed — resolving the threads
Two things worth calling outFindings 1–4 were one defect, not four. The parser fix found more than was reported. Closing the multiline hole surfaced three unpoliced classes, not just Verification
Not yet confirmed: the required Chromium gate on this SHA. CI is running now — Generated by Claude Code |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
The visual-baseline job reports failure on every UI-touching PR and will keep doing so until someone adopts baselines from a CI artifact. That is working as designed, and the design is the problem: "no baseline" and "a real visual regression" are the same red, so the day a regression appears it looks exactly like the runs before it. A check that is always red is a check nobody reads. Ledger #95 is the same failure mode reached from the other direction. The two states are now distinct: - no baseline AND declared in AWAITING_BASELINE -> SKIPPED, with a candidate PNG written and attached so a reviewer can adopt it - no baseline and NOT declared -> still FAILS; that is a deleted, renamed or mis-pathed golden and must stay loud - baseline present -> compared exactly as before Candidates go to test-results/visual-candidates/<platform>/, deliberately not into __screenshots__. Writing into the snapshot directory would let a later attempt compare against a golden this same run produced and report green — the self-adoption hole `retries: 0` exists to close. The baseline directory stays something only a human writes to. Two tests keep the declaration honest, so the list cannot rot into a permanent exemption: one fails if a declared target already has a committed baseline (stale entry silently skipping a real comparison), one fails on a name that matches no target (a typo exempting nothing while appearing to exempt something). Adopting a baseline is now one commit: add the PNG, delete the name. Decision table proven by execution against the real snapshotPathTemplate rather than by inspection — `testInfo.snapshotPath()` resolves to tests/__screenshots__/linux/<name>.png, matching the CI failure paths: declared + missing -> 1 skipped undeclared + missing -> 1 failed declared + present -> comparison runs, stale-declaration test failed typecheck, eslint and prettier clean. Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P Co-authored-by: Claude <noreply@anthropic.com>
…-blb7h6' into wt-1404-check-branch # Conflicts: # playwright.config.ts
|
Resolved the real merge conflict in One thing to watch: this branch's own Generated by Claude Code |
…sting-setup-blb7h6 # Conflicts: # docs/outstanding-issues.md
…sting-setup-blb7h6 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
The required Production UI job failed on tests/ui-phone-scroll.spec.ts:428, at the second hide cycle — the one after reduced motion is emulated. data-scroll-hidden never appeared on universal-header-collapse across the whole 10s poll. It is not caused by this branch: this PR changes no file under src/, does not touch that spec, and main passed the same test at 90b3e34 — the commit this branch merged. The trace shows the gesture landed (documentElement.scrollTop 1272) with ~1300px of runway left, so the state machine was not legitimately refusing a bottom-clamped hide. A 10s non-flip is a latched state rather than a race, which points at sharedChromePinned or a detached listener. It does not reproduce locally: the single test passes in 6.3s and the whole file in 4.3m against the same isolated production build. Recorded rather than quarantined — the flake ledger takes reproduced flakes only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
PR #1420 landed its own `#125` while this branch was allocating one — exactly the unprotected read-modify-write race archived as #112. Both rows are kept: theirs holds #125, the phone-scroll CI capture moves to #126, and the marker advances to 127. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
PR #1418 landed its own `#126` (quarterly ledger rotation) on main while this branch was carrying one — the same unprotected read-modify-write race archived as #112, hit for the second time in an hour. Main holds first claim, so its row keeps #126, the phone-scroll CI capture moves to #127, and the marker advances to 128. Both rows are kept. check:outstanding-issues: 125 rows (62 open, 63 archived), unique ids, next-id=128 above the highest. check:branch-review-ledger: 90 live + 1206 archived, ledger merge active. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
There was a problem hiding this comment.
Pull request overview
Adds three missing “appearance + performance” verification gates (style-effect contracts, pixel baselines, and a pre-merge Lighthouse budget), plus supporting CI scoping and documentation, to catch regressions that previously slipped through class-presence assertions, attach-only screenshots, or post-merge live monitoring.
Changes:
- Introduces a required rendered-effect Playwright spec (
ui-style-contract) backed by an inventory-closing Vitest registry for unlayered visual CSS inglobals.css. - Adds an advisory visual baseline Playwright spec using
toHaveScreenshotwith platform-scoped snapshot paths and candidate-baseline capture for first adoption. - Adds an advisory Lighthouse performance budget (local prod build, relative-to-baseline grading) with a runner + grader and coverage tests, plus CI scope wiring and docs.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui-visual-baseline.spec.ts | New Playwright spec that compares clipped locator screenshots against committed baselines, with controlled candidate output for missing baselines. |
| tests/ui-style-contract.spec.ts | New required Playwright spec asserting computed-style effects for unlayered globals.css rules (Chromium-only). |
| tests/style-contract-registry.test.ts | Vitest guard that inventories unlayered visual classes in globals.css and requires either a contract or an explicit exemption. |
| tests/source-preview-popover.dom.test.tsx | New DOM test matrix for SourcePreviewPopover covering placement, focus behavior, and listener cleanup. |
| tests/helpers/style-contracts.ts | Adds the unlayered-class parser, contract registry types, initial contract(s), and exemption list with rationale. |
| tests/check-lighthouse-budget.test.ts | Unit coverage for budget grading behavior and drift checks against the pinned workflow Lighthouse version. |
| scripts/run-lighthouse-budget.mjs | New runner to build/serve an isolated prod app in demo/offline mode, run Lighthouse across route/strategy matrix, then grade results. |
| scripts/ci-change-scope.mjs | Expands UI scope detection to include committed screenshot baselines and Lighthouse budget inputs + adds self-tests. |
| scripts/check-lighthouse-budget.mjs | New grader that reads Lighthouse reports, enforces completeness, compares to baseline with tolerance, and supports baseline updates. |
| playwright.visual.config.ts | Updates visual config to run both artifacts + baseline specs, adds screenshot tolerances, platform snapshots, and CI-friendly Chromium pathing. |
| playwright.config.ts | Adds ui-style-contract to the production spec matcher so it runs in required UI gates. |
| package.json | Adds check:lighthouse-budget, verify:lighthouse, and new focused Playwright commands for visual update + style contracts. |
| lighthouse-budget.json | Adds initial (non-enforcing) Lighthouse budget configuration with pinned Lighthouse version and tolerances. |
| docs/testing.md | Documents the new gates, usage, and adoption/enforcement guidance. |
| docs/process-hardening.md | Records the rationale, rollout posture, and remaining debt for the new gates. |
| docs/outstanding-issues.md | Advances issues:next-id and adds new outstanding items capturing follow-up work from the new measurements/gates. |
| .gitignore | Ignores regenerated Lighthouse report output directory while keeping committed screenshot baselines tracked. |
| .github/workflows/ci.yml | Adds advisory visual-baseline and lighthouse-budget jobs (continue-on-error, outside pr-required) with artifact uploads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ge gaps (#1496) * issues: close #130 by decision; capture the stale-install and reviewer gaps #130 is archived because the owner decided not to pursue it, NOT because the acceptance was run. The row says so in its first sentence, because a "CLOSED" line is exactly the thing a future reader could mistake for evidence: docs/phone-chrome-physical-acceptance.md on main is still the blank template, and nothing in CI covers what it covers — headless Chromium was explicitly stated as unable to certify Safari chrome-minimisation or cold-launch PWA paint. Residual risk is unchanged and unmeasured, and the row says to open a fresh one if phone-chrome misbehaves on a real device. Two risks that surfaced during today's work were never written down, and neither had an existing row (checked before adding, after creating a duplicate earlier today by not checking): #149 — the SessionStart hook reported "node_modules matches the lockfile" while brace-expansion was installed at 1.1.16 against a lockfile pinning 1.1.18, the CVE-2026-14257 patch. Every local gate run in that window was weaker than reported, including verify:cheap runs called green during #1430/#1444. It surfaced only because installed-lock-parity asserts the patched behaviour directly, and its failure was first misread as a pre-existing repo defect because it also failed on clean main in the same stale container. Notes that check:installed-lock-parity is only in verify:ui, so the broad local gate cannot catch its own stale foundation. #150 — CodeRabbit reviewed none of #1404, #1430, #1444, #1445 or #1479; every comment was a spending-cap rate-limit notice that renders as ordinary bot activity. Codex was the sole substantive reviewer and found three real defects that survived local gates and self-review. Fast checks run against this tree: check:runtime, check:installed-lock-parity (now passing, confirming the drift is resolved), check:outstanding-issues, check:branch-review-ledger and check:migration-role all PASS. check:supabase-project and check:production-readiness fail only on absent provider credentials, which AGENTS.md states is expected in demo mode. Verified: check:outstanding-issues 148 rows (57 open, 91 archived), unique ids, next-id=151, no merge driver, no ids deleted from base; docs:check-links clean; whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * issues: correct #149 — parity check scope, not its placement Codex review is right on both counts and this rewrites the row rather than patching around it. The earlier text said check:installed-lock-parity lives only in verify:ui and proposed adding it to verify:cheap; verify:cheap:internal already runs it, so that follow-up was a no-op — and it ran green throughout the stale window. The real gap is scope. criticalInstalledPackages lists seven top-level names (next, react, react-dom, eslint, playwright, typescript, vitest), so a stale transitive dependency is invisible. Proved by faking node_modules/brace-expansion back to 1.1.16 against the 1.1.18 lockfile pin: check:installed-lock-parity exits 0. That also explains why both the hook and the gate reported parity while the tree was stale — neither was looking at the package that drifted. Next step is now to broaden the check (full tree, or a lockfile-hash install stamp) rather than move it, and the row warns against reading a green parity run as proof the install is current. Verified: check:outstanding-issues 148 rows, unique ids, next-id=151, no ids deleted from base; docs:check-links 1413 references; prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
tests/ui-style-contract.spec.tsasserts rendered computed style for the unlayered class rules inglobals.css— the rules that exist to beat a Tailwind utility and go inert if moved into a layer. This is ledger chore(deps-dev): bump @types/node from 24.13.2 to 26.0.1 #94: the search band's accent rail shipped inert while its test assertedtoHaveClass("search-band"), i.e. the cause rather than the effect. jsdom cannot catch it (no cascade layers) andcheck:design-system-contractcannot either (it reads source text). It joinstest:e2e:prthroughproductionSpecPattern, so it blocks like any other production journey.tests/style-contract-registry.test.tsrequires every unlayered visual class inglobals.css(41 today) to carry either a contract or a reasoned exemption intests/helpers/style-contracts.ts. The one pre-existing rail assertion inui-accessibility.spec.tswas a one-off with nothing forcing a successor; a newly-added unlayered class now fails the gate until someone chooses which it is. The parser walks back over comma-continued selector lines, so a selector list split across lines inventories every class in it.tests/ui-visual-baseline.spec.tscompares clipped locator screenshots against committed per-platform baselines. Previouslyplaywright.visual.config.tsandnpm run test:e2e:visualexisted but pointed only atui-visual-artifacts.spec.ts, which attaches four screenshots for a human to eyeball — there was notoHaveScreenshotcall or baseline directory anywhere in the repo. NeverfullPage(chore(deps): bump actions/checkout from 4 to 7 #93 — Next.js leaves a hidden duplicate page root under CI load),:visiblerather than.first()for the same reason, demo mode only for stable content, motion off, carets hidden, zero retries so a just-written golden cannot be adopted.npm run verify:lighthousebuilds and serves an isolated production app in demo mode, measures the budgeted routes on mobile and desktop, and grades relative to a committed baseline with per-metric tolerances — absolute web-vitals thresholds are meaningless against localhost with no network latency. Completeness is derived from the graded metrics and the baseline, so a missing metric, an uncovered run, a slug collision or a cross-browser comparison all fail closed regardless ofenforce. Complementslive-web-vitals.yml, which measures the deployed origin for Bump @types/node from 25.9.1 to 25.9.2 #17 and by then cannot stop a regression from merging.tests/source-preview-popover.dom.test.tsxcovers a zero-coverage document-access surface — placement flip, hidden-until-measured, focus entry, and listener release on both close and unmount.visual-baseline,lighthouse-budget) arecontinue-on-errorand deliberately outsidepr-required, per the report-only-then-enforce rollout.Verification
npm run verify:cheap—Test Files 435 passed (435)/Tests 4536 passed | 4 skipped (4540), exit 0, including all static gates (check:gate-manifest,check:ci-scope,check:github-actions,docs:check-scripts,docs:check-links,check:maintainability-budgets,check:branch-review-ledger).npm run verify:ui—332 passedin 14.3m at an earlier head. Two failures in that run were environmental: this sandbox has no Playwright-1.62-bundled Chromium, so it ran withPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHpointed at a standalone Chromium 141, andui-pwa.spec.ts:143then reports installability error"in-incognito"whileui-smoke.spec.ts:3614never renders the pdfjs canvas. Both are browser-capability failures in files this PR does not touch; CI installs the bundled browser. Not reproduced against a clean baseline — the exoneration is causal, not empirical..search-bandwas temporarily moved back into@layer componentsand the contract re-run against a production build: it failed withExpected: "2px" / Received: "1px"— 1px being the Tailwindborderutility winning, exactly the chore(deps-dev): bump @types/node from 24.13.2 to 26.0.1 #94 mechanism.globals.csswas restored and the contract passes again. A gate never observed to fail is not yet evidence of anything.rmSync(recursive)on an unvalidated--dirand would have deleted the repository on--dir .. Details in the PR comments.Verification not run: npm run verify:pr-local— it addsformat:check, a conditional production build and RAG fixture validation on top ofverify:cheap; changed files were formatted with Prettier directly and no RAG surface is touched.npm run verify:releasenot run: provider-backed and not authorized.verify:lighthousewas exercised end to end locally against a production build (10 route/strategy measurements, exit 0, correctly warning with no baseline recorded).Risk and rollout
style-contractto the twotestMatchregexes inplaywright.config.ts, which makes one proven spec part of the required gate.style-contractfrom those two regexes, or delete thevisual-baseline/lighthouse-budgetjobs fromci.yml(both already non-blocking).Clinical Governance Preflight
Not applicable — the diff touches no clinical-risk path.
scripts/pr-policy.mjsclassifies it asui+operationalRiskonly (nosrc/lib/**retrieval/answer/privacy surface, nosupabase/**, nosrc/app/api/**), and no RAG-ranking protected surface, so noRAG impact:declaration is required either.Notes
visual-baselinefails by design; the job uploads its artifact on every run so those PNGs can be reviewed and committed. Paths are platform-scoped because font hinting and antialiasing differ between a developer machine and theubuntu-24.04runner.lighthouse-budget.jsonshipsenforce: falsewithbaseline: null, so the gate warns until someone runsnpm run check:lighthouse-budget -- --updateagainst a known-good build and flipsenforce. Do not flip it before the Therapy Compass finding below, or the baseline pins a known-slow route./therapy-compassmobile LCP 5229 ms against 2123–2460 ms on every other mobile route, caused by a 139 KB-gzipped catalogue index fetched client-side. Recorded as ledger#116with the per-field breakdown and the one decision that blocks the fix.docs/process-hardening.md: 40 of the 41 unlayered classes carry exemptions rather than effect contracts (chore(deps-dev): bump @types/node from 24.13.2 to 26.0.1 #94 stays open until the load-bearing ones have contracts), andscripts/run-lighthouse-budget.mjsduplicates ~50 lines of the isolated-server boot fromscripts/run-playwright.mjs.Generated by Claude Code