feat(filters): therapy-compass adopts the filter contract (PR E) - #1889
Conversation
Converges therapy-compass's bespoke phone-only TherapyFilterSheet / TherapyFilterTrigger onto the shared ResultFilterSheet / ResultFilterTrigger / ResultFilterFacetChips, fixing both invariants the contract doc already named against this mode: - The sheet's Clear all no longer wipes the query. It now uses the shared onClearAll contract (filters only); the composer's own "Clear search" covers the query on every breakpoint, matching every other adopted mode. - The trigger is now the shared ResultFilterTrigger instead of a bespoke reimplementation with a different icon, hardcoded test id, and label-hiding breakpoint. Three facet groups, desktop and phone sharing one array: - Topics (the six curated quick-filter tags) is many-of-N, OR within the group. This also fixed searchTherapies' tag matching, which was AND-within -group — the same defect class already fixed for document tags: a second topic narrowed instead of widening, an accumulating-selection dead affordance. - Review status and Handout are two independent one-option facet groups (Reviewed only / Brief available), not options inside one shared group, because they AND against each other and against Topics rather than OR. matchesTopics/matchesAvailability in data/select.ts are the single predicate both searchTherapies and the sheet's option counts run through, so a count can't drift from what the real filter does. Verification: full unit suite 6167 passed/4 skipped/0 failed; typecheck and lint clean; verify:pr-local 15/15 green (build run with the dev server stopped); bundle-budget production 1304.4 KiB and mockups 294.5 KiB, both within tolerance on a freshness-verified build; browser proof at 320/390/639/768/1440/1920px, 0px overflow at every width, 48px phone tap target, widen-on-multi-select and query-preserving Clear all verified live.
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:2 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:18e2b7ce97
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 12, 2026
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/filter-therapy-compass at starting commit 0bbe967; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/filter-therapy-compass, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
BigSimmo
commented
Aug 12, 2026
Addressed the review finding on the current head |
BigSimmo
commented
Aug 12, 2026
@claude Work only the current unresolved review finding on this PR at head |
Hi @BigSimmo — thanks for the @claude mention. Open this in Claude Code on the web to continue here: https://claude.ai/code Context: Automated backlink from the Claude backlink workflow. Claude Code can pick up the mention and push a scoped fix to the branch. |
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
BigSimmo
commented
Aug 12, 2026
@claude The current review fix is already on head |
Hi @BigSimmo — thanks for the @claude mention. Open this in Claude Code on the web to continue here: https://claude.ai/code Context: Automated backlink from the Claude backlink workflow. Claude Code can pick up the mention and push a scoped fix to the branch. |
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
CI triageCI failed on this PR. Automated classification of the 4 failed job(s):
Compared with main CI run #10317 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
BigSimmo
commented
Aug 12, 2026
Updated the filter DOM fixture so the independent Reviewed-only toggle is exercised with a reviewed therapy, while preserving the zero-count disabled guard. The PR remains blocked by its base-branch merge conflict; no rebase or force-push was performed. |
Resolve PR #1889 against current main while preserving the reviewed therapy filter-contract changes. Keep main's shared ledger and generated design inventories to avoid stale branch-era records and counts.
BigSimmo
commented
Aug 13, 2026
@claude Please main-sync this PR from exact head Preserve the verified Therapy filter-contract behavior, including disabled unselected zero-count topic and availability facets and the resolved focused DOM regression. For the shared Run the focused Therapy search filter/facet/mode-wiring tests plus the relevant accessibility selector, typecheck, and format. Report the new head and exact outcomes. |
Hi @BigSimmo — thanks for the @claude mention. Open this in Claude Code on the web to continue here: https://claude.ai/code Context: Automated backlink from the Claude backlink workflow. Claude Code can pick up the mention and push a scoped fix to the branch. |
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
BigSimmo
commented
Aug 13, 2026
Fixed the current-head CI defects on this PR. The design-system adoption manifest/maturity snapshot now reflects that the legacy |
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
left a comment
There was a problem hiding this comment.
Final review-and-fix summary
Final GitHub state
- I did not merge or close this pull request. GitHub records that PR #1889 was merged externally at 2026-08-13 03:36:42 UTC while this review was in progress.
- Final PR head:
5c165f227ddcea3135d4ed52f0d69813389fc466 - Base at merge:
2ae5a0aa5d339a7dc9089db134c2d9d0220444ae - Immediately before merge, the branch was 15 commits ahead, 0 behind, with the latest base as its merge base and a clean merge state.
- Merge commit recorded by the raw PR endpoint and current
main:ad2839aeebca6798f62d91e98ae69c8cd38e5aa2
Findings and disposition
- No P0/P1 defect was identified.
- The existing P2 review item about zero-count facets had already been addressed on the final head. Its only inline thread is now resolved and outdated.
- A distinct adversarial pass identified one additional reproducible P2 defect in
src/components/therapy-compass/bindings.tsx: when a non-empty query is cleared,queryMatchesimmediately switches to the full catalogue whilesearchResultsstill uses the deferred prior query for one render. Facet counts can therefore transiently disagree with the visible result set, contrary to the filter-count contract. - A minimal two-branch reorder and focused regression test were prepared and verified locally against the exact PR head before the external merge. They were not published after merge, because pushing to a closed PR branch or creating a workaround PR would have exceeded this task's authority. The defect remains present in the merged
maintree and requires a separately authorised follow-up change. - No unrelated or pre-existing debt was changed.
Adversarial review and local evidence
- CodeRabbit could not run because the repository bot reported a review spending/usage limit, and the local CodeRabbit CLI was unavailable. A separate manual adversarial pass was completed instead, covering deferred-state consistency, OR-within-group/AND-across-group semantics, disabled-option escape paths, accessibility behaviour, hidden filter state, and missing-test risk.
- Proposed-fix red/green/falsification proof: the regression failed on the original implementation, passed with the minimal fix, failed when the fix was reverted, and passed again when restored.
- Focused suite: 4 files, 26 tests passed.
- Repository PR-local planner: 6/6 checks passed.
- TypeScript, changed-file ESLint, Prettier, and
git diff --check: passed. - Independent projection cross-check: 9,216 topic projections, plus availability projections, agreed with
searchTherapiesacross the real catalogue.
Exact-head CI
- Required aggregate
PR required: success on exact head5c165f227ddcea3135d4ed52f0d69813389fc466. CIworkflow run31663671475: completed successfully.Production UIwas not selected for this head. No such exact-head check run existed.
Advisory and infrastructure state
- Advisory
PR mergeability: success on the exact head. - No exact-head
Ensure auto-mergecheck run existed at the final snapshot. - Supabase preview was intentionally ignored because the PR changed no
supabasefiles. - CodeRabbit review remained unavailable because of its external usage cap.
Threads and ledger
- Review threads: 1 total, 0 actionable, 1 resolved/outdated.
- No immutable heavy-review record was found for this exact final head. Repository policy prohibits a ledger-only tip, and the PR had already merged, so no post-merge branch or
mainmutation was made merely to add a record.
I did not merge the PR. Because GitHub had already merged it externally, final merge was no longer left to the user.
BigSimmo
left a comment
There was a problem hiding this comment.
Final review-and-fix status for PR #1889
- PR state: already merged before this finalisation pass. Final reviewed head:
5c165f227ddcea3135d4ed52f0d69813389fc466. Base at PR snapshot:2ae5a0aa5d339a7dc9089db134c2d9d0220444ae. Merge commit:9a0a00be33dedb01e9d59e81f42225cc6f9d3939. - Merge/sync state: no further sync or branch mutation performed because the PR is closed/merged; post-merge edits would violate the requested stop condition.
- Review finding: the P2 zero-count facet issue was addressed in the final head. Unselected zero-result Topics, Review status, and Handout options are now disabled while selected options remain removable.
- Review threads: 1 inline Codex thread; resolved and outdated. No actionable unresolved review threads remain.
- Adversarial pass: re-checked the final head specifically for the reported dead-end facet behavior and verified the focused guard is present for all three relevant facet groups. No additional high-confidence PR-introduced P0/P1 defects were identified in this finalisation pass.
- Local verification: no new local test run was started after discovering the PR had already merged. The final head's exact-head CI is the decisive evidence for this stop-state review.
- Exact-head CI: CI, SAST, and Secret Scan workflow runs completed successfully. Production UI critical and Production UI (1), (2), and (3) all completed successfully. Build, Static PR checks, Unit coverage, Safety/config, PR mergeability, PR policy, Semgrep/Semgrep ingestion, Gitleaks, and GitGuardian also completed successfully.
- Advisory checks: Lighthouse budget (advisory) succeeded. Visual baselines (advisory) and Advisory UI were skipped. Other scope-inapplicable jobs such as migration replay and container images were skipped.
- Blockers/residual risk: none identified that can act on this already-merged PR. No permission workaround, rebase, force-push, deployment, release, or production test was performed.
The PR was already merged before this pass. I did not merge it or alter its merged state.
Summary
TherapyFilterSheet/TherapyFilterTrigger(filter-sheet.tsx, now deleted) onto the sharedResultFilterSheet/ResultFilterTrigger/ResultFilterFacetChips(docs/filter-contract.md), fixing both invariants the contract doc already named against this mode: the sheet'sonClearAllno longer wipes the search box (the composer's own "Clear search" already covers the query on every breakpoint, matching every other adopted mode), and the trigger is the sharedResultFilterTriggerinstead of a bespoke reimplementation with a different icon, a hardcoded test id, and a different label-hiding breakpoint.searchTherapies' tag matching, which had been AND-within-group (a therapy needed every selected tag). That's the same defect class already fixed for document tags: a second topic selection narrowed instead of widening, making multi-select within one group a dead affordance instead of a feature. Review status and Handout (Reviewed only/Brief available) are modelled as two independent one-option facet groups rather than options inside one shared group, becausesearchTherapiesANDs them against each other and against Topics — a shared group would OR them, which is not what the real filter does.matchesTopics/matchesAvailabilityindata/select.tsare the single predicate bothsearchTherapies(the real filter) and the sheet's option counts (option.hint) run through, so a count can never drift from what clicking the option actually does (docs/filter-contract.mdsection 3).ResultFilterFacetChipsgroups render side by side; neutralised the shared component'sborder-tdivider (meant for groups stacked vertically inside the sheet) with a[&>section]:border-t-0wrapper so it doesn't show as a floating hairline above the second/third group in this new side-by-side layout — verified in a real browser before and after.docs/filter-contract.md(section 6's two invariant callouts, plus a new Rollout entry) andtests/therapy-compass-mode-wiring.test.ts's static wiring assertions, which had locked in the old bespoke sheet's deliberate full-resetonClear={b.clearSearch}behavior — now updated to assert the newonClearAllcontract instead.Verification
npm run verify:pr-local— all 15 steps green (build run with the local dev server stopped, per the gate's own refusal check)npm run test— full unit suite, 565 files / 6167 passed / 4 skipped / 0 failednpm run typecheck— cleannpm run lint— cleannpm run check:bundle-budgeton arm -rf .nextfresh build — production 1304.4 KiB vs 1279.1 KiB baseline, mockups 294.5 KiB vs 267.5 KiB baseline, both within tolerancenpm run verify:uinot run — this environment's Playwright is pinned to a browser revision (r1234) not preinstalled here (r1194); direct Chromium browser proof above substitutes for the routes this PR touches. Full Chromium gate is a normal CI/hosted-environment run.Risk and rollout
filter-sheet.tsxand its test file are recoverable from history if a revert is ever needed.Notes
reviewStatus: "needs_review"— none has cleared review yet — so the "Reviewed only" facet chip currently shows(0)regardless of other selections. It is intentionally not hidden or disabled: it's a legitimate, functioning filter whose count reflects real (if currently empty) data state, exactly matching how the old bespoke toggle behaved before this PR (always shown, unconditionally). It will show real counts once therapies clear review.docs/filter-contract.md). Prior PRs in the same rollout (lens modes, formulation, services, factsheets) are separate branches/PRs at various stages — this PR is independent and does not depend on any of them merging first.Generated by Claude Code