Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2b21023
Add visual regression, style-contract and pre-merge performance gates
claude Jul 30, 2026
afad829
Pass projectRoot to the run coordinator in the Lighthouse runner
claude Jul 30, 2026
b72dd63
Build the Lighthouse app into the guarded isolated output path
claude Jul 30, 2026
476ed49
Merge remote-tracking branch 'origin/main' into claude/mature-repo-te…
claude Jul 30, 2026
1aaf513
Merge branch 'main' into claude/mature-repo-testing-setup-blb7h6
BigSimmo Jul 30, 2026
a6f5fa6
issues: capture Therapy Compass mobile LCP finding (#116), baseline a…
claude Jul 30, 2026
9ce641d
issues: refine #118 — PyMuPDF pin is valid, so exclude it as the Circ…
claude Jul 30, 2026
3c0fd4d
Fix three Codex review findings in the new gates
claude Jul 30, 2026
1c746c8
Confine the report-directory delete and stop retries adopting a writt…
claude Jul 30, 2026
03051ed
docs(testing): scope the style-contract inventory claim to single-lin…
claude Jul 30, 2026
b6e21b1
Remove the unproven variant assertion from the style contract
claude Jul 30, 2026
5e7efad
Close the remaining Codex findings on the new gates
claude Jul 30, 2026
b9892c9
Merge branch 'main' into claude/mature-repo-testing-setup-blb7h6
BigSimmo Jul 30, 2026
5e9a4f3
merge(main): sync PR #1404; renumber issues past main #115/#116
cursoragent Jul 30, 2026
f7f74af
docs(issues): advance next-id to 125 after cross-PR reservation
cursoragent Jul 30, 2026
db25021
merge(main): sync PR #1404 with latest main
cursoragent Jul 30, 2026
ac5d94a
Merge branch 'main' into claude/mature-repo-testing-setup-blb7h6
BigSimmo Jul 30, 2026
368af2e
test(visual): separate "no baseline yet" from "the pixels moved" (#1417)
BigSimmo Jul 30, 2026
1aa863f
Merge remote-tracking branch 'origin/claude/mature-repo-testing-setup…
claude Jul 30, 2026
5287a71
Merge branch 'main' into claude/mature-repo-testing-setup-blb7h6
BigSimmo Jul 30, 2026
233b358
Merge remote-tracking branch 'origin/main' into claude/mature-repo-te…
claude Jul 30, 2026
64007bd
Merge remote-tracking branch 'origin/main' into claude/mature-repo-te…
cursoragent Jul 30, 2026
282f81e
issues: capture the CI-only phone-scroll hide failure (#125)
claude Jul 30, 2026
ceae168
Merge origin branch head; renumber the colliding #125 to #126
claude Jul 30, 2026
5b538c4
Merge origin/main; renumber the colliding #126 to #127
claude Jul 30, 2026
0af5ea2
Merge branch 'main' into claude/mature-repo-testing-setup-blb7h6
BigSimmo Jul 30, 2026
84c1f6d
test: consolidate rendered style contracts
BigSimmo Jul 30, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,84 @@ jobs:
playwright-report/
if-no-files-found: ignore

# Pixel baselines (tests/ui-visual-baseline.spec.ts). Advisory on purpose, and NOT
# part of `pr-required`:
# - Baselines are platform-specific, so the FIRST run on a platform with no
# committed baseline fails with "snapshot doesn't exist" by design. Adopt the
# images from this job's artifact — never from a developer machine, or font
# hinting alone makes every later run red.
# - Pixel comparison needs a soak before it can block a merge; this repo has
# already paid for a sub-pixel rounding flake (the min-h-11 -> min-h-12 change).
# Promote to a required check by adding it to `pr-required` once the baselines have
# held across a few runs, and drop `continue-on-error` at the same time.
visual-baseline:
name: Visual baselines (advisory)
needs: changes
if: needs.changes.outputs.ui_changed == 'true'
Comment thread
BigSimmo marked this conversation as resolved.
continue-on-error: true
runs-on: ubuntu-24.04
timeout-minutes: 45
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false

- name: Setup UI e2e environment
uses: ./.github/actions/setup-ui-e2e

- name: Chromium visual baselines
run: npm run test:e2e:visual

# The actual/diff/expected PNGs live here. On a first run these are the images
# to commit as the baseline; on a later run they are the evidence of what moved.
- name: Upload visual diffs
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
with:
name: visual-baseline-${{ github.run_id }}
path: |
test-results/
tests/__screenshots__/
if-no-files-found: ignore

# Pre-merge performance budget against a LOCAL production build, graded relative to
# the committed baseline in lighthouse-budget.json. Advisory until that baseline
# exists and `enforce` is flipped to true — see the notes in that file.
#
# Distinct from .github/workflows/live-web-vitals.yml, which measures the deployed
# origin for ledger #017 and by then cannot stop a regression from merging. Uses no
# secrets and no providers: the server runs in demo mode with inert loopback
# Supabase values, exactly like the Playwright runner.
lighthouse-budget:
name: Lighthouse budget (advisory)
needs: changes
if: needs.changes.outputs.ui_changed == 'true' || needs.changes.outputs.build_changed == 'true'
Comment thread
BigSimmo marked this conversation as resolved.
continue-on-error: true
runs-on: ubuntu-24.04
timeout-minutes: 45
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false

# Lighthouse drives the Chrome that ships in the ubuntu-24.04 runner image, so
# no browser install is needed here (matching live-web-vitals.yml).
- name: Setup Node and dependencies
uses: ./.github/actions/setup-node-cached

- name: Measure routes and grade against the baseline
run: npm run verify:lighthouse -- --keep --dir lighthouse

- name: Upload Lighthouse reports
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
with:
name: lighthouse-budget-${{ github.run_id }}
path: lighthouse/
if-no-files-found: ignore

db-reset-verify:
name: Migration replay
needs: changes
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@
/.next-playwright/
/out/

# Lighthouse budget reports (scripts/run-lighthouse-budget.mjs). Regenerated per run;
# the committed artefact is the baseline inside lighthouse-budget.json, not the JSON
# reports. Visual baselines under tests/__screenshots__/ ARE tracked deliberately.
/lighthouse/

# production
/build

Expand Down
6 changes: 5 additions & 1 deletion docs/outstanding-issues.md

Large diffs are not rendered by default.

41 changes: 41 additions & 0 deletions docs/process-hardening.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,47 @@ All approved render-surface modules are extracted. `ClinicalDashboard.tsx` went
- Add explicit review ownership for clinical source governance, outdated-source handling, incident review, and decommission decisions.
- Record production-readiness outcomes in release notes whenever clinical workflow, source governance, privacy, or deployment assumptions change.

## Visual regression, style contracts and the pre-merge performance budget (2026-07-30)

Three gates added for the "mature repo" verification pass. Full usage is in
[`docs/testing.md`](testing.md); this records the reasoning and the remaining debt.

- **Problem addressed.** Nothing verified rendered appearance. `playwright.visual.config.ts` and
`npm run test:e2e:visual` existed but pointed at `ui-visual-artifacts.spec.ts`, which only
`testInfo.attach()`es four screenshots for a human to eyeball — there was no `toHaveScreenshot`
call and no baseline directory anywhere in the repo. Separately, ledger #094 showed a style
contract passing while the style was inert, and Lighthouse only ran against the deployed origin,
after a merge had already auto-deployed.
- **Style contracts are required and deterministic.** `tests/ui-style-contract.spec.ts` joins
`test:e2e:pr` through `productionSpecPattern`, so it blocks like any other production journey. It
is Chromium-only by design (computed-style serialisation is engine-specific) and self-skips on the
other engines in the release matrix. `tests/style-contract-registry.test.ts` closes the inventory
so the next unlayered class cannot be added unnoticed.
- **Verified it bites, not just that it passes.** The contract was re-run against a production build
with `.search-band` deliberately moved back into `@layer components`; it failed on
`borderTopWidth` as intended, and passed once reverted. A gate that has never been observed to
fail is not yet evidence of anything.
- **Pixel baselines are advisory on purpose.** `visual-baseline` in CI is `continue-on-error` and
outside `pr-required`. Two reasons: no baselines are committed yet (the first run's artifact is
what supplies them), and this repo has already paid for a sub-pixel rounding flake — the
`min-h-11` → `min-h-12` tap-target change. Promote by adding the job to `pr-required` and dropping
`continue-on-error` in the same edit, once baselines have held across a few runs.
- **Baselines must come from CI, not a laptop.** Paths are platform-scoped
(`tests/__screenshots__/{platform}/`). Font hinting and antialiasing differ between a developer
machine and the `ubuntu-24.04` runner, so a locally-generated baseline would make every CI run red.
- **The performance budget is relative, not absolute.** `lighthouse-budget.json` holds a committed
per-route baseline and a per-metric tolerance, following `check:bundle-budget`. Absolute
web-vitals thresholds are meaningless against localhost with no network latency. `enforce` starts
`false` with no baseline; incomplete evidence fails regardless of `enforce`.
- **Verification debt remaining.** (1) No pixel baselines are committed, so `visual-baseline` reports
rather than gates until an operator adopts them from the CI artifact. (2) No Lighthouse baseline is
recorded, so the budget warns rather than grades until `--update` runs against a known-good build.
(3) 37 of the 38 unlayered visual classes carry exemptions rather than effect contracts — the debt
is enumerated in `STYLE_CONTRACT_EXEMPTIONS`, and ledger #094 stays open until the load-bearing
ones have contracts. (4) `scripts/run-lighthouse-budget.mjs` duplicates roughly 50 lines of the
isolated-server boot in `scripts/run-playwright.mjs`; extracting a shared module was deliberately
deferred rather than destabilise the required UI gate in the same change.

## Text formatting and copy conventions

- **Document-derived text must never be rendered raw.** Any value pulled from an ingested document — answer prose, exact quotes, source snippets, document titles, image captions, extracted table text — must be routed through a `source-text-sanitizer` (`src/lib/source-text-sanitizer.ts`) or `display-text` (`src/components/clinical-dashboard/display-text.ts`) helper before it reaches JSX. Verbatim quotes use `sourceTextForVerbatimQuote`; titles use `cleanDisplayTitle`; snippets/captions use `sourceTextForCompactDisplay`.
Expand Down
66 changes: 66 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ Ordinary Vitest and Playwright runs remove OpenAI, Supabase, database, and E2E c
| `npm run verify:pr-local` | PR-like local gate. Formatting is checked on the changed set, the full unit suite runs once, and RAG scope adds fixture/manifest validation. |
| `npm run verify:phone-chrome` | Smart phone-chrome gate: lock parity, affected contracts, browser/PWA owners and exact journeys, then full UI only for shared foundations. |
| `npm run verify:ui` | Complete required production Chromium gate. |
| `npm run test:e2e:style-contract` | Focused rendered-effect assertions for the unlayered classes in `globals.css` (also runs inside `test:e2e:pr`). |
| `npm run test:e2e:visual` | Pixel baselines. Advisory in CI; a platform with no committed baseline fails by design. |
| `npm run test:e2e:visual:update` | Rewrite the pixel baselines for the current platform. Review every changed PNG before committing. |
| `npm run verify:lighthouse` | Build, serve, and measure the budgeted routes with Lighthouse, then grade against the committed baseline. `-- --dry-run` prints the plan. |
| `npm run check:lighthouse-budget` | Grade Lighthouse JSON that already exists. `-- --update` refreshes the baseline in `lighthouse-budget.json`. |

Set `FAST_CHECK_SEED` to reproduce a property-test run. Local and ordinary CI runs default to `424242`; scheduled CI may derive a bounded seed from the run ID.

Expand Down Expand Up @@ -54,6 +59,64 @@ Blocking tests run with zero retries. CI publishes list, JUnit, and JSON reports

Phone-chrome work uses `npm run verify:phone-chrome`. Inspect its classification with `-- --dry-run` or provide an explicit changed set with `-- --files pathA,pathB`. The default `--full=auto` escalates shared shell/header/footer, scroll-coordinator, reserve, or global-style changes to `verify:ui` only after focused ownership and journey checks pass. Page-local owners and test-helper changes remain focused; use `--full=always` for deliberate extra confidence or `--full=never` only when the dry run records why the recommended broad gate is unavailable. Physical Safari and cold-launch PWA paint still follow [phone-chrome-physical-acceptance.md](phone-chrome-physical-acceptance.md).

## Visual regression and style contracts

Appearance is verified at two levels, because they fail differently.

**Style contracts (`tests/ui-style-contract.spec.ts`) — required, deterministic.** Class rules in
`globals.css` that sit outside `@layer` are there to beat a Tailwind utility. If one is moved into a
layer it goes inert: still in the DOM, still in the class list, painting nothing. That is ledger
#094 — PR #1316 shipped the search band's accent rail inert, and the test guarding it asserted
`toHaveClass("search-band")`, i.e. the cause rather than the effect. jsdom cannot catch this (it does
not implement cascade layers) and `check:design-system-contract` cannot either (it reads source
text), so the assertions run in a real browser and read computed style. Where a rule has an
attribute-scoped variant, the contract also proves the variant wins the cascade.

`tests/style-contract-registry.test.ts` keeps the inventory closed: every unlayered visual class must
appear in `STYLE_EFFECT_CONTRACTS` or carry a reasoned exemption in
`tests/helpers/style-contracts.ts`. A newly-added unlayered class fails that test until someone
chooses which it is — the missing piece before, when the one existing rail assertion was a one-off.
Prefer deleting an exemption by adding a contract.

The parser walks back over comma-continued selector lines, so a selector list split across lines inventories every class in it, not just the one on the line that opens the block. Closing that hole immediately surfaced three previously-unpoliced classes — `dashboard-composer-edge`, `edge-glass-header` and `medication-also-matches` — which is the gate doing its job.

**Pixel baselines (`tests/ui-visual-baseline.spec.ts`) — advisory.** Run by
`playwright.visual.config.ts`, which also still runs the older attach-only
`ui-visual-artifacts.spec.ts`. Three constraints are deliberate: never `fullPage` (under CI load
Next.js leaves a hidden duplicate page root in the stream — ledger #093 — so a whole-page capture can
contain the layout twice; every target is clipped to a locator), demo mode only (the Playwright
runner forces `NEXT_PUBLIC_DEMO_MODE` and offline providers, so content is stable between runs), and
motion off with carets hidden.

Baselines are committed per platform (`tests/__screenshots__/{platform}/`). **Adopt them from the CI
job's artifact, not from a developer machine** — font hinting and antialiasing differ, and a
laptop-generated baseline makes every CI run red. A platform with no baseline fails loudly rather
than passing silently. The CI job is `continue-on-error` until the baselines have held across a few
runs; promote it by adding it to `pr-required` and dropping that flag together.

## Performance budget

`npm run verify:lighthouse` builds and serves an isolated production app in demo mode, measures the
routes in `lighthouse-budget.json` on mobile and desktop, and grades the result. It is a **relative**
gate: absolute web-vitals thresholds are meaningless against a localhost server with no network
latency, so each route is compared to a committed known-good baseline with a per-metric tolerance,
following the same shape as `check:bundle-budget`.

- No baseline recorded → warn, exit 0. Within tolerance → pass. Over tolerance → fail when
`enforce` is true, warn otherwise.
- Incomplete evidence — a route that produced no report, a report with no LCP/CLS, or a report that
measured a different page after a redirect — **always fails**, regardless of `enforce`. An ungraded
route counted as a pass is the failure mode `summarise-web-vitals.mjs` documents at length.
- CLS is graded on absolute movement; LCP and TBT need to clear both a percentage and an absolute
floor, so 12 ms → 16 ms is not reported as a 33% regression.

Refresh the baseline deliberately after a known-good run: `npm run check:lighthouse-budget -- --update`.

This is distinct from `.github/workflows/live-web-vitals.yml`, which measures the deployed origin for
ledger #017 and is dispatch-only — by the time it runs, `main` has already auto-deployed. Both pin
the same Lighthouse version, and `tests/check-lighthouse-budget.test.ts` fails if they drift apart.
Neither uses secrets or providers.

## Flake policy

`tests/flake-ledger.json` may be empty. Each entry must match the exact spec and title, and the test title must include `@quarantine` but not `@critical`. Entries require an owner, reproduction command, local tracking reference, first/last-seen dates, and an expiry no more than 30 days away. Reproduce a candidate three times on the same SHA before adding or retaining it: fix fail/pass races, treat repeatable failures as regressions, and remove entries that no longer reproduce.
Expand All @@ -62,6 +125,8 @@ Phone-chrome work uses `npm run verify:phone-chrome`. Inspect its classification

PR CI keeps static checks separate from one required full unit run with coverage. UI scope runs a fail-fast `@critical` Chromium job on pull requests, then one required full production Chromium invocation (`test:e2e:pr`) for non-quarantined journeys, plus one advisory invocation for quarantined and mockup journeys. `src/app/api/**` does not set `ui_changed` or `db_changed` — API handlers stay on unit/coverage (and offline RAG when retrieval-scoped). The `PR required` aggregate keeps `if: always()` and distinguishes `cancelled` from `failure` in its messages (stays red; a skipped required check would count as passing). Secret Scan pins Gitleaks to the workflow event base/head SHAs and the checked-out commit, and verifies the linux_x64 release tarball against a pinned SHA-256 before install. The weekly `release-browser-matrix` depends on static/build/UI success, not on the full aggregate, so a blocking scheduled dependency audit cannot skip Firefox/WebKit. Container scope calls the reusable Docker workflow and requires both app and worker image builds through the `pr-required` aggregate. Build, migration, safety/RAG, and release behavior remain independently scoped.

Two further jobs are advisory (`continue-on-error`, deliberately outside `pr-required`): `visual-baseline` on UI scope and `lighthouse-budget` on UI-or-build scope. Both upload their evidence on every run, pass or fail, because the artifact is the whole point on a first run — the baselines to adopt and the reports to grade. Promote either to required by adding it to `pr-required` and removing `continue-on-error` in the same edit.

## Contribution checklist (UI changes)

Before opening a UI PR, confirm:
Expand All @@ -71,5 +136,6 @@ Before opening a UI PR, confirm:
- **States.** Handle loading / empty / error / disabled where they apply; async surfaces expose a retry, not a dead end.
- **Accessibility** ([design-system §7](./design-system.md)): keyboard operable, visible focus, accessible names on icon controls, live regions for async status, and reduced motion honoured — scripted `scrollTo`/`scrollIntoView` go through `resolveScrollBehavior` (`src/lib/scroll-behavior.ts`), never a hard-coded `behavior: "smooth"`.
- **Tests.** Add a `.dom.test.tsx` for changed component behaviour (see "Component tests" above) and update the E2E journeys for changed flows.
- **Unlayered CSS.** If the change adds a class rule outside `@layer` that sets a border, background, colour, shadow or outline, `tests/style-contract-registry.test.ts` will fail until it is registered. Add a rendered-effect contract rather than an exemption where the rule matters visually — see "Visual regression and style contracts".
- **Verify** ([design-system §9](./design-system.md)): run `npm run verify:cheap`, then `npm run verify:pr-local` before handoff; run `npm run ensure` before browser work and `npm run verify:ui` for UI/routing/styling changes, plus a manual dark-mode + forced-colors spot check on touched surfaces.
- Architecture and state-ownership conventions: [`docs/frontend-architecture.md`](./frontend-architecture.md).
Loading
Loading