From 83d6f9bd415dd8d7fcdabb836c93b8924626caf1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 21:02:34 +0000 Subject: [PATCH 1/7] feat(therapy-compass): converge filter sheet onto the shared contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retires the bespoke TherapyFilterTrigger/TherapyFilterSheet in favour of ResultFilterTrigger/ResultFilterSheet/ResultFilterFacetChips, the shared components every other converged mode already uses. Two facet groups (Topics, Availability) now feed both the desktop rail and the phone sheet from one source, closing the last item in /issues #170. Two decisions this PR resolves rather than silently changes, both stated here per docs/filter-contract.md's own rollout convention: - onClearAll vs the query: the old sheet's Clear all wiped the search query too (phone-only, deliberate). That contradicted filter-contract.md section 6 ("onClearAll never touches the query"). The phone-only justification no longer holds — the shared composer already renders its own always-visible clear-query control at every viewport (master-search-header.tsx) — so onClearAll now wires to clearSearchFilters (filters only), matching the invariant. No contract change needed. - AND-within-group tags: therapy's Topics predicate (select.ts's wantTags.every) narrows rather than widens, unlike the contract's OR-within-group default for facets. This is mode-owned filtering logic the shared component's kind type doesn't dictate, so it stays unchanged; only the UI layer converges. Hint counts are computed by re-running the real predicate with each candidate added, which stays honest either way. Also updates the Playwright and vitest coverage that pinned the retired component's exact shape, and regenerates the design-system adoption manifest. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TtjsdU7PRAKYHV98XJRMF6 --- docs/design-system/COMPONENTS.md | 2 +- docs/design-system/adoption-manifest.json | 4 +- docs/outstanding-issues.md | 2 +- docs/search-results-bar-decisions.md | 7 +- .../therapy-compass/filter-sheet.tsx | 175 ------------------ .../therapy-compass/screens/search-screen.tsx | 165 +++++++++-------- tests/therapy-compass-mode-wiring.test.ts | 43 +++-- tests/therapy-filter-sheet.dom.test.tsx | 74 -------- tests/ui-accessibility.spec.ts | 32 ++-- 9 files changed, 131 insertions(+), 373 deletions(-) delete mode 100644 src/components/therapy-compass/filter-sheet.tsx delete mode 100644 tests/therapy-filter-sheet.dom.test.tsx diff --git a/docs/design-system/COMPONENTS.md b/docs/design-system/COMPONENTS.md index 2561da50d4..0af33ce0b6 100644 --- a/docs/design-system/COMPONENTS.md +++ b/docs/design-system/COMPONENTS.md @@ -1001,7 +1001,7 @@ This generated snapshot is a local source-derived inventory. It does not assert | `SearchField` | controls | yes | yes | no | yes | no | 0 | | `SegmentedControl` | controls | yes | yes | inherited-global-root | yes | no | 7 | | `Select` | controls | yes | yes | inherited-global-root | yes | no | 2 | -| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 25 | +| `Sheet` | layout | yes | yes | inherited-global-root | yes | no | 24 | | `Skeleton` | feedback | yes | yes | inherited-global-root | yes | no | 6 | | `SourceDesignationBadge` | source | yes | yes | inherited-global-root | yes | no | 1 | | `SourceProvenance` | source | yes | yes | inherited-global-root | yes | no | 1 | diff --git a/docs/design-system/adoption-manifest.json b/docs/design-system/adoption-manifest.json index 618868708e..3e549abfca 100644 --- a/docs/design-system/adoption-manifest.json +++ b/docs/design-system/adoption-manifest.json @@ -1496,7 +1496,6 @@ "src/components/in-page-nav/in-page-nav-header.tsx", "src/components/mode-nav/mode-nav.tsx", "src/components/services/service-group-nav.tsx", - "src/components/therapy-compass/filter-sheet.tsx", "src/components/ui/confirm-dialog.tsx" ], "productImportFiles": [ @@ -1523,8 +1522,7 @@ "src/components/forms/forms-search-results-page.tsx", "src/components/in-page-nav/in-page-nav-header.tsx", "src/components/mode-nav/mode-nav.tsx", - "src/components/services/service-group-nav.tsx", - "src/components/therapy-compass/filter-sheet.tsx" + "src/components/services/service-group-nav.tsx" ], "designSync": { "listedInSourceMap": true, diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 9d8a9ce3c5..1348fc7c81 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -193,7 +193,7 @@ removed after current-main verification; it is not missing recommended work. | #165 | P2 | task | Adopt a consolidated answer-home notice block — the studies exist, nothing adopts them | **Outcome:** the answer hero states its safety obligation, its scope, and its verification requirement as one block in one voice. **Detail:** `/mockups/warning-consolidation` (PR #1437) diagnoses today's three stacked notices — the APP-5 privacy warning at 11px muted, a bare `/privacy` link, and an accent-blue `ShieldCheck` capability claim at 14px semibold — and shows the hierarchy is inverted: the least important line is the loudest, and two shields with opposite meanings sit ~40px apart. Three consolidations are drawn at 1440px and 390px. Recommended: **02 Safety card** on the hero (obligation on a warning-tinted top row, everything descriptive in one grey voice below) and **01 Assurance bar** on the docked composer — the same content model at two densities, so one component with a `density` prop covers both. **This is a governance change, not just a design one:** `PrivacyInputNotice` is the single site-wide APP-5 line and renders on the answer, documents and calculators composers, so all three move together; `tests/privacy-ui.test.ts`, `tests/ui-accessibility.spec.ts` and the phone-chrome reserve coverage all assert against the current markup and must change in the same commit; and the PR will need a full `## Clinical Governance Preflight` (the mockup PR correctly did not). **Third study (before/after):** `/mockups/answer-home-proposal` draws the concrete D-direction proposal as a full hero before/after rather than an isolated notice. **Second study (words only):** `/mockups/warning-line` answers a narrower brief — no icon, border, tint or background, one line where width allows. Six variants A-F; line counts measured from the rendered DOM, not asserted. Only B (middot clauses), D (obligation + verify) and F (compressed obligation) hold one line at desktop width, and **none fit one line on a 390px phone while the pinned APP-5 sentence stays verbatim** — 46 characters of obligation plus the 27-character link exceeds the ~60 available at 11px. Recommended there: **D**, the only compliant variant that is both one line and keeps weight-only hierarchy, reached by dropping the scope claim (a capability statement already visible on the answer itself). F fits best but rewrites the pinned obligation to \|No patient-identifiable information.\| and so needs the same privacy sign-off as `#166` plus a matching `tests/privacy-ui.test.ts` update. **Status:** PR #1437 was closed unmerged on 2026-07-30 as a deliberate pause during an owner-authorized ordered merge sweep, to be reopened at its queued place; branch `claude/warning-consolidation-mockups-09jyj7` is preserved and merged onto current `main`; these follow-up rows have been renumbered on each sync because `main` kept claiming the next ids while the PR was paused; the superseded numbers are deliberately not listed, since they now belong to unrelated rows. **Next:** decide block (02 + 01) versus line (D) direction, get wording sign-off for `#166`, then implement behind one component and run `verify:phone-chrome` before `verify:ui`. | session 2026-07-30; PR #1437; `/mockups/warning-consolidation`; `/mockups/warning-line` | 2026-07-30 | | #168 | P2 | rec | Sequential issue ids force every concurrent append to conflict | **Outcome:** two sessions can append to this ledger at the same time without conflicting. **Detail:** ids are allocated read-modify-write against the `issues:next-id` marker inside the file being edited, so two branches both read N and both write N. Because duplicate ids are unacceptable, a union merge driver is unsafe — .gitattributes says so explicitly — which is why this file deliberately has no driver and every overlapping append conflicts by hand. Manual resolution is where rows get dropped: PR #1490 was closed during one and took the only record of four snapshots with it (#152), and ids were renumbered under in-flight work three times in one session (#154, #155). The new writer (`scripts/outstanding-issues.mjs`) removes the mechanical errors but explicitly not this one. **Next:** replace the counter with a collision-free id (ULID, timestamp+suffix, or a content hash), keeping a short display number derived at render time if `#151` reads better than 01JQ…; then a union driver becomes safe to reinstate and concurrent appends stop conflicting at all. A larger variant is one row per file under `docs/issues` with the table generated, which the repo already does for `site-map.md`. **Stop:** do not reinstate `merge=union` while ids are sequential — that combination was tried in PR #1416 and removed for duplicating rows and the marker. Renumbered from this PR's original `#159` because `main` already used `#159` for the duplicated test-file-list finding. | session 2026-07-31; .gitattributes; #154/#155; PR #1524 sync | 2026-07-31 | | #169 | P2 | issue | Local branches carry work that exists on no remote | **Outcome:** committed work is not lost when a machine or worktree is reclaimed. **Detail 2026-07-31:** six `claude/*` branches in this checkout have commits and no `origin/` counterpart. Verified real for `claude/clinical-kb-design-system-333a69` — 57 files / +4069, tip `feat(design-system): v2 token layer, 26 components, browser-crash fix` dated 2026-07-31 17:40, whose added `.design-sync/previews/*.tsx` files are absent from main. Others unverified: `design-sync-db0a54`, `fable-implementation-fc937c`, `frosty-mayer-2c6167`, `issues-133-evidence`. **How to check, because the obvious measure lies:** `git rev-list --count origin/main..` and a three-dot diff both report landed work as unmerged, since this repo squash-merges and the original commits never become ancestors — my own merged branch reported 1 commit and +476 by that measure. Test instead whether files the branch adds exist on main (`git ls-tree origin/main `). **Next:** per branch, push it for review or confirm it is superseded and delete it; do not bulk-delete on the commit count. Sibling of #152, which covers uncommitted work in worktrees rather than unpushed commits on branches. NOT VERIFIABLE FROM A CLOUD SESSION (checked 2026-08-12): this row describes machine-local git or process state. Claude Code on the web runs in a container cloned fresh at session start, so it reports 1 worktree, 0 upstream-less branches and no running dev servers no matter what is true on the machine that raised this. Do NOT close this row from a cloud session — that reading is an artifact of the container, not evidence. Verify from the originating machine. | session 2026-07-31; local branch audit | 2026-07-31 | -| #170 | P2 | task | Phone filter sheets exist for documents and therapy; shared-band adoption remains | **Outcome:** phone filtering opens the repo's own bottom sheet instead of competing for width in the utilities rail, and both breakpoints agree on what each dimension means. **Current state (2026-08-12):** the shared contract landed (PR #1847, docs/filter-contract.md) discriminating ResultFilterGroup into lens (one-of-N) and facet (many-of-N); SegmentedControl gained an option hint slot (PR #1848); and the four lens modes now adopt it (PR #1857) — differentials, medication, applications and specifiers each replaced a bespoke aria-pressed desktop rail with SegmentedControl and build ONE option array shared by rail and sheet. That closes the defect where the same dimension claimed one-of-N in the sheet (role=radiogroup) and many-of-N on the rail (aria-pressed). Specifiers' footerNote also fixed: it reported results.length + catalogueMatches.length while its groups narrow only results. Formulation is the first real FACET adoption: domain list derived from the mechanisms that carry it (9 of 12 declared — Biological, Social and Cultural are carried by 0 of 12 and are gone), domain converted to resultFilterFacetGroup with union counts verified monotonic and non-additive, zero-yield options rendered as focusable dead ends, the desktop select retired in favour of the shared ResultFilterFacetChips (exported so facets get the same both-breakpoints primitive lens modes get from SegmentedControl), and the query-replacing pattern group evicted from the sheet to AnswerSuggestionChips below the band (all five presets, not the four the old .slice(0,4) exposed). **Remaining:** services (facet index + selection state + URL + the scope segment; substance_flags is an exact partition so it is a lens, not a facet, and ServiceRecord.tags is flattened with "general" colliding across three dimensions, so a typed facet carrier is needed first), factsheets (evict category presets to the composer), therapy-compass (converge onto the shared trigger/sheet; its onClearAll still wipes the query, which the contract forbids), documents last (port find-a-filter + collapse up as the >20 tier). **Stop:** the scope segment is deliberately NOT retrofitted to the adopted modes yet — filter-contract.md section 4 gates it on a catalogue meaningfully larger than the result set, and it lands with services where that is true. Do not ship a mode conversion without browser proof at 390px and 320px; these change rendered output, unlike #1847. | Round-7 design study; PR #1523 notes; `src/components/ui/sheet.tsx` | 2026-07-31 | +| #170 | P2 | task | Phone filter sheets exist for documents and therapy; shared-band adoption remains | **Outcome:** phone filtering opens the repo's own bottom sheet instead of competing for width in the utilities rail, and both breakpoints agree on what each dimension means. **Current state (2026-08-12):** the shared contract landed (PR #1847, docs/filter-contract.md) discriminating ResultFilterGroup into lens (one-of-N) and facet (many-of-N); SegmentedControl gained an option hint slot (PR #1848); the four lens modes adopted it (PR #1857) — differentials, medication, applications and specifiers; formulation was the first real FACET adoption (domain, union counts, dead ends, ResultFilterFacetChips); and therapy-compass is now converged too (PR E): TherapyFilterTrigger/TherapyFilterSheet retired in favour of the shared components, two facet groups (Topics — 6 tags, deliberately AND-within-group per select.ts's existing wantTags.every, preserved rather than silently widened to the contract's OR-within default — and Availability), hints computed by re-running the real predicate with each candidate added, and the sheet's onClearAll rewired to clearSearchFilters (filters only) — resolving the documented onClearAll-vs-query conflict in favour of the section 6 invariant, since the shared composer's own always-visible clear-query control already covers what the old double-duty Clear all used to. **Remaining:** services (facet index + selection state + URL + the scope segment; substance_flags is an exact partition so it is a lens, not a facet, and ServiceRecord.tags is flattened with "general" colliding across three dimensions, so a typed facet carrier is needed first), factsheets (evict category presets to the composer), documents last (port find-a-filter + collapse up as the >20 tier). **Stop:** the scope segment is deliberately NOT retrofitted to the adopted modes yet — filter-contract.md section 4 gates it on a catalogue meaningfully larger than the result set, and it lands with services where that is true. Do not ship a mode conversion without browser proof at 390px and 320px; these change rendered output, unlike #1847. | Round-7 design study; PR #1523 notes; `src/components/ui/sheet.tsx` | 2026-07-31 | | #175 | P2 | task | Therapy modality is now null on all 205 records and needs curation or removal | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: public/therapy-compass-data/therapies-index.json holds 205 records and 0 carry a modality value, exactly as described. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. **Outcome:** the Therapy detail and recommend screens either show a curated modality or stop carrying the field at all. **Detail:** the source catalogue derived `modality` from each record's own tag list — all 205 records had one, every value was also present in that record's `tags`, and the whole catalogue collapsed to CBT/ACT/DBT. It mislabelled the treatments it could not describe: ECT and rTMS as "ACT", Psychoanalysis and Psychodynamic Psychotherapy as "CBT", MBT and TFP as "DBT". Pre-existing on main, surfaced by the PR #1489 review. The generator emits it only when the source curates a value that is not already a tag, which today means null for 205/205 on the index projections *and* the full catalogue the detail/recommend screens load (`catalogue: "full"`), so the two chips (`detail-screen.tsx:49`, `recommend-screen.tsx:115`) never render and `select.ts:117` contributes no same-modality point. Removal was provably search-neutral: `src/lib/therapies.ts` scores with boolean `haystack.includes(token)`, not term frequency, and every modality value was already contributed by `tags.join(" ")` in the same haystack. **Next:** one of two — curate real modality values in `public/therapy-compass-data/therapies.json` (clinical work, needs the psychiatrist), or drop the field from `types.ts`, `src/lib/therapies.ts`, the two chips and `select.ts`. **Stop:** do not reinstate the tag-derived value to make the chips reappear; a guess rendered as curated fact is the defect. `tests/therapy-compass-pathways.test.ts` pins the echo invariant on both the index and the full catalogue asset. Renumbered from this PR's original `#169` because `main` claimed `#169`–`#174` while the branch was open. | PR #1489 review remediation; PR #1532; session 2026-07-31 | 2026-07-31 | | #177 | P3 | rec | Therapy catalogue aliases duplicate 2.53 MB of bytes instead of pointing at the hashed file | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: public/therapy-compass-data/ holds THREE copies of the catalogue (therapies.json plus two hashed assets), which is the grace-generation case this row predicted. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. **Outcome:** the unversioned catalogue aliases stop costing a second copy of every payload in the repo and the image. **Detail:** `public/therapy-compass-data/` ships each catalogue twice — `therapies.json` (2,529,429 bytes) alongside `therapies..json` with identical content, and the same for the index and home projections. Git stores one blob for each identical pair, so history is unaffected, but the working tree and every Docker image carry both. PR #1489's remediation adds a one-deploy grace generation, so a stem whose content changed now holds three copies until the next regeneration. **Next:** serve the alias as a Next rewrite to the current hashed filename instead of writing a duplicate file — the alias would keep its `max-age=0, must-revalidate` header (the hashed target keeps `immutable`, so the rewrite must not inherit it) and the generator would stop copying bytes. Check that a rewrite still satisfies the alias-equality assertions in `check:therapy-data-index` and `tests/therapy-compass-mode-wiring.test.ts`, which currently compare file bytes. **Stop:** do not drop the aliases outright — they are the fallback `useTherapyData` uses when a pre-deploy bundle names a hash that no longer exists. Renumbered from this PR's original `#171`. | PR #1489 review remediation; PR #1532; session 2026-07-31 | 2026-07-31 | | #178 | P2 | rec | pr-policy does not flag operational risk bundled with clinical or UI risk | IN FLIGHT 2026-08-12 in PR #1837 (harden verification & PR policy guards). Checked against the open-PR list during the full ledger sweep. Do NOT start this row while that PR is open — duplicating a queued conversion is the exact failure #292 records, and it has happened twice. Re-verify this row against main after that PR merges, and close it there rather than here. **Outcome:** a PR that mixes operational-risk paths with clinical or UI risk is called out before it merges, because squash-merging that mix destroys per-item revert. **Detail:** `classifyPullRequestFiles` already computes `operationalRisk`, `clinicalRisk`, `ragRanking` and `ui` independently, but nothing reacts to the combination. AGENTS.md's "PR bundling" section forbids bundling anything once `operationalRisk` is true; the classifier is where that could be enforced. PR #1489 is the worked example: 33 files spanning `.github/workflows/ci.yml`, both Dockerfiles, a rewrite of the bundle-budget gate, a phone-chrome scroll change and a therapy data restructure, merged as one squash (945148251). Reverting any single item now means hand-reverting hunks out of the squash commit, because the branch commits are unreachable. The remediation PR for that review repeats the pattern on a smaller scale (clinical data plus a one-line ci.yml timeout), which is why this is a recommendation rather than a hard gate — the right severity is probably a warning that names the mixed classes, not a merge block. **Next:** emit an advisory line from `evaluatePullRequestPolicy` when `operationalRisk` coincides with `clinicalRisk` or `ui`, listing which paths drove each; decide separately whether it ever blocks. Cover it in the `--self-test` block. **Stop:** do not make it a hard failure in the same change that introduces it — land the signal first and see how often it fires. Renumbered from this PR's original `#172`. | PR #1489 review remediation; PR #1532; session 2026-07-31 | 2026-07-31 | diff --git a/docs/search-results-bar-decisions.md b/docs/search-results-bar-decisions.md index 3ba51e9dcf..9aef7ddf12 100644 --- a/docs/search-results-bar-decisions.md +++ b/docs/search-results-bar-decisions.md @@ -143,9 +143,10 @@ Two traps met while drawing that line: and phrased as reach with the corpus count beside it, and the zero-result state. Do not delete either without giving nav a query-preserving route first. - **Sort does not move into the phone filter sheet.** Only documents and therapy-compass - currently have phone filter sheets (`Filter documents` / `TherapyFilterSheet`). Of the - four production `onSortChange` consumers, documents already pairs Sort with a sheet; - therapy has a sheet but no Sort. Moving Sort into the sheet from the shared band would + currently have phone filter sheets (`Filter documents` / the shared `ResultFilterSheet`, + `testId="therapy-filter"`). Of the four production `onSortChange` consumers, documents + already pairs Sort with a sheet; therapy has a sheet but no Sort. Moving Sort into the sheet + from the shared band would remove Sort from phones in the three sheetless Sort consumers — differentials, forms, and services — the exact defect an earlier round fixed. If this is ever wanted it is per-page work on those consumers, not a shared-band change. diff --git a/src/components/therapy-compass/filter-sheet.tsx b/src/components/therapy-compass/filter-sheet.tsx deleted file mode 100644 index b130960e11..0000000000 --- a/src/components/therapy-compass/filter-sheet.tsx +++ /dev/null @@ -1,175 +0,0 @@ -"use client"; - -import { Sheet } from "@/components/ui/sheet"; - -import { outlineControl, softControl } from "./controls"; -import { CheckIcon, SlidersIcon, XIcon } from "./icons"; - -/** - * The phone filtering surface for therapy search. - * - * It replaces two native `