Therapy: build the compare set from where the therapies are, on a phone - #2387
Conversation
Choosing therapies to compare on a phone was the wrong shape twice over. `/therapy-compass/compare` drew four empty slots before anything was chosen, and once two were chosen the comparison was a `min-w-[720px]` table inside a horizontal scroller: on a 390px phone that is two thirds of a column at a time, with the field labels scrolling away from the values they label. Four directions were prototyped as design scratch (PR #2339); this builds the chosen one for real. The model is that you fill the compare set *where the therapies are* — from a list, a search result, or while reading a record — and the comparison screen is only ever reached with something already in it. Adding no longer navigates. `ResultCard` moves from `toggleCompare` to `addCompare`/`removeCompare`, and the label moves with the behaviour: "Compare" promised a destination it no longer goes to, so it reads "Add to compare" / "In compare tray". `toggleCompare` is removed from the bindings API rather than left as a dead navigate-on-add path, and the record nav rail's Compare slot becomes pure navigation in both branches — one control with two different meanings, depending on state the reader could not see, was the problem it existed to demonstrate. The tray docks above the phone search pill through the documented addon slot, so it inherits the dock's fixed position, z-index, safe-area padding and scroll-hide transform: no second scroll listener and no bottom-offset arithmetic. Two invariants hold it there. It stays exactly one row tall — the dock's content clearance is a static token, so a dock that grows covers page content by exactly its own growth, which is why the expanded state is a bottom Sheet rather than the taller bar the prototype drew. And the shell claims the slot only while the URL carries a compare set, because the reserve inflates on claim rather than on render, so a claim with an empty tray would open a blank band under a row that is not there. Record routes have no search pill, so they get an "Add to compare" button in the page body instead — page content, not chrome, introducing no second bottom owner. It announces through `role="status"`: with no tray on screen, an add there would otherwise be completely silent. The comparison forks at `md`, not `sm`. The table is 720px wide, so at 640–767px it would still scroll sideways — the exact defect being fixed. Below that width the same `rows` memo is turned inside out: one card per field, every therapy listed against it, so the label never leaves the value. Device memory is additive and the URL stays the source of truth. A shared `?ids=` link always wins; memory only fills the gap when you arrive with no `ids` at all. Remembered slugs are validated against the live catalogue at restore time, so one from an older data generation is dropped rather than resurrected as a therapy that no longer exists. `data-therapy-scroll-sm` and its globals.css rule are removed together: phones no longer render that table, so the rule could not match anything and the contract assertion on it would have passed for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WKxKU17WQqryM8iNhpyJk7
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 95 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (33)
Comment |
Travels with its owning product PR rather than on a ledger-only tip, per AGENTS.md PR bundling. Pushed seconds after the PR opened so the restarted CI run loses no meaningful work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WKxKU17WQqryM8iNhpyJk7
Gitleaks' generic-api-key rule flagged the exported browser-storage key name for the compare set. It is a localStorage key — visible in any browser's devtools, never a credential. The rule matched only because the identifier ends in "Key" and the hyphenated slug measures 3.99 Shannon entropy; its five siblings (answer-thread, recent-queries, saved-*, search-pins) are the same shape at 3.72-3.78, just under the threshold. Pinned by fingerprint rather than reworded. Rewording would be tuning an unverifiable number — gitleaks is not installed in this container, so the threshold cannot be checked locally — and every candidate slug still measured above the passing siblings, so it would trade a good name for a worse one and no certainty. The .gitleaksignore file exists for exactly this, with ten prior entries against the same rule. The scanner is not weakened: this pins one commit's one line, not the rule or the path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WKxKU17WQqryM8iNhpyJk7
Static PR checks went red on `check:repo-awareness-snapshot`: the immutable review record committed alongside this change moves the review count, and the generated inventory is derived from it, so the two must be committed together. Regenerated with the repo's own tooling, never by hand. Verified in step (192 pages, 436 documents, 2617 reviews), with sitemap:check, docs:check-index, check:ledger-write-discipline, check:branch-review-ledger and prettier all clean before pushing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WKxKU17WQqryM8iNhpyJk7
…e-tray # Conflicts: # data/repo-awareness-snapshot.json
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:9b34a01497
ℹ️ 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".
| const therapyCompareAddonActive = | ||
| searchMode === "therapy-compass" && | ||
| isTherapyPhoneDockRoute(pathname) && | ||
| readTherapyCompareSlugCount(searchParams) > 0; |
There was a problem hiding this comment.
Claim the dock only when the tray can render
On a phone route with ids while the catalogue is pending or failed, or when a shared link contains only retired slugs, this condition claims the addon and inflates the 9rem reserve, but TherapyCompareTray derives its count from resolved compareTherapies and returns null at zero. This leaves a tray-sized blank band, permanently for failed loads or retired links; base the claim on renderable selections or render a placeholder, and cover the pending/error/unknown-ID paths.
AGENTS.md reference: AGENTS.md:L525-L525
Useful? React with 👍 / 👎.
| if (!raw) return []; | ||
| try { | ||
| return normalizeTherapyCompareSlugs(JSON.parse(raw)); |
There was a problem hiding this comment.
Prefer the session fallback after failed writes
When localStorage.setItem throws because storage is full or write-blocked while getItem remains readable, the writer saves the current set in inMemorySlugs, but the next read returns an empty or stale persisted value instead of that fallback. Leaving and re-entering Therapy in the same tab therefore loses the latest comparison; a storage stub whose setItem throws and getItem returns null reproduces this, and reads should prefer the pending in-memory value until a successful write clears it.
Useful? React with 👍 / 👎.


Summary
Choosing therapies to compare on a phone was the wrong shape twice over.
/therapy-compass/comparedrew four empty slots before anything had been chosen, and once two were chosen the comparison was amin-w-[720px]table inside a horizontal scroller — on a 390px phone that is about two thirds of a column at a time, with the field labels scrolling away from the values they label.Four directions were prototyped as design scratch in PR #2339; this builds the chosen one for real, on Therapy Compass only. The model is that you fill the compare set where the therapies are — from a list, a search result, or while reading a record — and the comparison screen is only ever reached with something already in it. Empty slots are never drawn.
ResultCard(src/components/therapy-compass/therapy-card.tsx) moves fromtoggleComparetoaddCompare/removeCompare, and the label moves with the behaviour: "Compare" promised a destination it no longer goes to, so it now reads "Add to compare" / "In compare tray".toggleCompareis removed from the bindings API rather than left behind as a dead navigate-on-add path. A full tray takesaria-disabledplus a stated reason rather than silently ignoring the tap, per the button-wiring convention.therapy-record-nav-header.tsx). It previously added the therapy when it was absent from the set and navigated when it was present — one control with two different meanings depending on state the reader could not see.src/components/therapy-compass/therapy-compare-tray.tsxportals into the documented phone dock addon slot, so it inherits the dock'sposition: fixed, z-index, safe-area padding and scroll-hide transform; there is no second scroll listener and no bottom-offset arithmetic. It registers a thirdPhoneDockAddonKind(therapy-compare) across all four required sites: the slot id insrc/lib/mode-home-composer.ts, the reserve constant and both resolver branches inmobile-composer-reserve.ts, the clearance tokens / backdrop scrim heights / hide-transform overshoot inglobals.css, and the claim inglobal-search-shell.tsx.record/compare-action.tsxis therefore ordinary page content, introducing no second bottom owner, and announces throughrole="status"— with no tray on screen an add there would otherwise be completely silent.compare-screen.tsxkeeps today's table frommdup, unchanged, and below it renders the samerowsmemo turned inside out: one card per field, every selected therapy listed against it, so the label never leaves the value. The fork ismd(768px) rather thansm, because the table is 720px wide and at 640–767px it would still scroll sideways — the exact defect being fixed.src/lib/therapy-compare-memory.ts, wired intoTcProvider. This is strictly additive: the URL stays the source of truth, a shared?ids=link always wins, and memory only fills the gap when you arrive with noidsat all. Remembered slugs are validated against the live catalogue at restore time, so a slug from an older data generation is dropped rather than resurrected as a therapy that no longer exists. Cross-tab sync is a deliberate non-goal — a remote write would rewrite this tab's address bar mid-comparison.data-therapy-scroll-smand itsglobals.cssrule are removed together. Phones no longer render that table, so the rule could not match anything, and the existing contract assertion on the attribute would have kept passing for the wrong reason. The assertion now pins the new phone behaviour instead.Desktop is deliberately unchanged, and no other mode is touched — Dictionary, DSM, formulation and specifiers keep today's behaviour, even though
src/components/compare/is shared with them.Verification
npm run verify:pr-local— Verification not run: superseded by the gates below.npm run verify:cheapran on this exact tree and covers the failure classes this diff can plausibly break (lint, typecheck, the full offline unit suite, and the 34 static/consistency gates), and the browser evidence below is stronger than the conditional build step PR-local would have added. CI's own routing remains the authority.npm run verify:ui— pass, vianpm run verify:phone-chromeon this exact commit. That wrapper classified the change as touching shared phone-chrome foundations (src/app/globals.css+mobile-composer-reserve.ts) and escalated to the complete Chromium gate rather than stopping at focused owners:521 passed (17.3m), exit code 0.npm run verify:cheap— pass, exit code 0, on this commit and this base:Test Files 876 passed (876)·Tests 10547 passed | 1 skipped (10548), plus lint, typecheck and the 34 static/consistency gates.npm run lintseparately:[gate-receipts] recorded a pass for "lint:internal" (5450 input files).The two journeys that carry this change's own contract are inside that 521. They were also run in isolation during development, against a clean production build:
tests/ui-therapy-nav-scroll.spec.ts→2 passed (18.9s), including the newphone compare tray hides with the composer and releases its reserve(drives/therapy-compass/searchat 390×844 with a simulated 34px bottom inset, asserts the tray is a descendant of.answer-footer-search-dock, thatdata-footer-addon="therapy-compare"is set, that after scrolling the tray's top clears the viewport and--mobile-composer-reservereads exactly0rem, and that both return on scroll-up); andtests/ui-route-coverage.spec.ts→13 passed (33.7s), including the rewritten card journey which now asserts the opposite of what it used to — that activating the compare control does not navigate, leaves you on/therapy-compass/searchwithidswritten to the URL, and makes the tray appear.Two design-system gates went red during development and were repaired properly rather than suppressed:
check:design-system-contractflagged the tray's raw<button>elements and a rawgapliteral, so the controls moved onto the sharedButton/interactiveRowBaseand the spacing onto tokens;style-contract-registryrequired every new unlayered visual class to be either contracted or explicitly exempted, so the five that remain carry a reasoned exemption naming the coverage that does exist (the tray is phone-only and therefore unreachable at the desktop viewportui-style-contract.spec.tsdrives — the same reason the Patient details pill is exempt).npm run verify:release— not a release or handoff-confidence claim.npm run eval:retrieval:quality— no retrieval, ranking, selection, chunking or scoring behaviour changed.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-only— no answer generation, synthesis prompt or answer post-processing changed.npm run check:production-readiness— no clinical workflow, privacy, environment, Supabase, source governance or deployment behaviour changed. The diff adds a browser-local convenience and re-lays out an existing comparison; it touches no server route, no data access path and no environment.npm run check:deployment-readiness— no deployment startup, hosting or rollout behaviour changed.Risk and rollout
tests/ui-therapy-nav-scroll.spec.tsnow asserts it in Chromium at 390×844. The second risk is that adding a therapy no longer navigates — a deliberate behaviour change, confirmed with the repository owner, whose two existing assertions were rewritten knowingly rather than left to pass silently.git revertof this commit. The memory layer is purely additive and the?ids=URL contract is untouched, so a revert cannot strand or invalidate an already-shared comparison link.src/lib/rag/, the retrieval RPCs, the ranking/selection/answer-ranking surfaces, the eval harness or the golden fixtures is touched. The changed library files aretherapy-compass-navigation.ts(URL parsing for the compare set), the newtherapy-compare-memory.ts(browser storage), andmode-home-composer.ts(a phone dock slot id); none of them participates in retrieval, ranking or answer generation.Clinical Governance Preflight
This section is required because
src/lib/therapy-compare-memory.tsandsrc/lib/therapy-compass-navigation.tsmatch the clinical-risk path pattern^src/lib/.*therapinscripts/pr-policy.mjs. Each item below is a statement of fact about this diff, checked against the changed-file list.Clinical KB Database(sjrfecxgysukkwxsowpy)Evidence for each. No clinical claim, source field, review status or citation is created, edited or reworded anywhere in this diff; the comparison renders the same
ROWSgetters over the same therapy records it did before, only in a second layout belowmd. No document upload, download, storage or access path is touched, and no patient-identifiable data is read, written or persisted — the one thing stored on the device is a list of therapy slugs (["cognitive-behavioural-therapy-cbt", …]), which is the same public catalogue identifier that already travels in the shareable URL. No Supabase client, environment value or credential appears in the diff, so the project target and the server-only confinement of the service-role key are unchanged by construction. Demo and synthetic content are untouched: no fixture or demo corpus file is modified, and the design-scratch mockups that seeded this work stayed in PR #2339 rather than here. Review status stays conservative and stays visible — the comparison'sReviewed/Needs reviewmarker is preserved in the desktop table, the new phone layout keeps theEvidence levelrow (which falls back to "Source review required" for an unreviewed record), and the compare tray's expanded sheet lists each therapy by full name. The remembered set fails conservative in three separate ways: a slug absent from the live catalogue is dropped at restore, a malformed or unreadable store returns an empty set rather than throwing, and a shared link always overrides device memory so a colleague can never be shown a different comparison from the one they were sent. Because no clinical decision-support behaviour changed — no ranking, no recommendation, no answer, no clinical text — the SaMD/TGA classification question was checked and returns no impact.Notes
AGENTS.mdforbids combining the two. PR Add four phone mockups for the therapy comparison picker #2339 has since been closed unmerged — it existed to let a direction be chosen, that happened, and its branch kept conflicting withmainon a generated snapshot file. The prototypes remain readable from that closed PR and its branchclaude/therapy-comparison-mobile-design-z0dagr.src/components/compare/is shared by therapy, dictionary, DSM, formulation and specifiers, so the empty-slot problem this addresses is present on all five. This change is scoped to Therapy Compass only, by explicit decision; the other four are untouched and unaffected.Generated by Claude Code
Note
Low Risk
Risk is mostly phone layout chrome (dock reserve, scroll-hide, addon exclusivity) and a deliberate UX change that add no longer navigates; clinical content and retrieval paths are untouched, with broad test coverage on the new contracts.
Overview
Therapy Compass on phones now builds the compare set where users find therapies instead of sending them to an empty comparison page or a wide horizontal table.
Compare behaviour: Adding a therapy updates the URL
idsand stays on the current page (addCompare/removeComparereplacetoggleCompare). Result cards and record pages use Add to compare with full-tray handling; the record nav Compare control only navigates to the comparison screen. Device memory (therapy-compare-memory.ts) restores the last set when the URL has noids, with shared links and catalogue validation winning over stale slugs.Phone chrome: A one-row compare tray (
therapy-compare-tray.tsx) portals into the search dock addon slot (therapy-compare), with matching reserve tokens, backdrop height, and scroll-hide overshoot inglobals.cssandmobile-composer-reserve.ts. The shell claims the addon only on therapy dock routes and whenreadTherapyCompareSlugCountis non-zero. Expanded tray management uses a bottom Sheet; record routes get in-pageTherapyCompareActionwith live status announcements because there is no dock there.Comparison UI: From
mdup the table is unchanged; belowmdthe same rows render as a per-field stacked layout (TherapyCompareStack). The old[data-therapy-scroll-sm]horizontal-scroll path is removed.Docs, adoption manifest, and contract/browser tests are updated for the new addon kind and behaviour.
Reviewed by Cursor Bugbot for commit 9b34a01. Configure here.