test(visual): separate "no baseline yet" from "the pixels moved"

[BigSimmo]

Summary

Targets PR #1404's branch, not main — tests/ui-visual-baseline.spec.ts only exists there. Merge or cherry-pick into #1404; nothing here is useful on its own.

Visual baselines (advisory) 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 #1404 documents it. The design is what this changes.

Right now "no baseline committed" and "a real visual regression" produce the same red. Because the first state persists indefinitely, the job is red on every run — so the day a genuine regression appears it looks exactly like the eleven runs before it. A check that is always red is a check nobody reads. That is ledger #095's failure mode reached from the other direction: #95 is a false red people learned to ignore, this is a by-design red people would learn to ignore.

The two states are now distinct:

baseline	declared in AWAITING_BASELINE	outcome
absent	yes	skipped, candidate PNG written and attached for review
absent	no	fails — a deleted, renamed or mis-pathed golden stays loud
present	—	compared exactly as before

Two things worth calling out, because both are places a weaker version of this would have gone wrong:

• 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 that retries: 0 exists to close in playwright.visual.config.ts. The baseline directory stays something only a human writes to.
• The declaration cannot rot into a permanent exemption. declares no baseline it already has fails if a declared target already has a committed golden (a stale entry silently skipping a real comparison), and declares only real targets fails on a name matching no target (a typo that exempts nothing while looking like it exempts something). This mirrors the contract-or-exemption registry Add visual regression, style-contract and pre-merge performance gates #1404 already uses for style contracts.

Adopting a baseline stays a one-commit operation: add the PNG, delete the name from the list.

Verification

Proved the decision table by execution, not by inspection. A standalone harness reproduced the branching against the same snapshotPathTemplate, with one target given a committed baseline and one left undeclared:

RESOLVED declared-missing     -> tests/__screenshots__/linux/declared-missing.png (exists=false)
RESOLVED undeclared-missing   -> tests/__screenshots__/linux/undeclared-missing.png (exists=false)
RESOLVED declared-but-present -> tests/__screenshots__/linux/declared-but-present.png (exists=true)

2 failed
  undeclared-missing matches its baseline      <- missing + undeclared stays red
  declares no baseline it already has          <- stale declaration caught
1 skipped
  declared-missing matches its baseline        <- the state that was previously red
2 passed

testInfo.snapshotPath() resolves to tests/__screenshots__/linux/<name>.png, matching the paths in the failing run's log (/home/runner/work/Database/Database/tests/__screenshots__/linux/therapy-compass-home.png), so the existence check is reading the same location Playwright compares against.

• npm run typecheck — clean
• npx eslint tests/ui-visual-baseline.spec.ts — exit 0
• npx prettier --check — All matched files use Prettier code style!

Not verified: the real visual-baseline job has not run against this change. .github/workflows/ci.yml triggers on pull_request: branches: [main, "release/**"], so a PR targeting #1404's branch does not start it — only the two repo-wide checks (GitGuardian, Supabase Preview) fire here. An earlier revision of this description claimed this PR's CI run would be the proof; that was wrong and is corrected here. The genuine end-to-end proof comes only once this is on a branch whose PR targets main — i.e. after it lands in #1404, where the job should report 6 skipped where it currently reports 6 failed. Treat the harness above as proof of the decision logic, and that future run as proof of the integration.

npm run test:e2e:visual was also not run end to end locally: it needs a production build and server (~15 min) and this sandbox's Chromium is not the bundled one, so the browser-dependent half would not be trustworthy.

No provider calls; none apply.

Risk and rollout

• Risk: low, and strictly narrowing what is reported red. One test file; no production code, no config, no CI change. The job stays continue-on-error and stays outside pr-required, so its result blocks nothing either way.
• The one thing this trades away is that a baseline deleted at the same time as its name is added to AWAITING_BASELINE would skip rather than fail. That is a deliberate two-part edit visible in the diff, which is the same standard the style-contract exemptions are held to.
• Rollback: revert the commit; the job returns to failing on every missing baseline.
• Provider or production effects: none.

Clinical Governance Preflight

Not applicable — the diff is a single Playwright spec. No ingestion, answer generation, search/ranking, source rendering, document access, privacy, or clinical output path is touched.

• Source-backed claims still require linked source verification before clinical use
• No patient-identifiable document workflow was introduced or expanded
• Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
• Service-role keys and private document access remain server-only
• Demo/synthetic content remains clearly separated from real clinical sources
• Source metadata, review status, and outdated/unknown-source behavior remain conservative
• Deployment classification/TGA SaMD impact checked — no clinical decision-support behaviour changed

RAG impact: no retrieval behaviour change — one Playwright spec; nothing under src/lib/rag/**, clinical-search, ranking-config, the eval harness or the golden fixture is touched.

Notes

This does not adopt the baselines, and adopting them is still the real fix — the gate compares nothing until someone commits the six PNGs. I could not do that from this session: artifact download returns 403 GitHub access is not enabled for this session, and generating them locally is the one approach the spec's own docstring rules out, since font hinting differs from the runner. PR #1405 hit exactly that — its thresholds failed CI by one pixel because labels rasterise wider on ubuntu-24.04.

Checked before proposing this: none of the six targets captures the ModeNav bar that #1405 changes. It portals into universal-header-collapse in the header, while every target clips to #main-content or search-query-ribbon; and the only Therapy target is desktop 1280px, above every density band, where the old CSS already used display: flex. So adopting baselines now would not collide with #1405.

Opened as a separate PR rather than pushed onto #1404 because another session was actively committing there; this way it cannot clobber work mid-review.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f5f6b500-078f-4edf-92d9-bf4ba4c1c275

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ab1b47090

ℹ️ 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".

[chatgpt-codex-connector]

Keep missing baselines failing until adoption

On a clean CI checkout, all six targets are in AWAITING_BASELINE and have no committed PNG, so this unconditional test.skip(true) makes every comparison skipped and lets npm run test:e2e:visual exit successfully even when a UI change visibly regresses those surfaces. This contradicts the fail-loud contract in playwright.visual.config.ts:33-43, the advisory workflow design in .github/workflows/ci.yml:476-485, and the explicit stop condition in docs/outstanding-issues.md:157; the job is already non-blocking via continue-on-error, so missing snapshots can remain failed while still uploading candidates. A clean-checkout run showing six skipped tests and exit code 0 is the smallest proof.

Useful? React with 👍 / 👎.