Align the therapy and services fact cards, clear stale dev output, and re-sign the lexicon - #2731
Align the therapy and services fact cards, clear stale dev output, and re-sign the lexicon#2731BigSimmo wants to merge 4 commits into
Conversation
… before a build Two card grids still carried the layout PR #2670 fixed on the form detail page. Both sit in one stretched grid row, so the card with the most to say sets the height for all of them. - Therapy key facts had the full fault: an interactive card filled its height while a card with no "Tap for detail" line dropped that line entirely, so no two card bottoms agreed. Both variants now lay out in the same three zones, header pinned top, face centred in the shared height, footnote pinned bottom and always occupying a line. - Services priority facts had the milder half: nothing misaligned, but a short card printed its text hard against the top of a tall box. The block is centred instead. Title and detail stay together, since there is no separate footnote zone here to pin to the bottom. Separately, guard-next-build now discards `.next/dev` once it has established no dev server is running. A dev server stopped mid-write leaves a truncated `.next/dev/types/validator.ts`, and `next build` type-checks it, so the build fails with a syntax error in a generated file nobody wrote on a tree where nothing is wrong. That cost a full verify:pr-local run on 2026-09-06. guard-push.mjs already works around the same artefact for Prettier. Production output is untouched, and the cost of being wrong is one slower dev start. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZmPsaAQSnYjtTLJhS5Xdc
…hash The 2026-08-22 sign-off recorded no mappings hash, so nothing bound it to the table it had reviewed and the sheet could not say which of its 37 terms were still covered. Every verify run has warned about it since. Reviewed and accepted by the Repository Lead, and the block now carries the hash. Four things moved since 2026-08-22, none of which reopened a term reviewed then: - `acei` and `antihypertensives` gained ramipril, and `statins` gained simvastatin. No selector was touched: the catalogue grew from 328 to 330 medications with the cardiovascular set, and the already-reviewed selectors resolved the new drugs. - `statins` and `fibrates` gained sourceDenySlugs for simvastatin and atorvastatin, because those drugs' own gemfibrozil rows name their own class rather than a second interacting family. Accepted as a narrowing of the same too-broad shape as the three 2026-08-22 corrections. The scope statement is unchanged and now reads against the recorded hash rather than against a date alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZmPsaAQSnYjtTLJhS5Xdc
…ture cleanup
Two failures from the two commits before this one.
`tests/medication-lexicon-report-signoff.test.ts` pinned the state the sheet
was in when the test was written — an unsigned 2026-08-22 block — under a
name that described that state as temporary ("until it is re-recorded with a
mappings hash"). Re-recording it therefore turned the fix into a failure. The
guard it was actually written for is that the sheet's status line is derived,
so it can never claim a review the recorded hash does not cover, and that
assertion is untouched. The hardcoded date becomes a shape check. Staleness
deliberately stays a generator warning rather than a test failure: an ordinary
lexicon edit must not go red until a clinician is available to re-sign it.
`tests/guard-next-build.test.ts` cleaned its scratch directories with a bare
recursive rmSync, which tests/test-runner-safety.test.ts rejects repo-wide.
Now bounded with maxRetries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZmPsaAQSnYjtTLJhS5Xdc
|
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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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_617ee485-a632-4769-9cb3-8dfe1a2c82ae) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c6cd557b1
ℹ️ 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".
…ev server Codex review on #2731, P2, and correct. `dev-free-port.mjs` honours any PORT or --port, so this checkout's dev server can sit below its stable port — PORT=3130 against a stable 3131. findRunningProjectServer scanned stablePort upward to 4599 only, so it never reached such a server and reported none running. That already let a production build run concurrently with a live dev session; the cleanup added earlier in this PR made the blind spot destructive, clearing `.next/dev` from under that session. Now uses circularProjectPortRange, which wraps at the top of the range. run-playwright.mjs, run-lighthouse-budget.mjs and measure-cls-attribution.mjs already locate the server this way — this script was the one that did not. The new test binds a server on the port below the stable one and asserts it is found. Against the previous upward-only scan it fails with "expected null to be 3416", which is the reported defect exactly. A port outside the project range entirely, PORT=9999, is still missed and cannot be found from here: the build process cannot see the environment the dev server was started in. Said so in the function's own docblock rather than leaving it implied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZmPsaAQSnYjtTLJhS5Xdc
Summary
Three follow-ups from the session that produced #2670, done as one PR because each is small and they do not interact.
guard-next-buildnow discards.next/devonce it has established no dev server is running. A dev server stopped mid-write leaves a truncated.next/dev/types/validator.ts, andnext buildtype-checks it, so the build fails with a syntax error in a generated file nobody wrote on a tree where nothing is wrong. That cost a fullverify:pr-localrun on 2026-09-06.guard-push.mjsalready works around the same artefact for Prettier. Production output (.next/server,.next/static,.next/types) is untouched, and the cost of being wrong is one slower dev start.The other half of the container-quirk item needed nothing: the shallow-clone deepening landed on
mainearlier the same day in 5c6f196, so the threereviewedCommit does not existfailures no longer occur.Verification
npm run verify:pr-localAll 40 steps green on the head of this branch —
failed: (none),not reached: (none)— includinglint,typecheck, the full offline unit suite,build, and the trailingcheck:rag:fixtures/check:medication-interactions/check:medication-lexicon-reportsteps. The lexicon check no longer prints the stale-sign-off warning.An earlier run of the same gate failed two tests, both caused by the first two commits here; the third commit fixes them and is described below.
npm run verify:ui— not run locally by cost policy.npm run plan:browserfails an unattributable UI change closed to the full suite, which is exactly what Production UI runs on a non-draft PR. Visual proof was taken instead by driving Chromium against the local dev server: the therapy record page and a services detail page at 1440px, both showing the four cards aligned.npm run eval:retrieval:quality— not applicable; no retrieval, ranking, selection, chunking or scoring behaviour changed.npm run check:production-readiness— not applicable; no clinical workflow, privacy, environment, Supabase, deployment or answer behaviour changed. Source governance is touched only in that a clinical sign-off is recorded, which is the subject of the preflight section below.Risk and rollout
/therapy-compass/[slug]and/services/[slug], one local build-tooling change that cannot run in CI's path before a build, and one document. No runtime, data, schema or configuration change.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)The sign-off is a clinical record, so what it now asserts is set out in full. It was reviewed and accepted by the Repository Lead before being recorded. Four things had moved since 2026-08-22, none of which reopened a term reviewed then:
aceiandantihypertensivesgained ramipril, andstatinsgained simvastatin. No selector was touched — the catalogue grew from 328 to 330 medications with the cardiovascular set, and the already-reviewed selectors resolved the new drugs.statinsandfibratesgainedsourceDenySlugsfor simvastatin and atorvastatin, because those drugs' own gemfibrozil rows name their own class rather than a second interacting family. Accepted as a narrowing of the same too-broad shape as the three corrections made on 2026-08-22.The document's scope statement is unchanged and now reads against the recorded hash rather than against a date alone.
Notes
tests/medication-lexicon-report-signoff.test.tspinned the state the sheet was in when the test was written — an unsigned 2026-08-22 block — under a name calling that state temporary ("until it is re-recorded with a mappings hash"), so re-recording it turned the fix into a failure. The guard it was actually written for is that the status line is derived and cannot claim a review the recorded hash does not cover; that assertion is untouched and the hardcoded date becomes a shape check. Staleness deliberately stays a generator warning rather than a test failure, so an ordinary lexicon edit does not go red until a clinician is available to re-sign it.tests/guard-next-build.test.tsgains three cases for the new cleanup, including one proving production build output survives it. Its scratch-directory cleanup is bounded withmaxRetries, pertests/test-runner-safety.test.ts.🤖 Generated with Claude Code
https://claude.ai/code/session_01DZmPsaAQSnYjtTLJhS5Xdc
Generated by Claude Code
Note
Low Risk
Presentation and local build-tooling only, plus a clinical governance document; no runtime API, data, or interaction-routing code changes in this diff.
Overview
Three independent follow-ups: UI alignment on therapy and service detail fact cards, local build guard cleanup, and clinical lexicon sign-off documentation.
Therapy key facts now use the same three-zone layout as form priority-fact cards: header top, face vertically centred in the shared grid height, and a bottom footnote line that always reserves space (
CardFootnotewith a non-breaking blank when empty). Interactive cards useitems-stretchso “Tap for detail” stays pinned at the bottom and row baselines match non-interactive neighbours.Service priority summary cards add
flex flex-col justify-centerso short cards centre their content in a row sized by the tallest card instead of hugging the top.guard-next-buildexports and runsdiscardDevServerTypes()after confirming no dev server is running, recursively removing.next/devso truncated dev-generatedvalidator.tscannot breaknext buildtypecheck. Production paths under.next/server,.next/static, and.next/typesare untouched.The medication interaction lexicon review sheet is re-signed 2026-09-06 with a mappings hash, documents acceptance of four interim mapping changes (ramipril/simvastatin catalogue growth and
sourceDenySlugson statins/fibrates), and updates the status line.medication-lexicon-report-signoff.test.tsstops pinning a specific stale date and instead asserts the status line matches the derived hash check.Reviewed by Cursor Bugbot for commit 8c6cd55. Configure here.