Skip to content

Therapy Compass: stop faking multi-select with a listbox on phones - #1548

Merged
BigSimmo merged 9 commits into
mainfrom
claude/top-search-design-mockups-w53znc
Aug 1, 2026
Merged

Therapy Compass: stop faking multi-select with a listbox on phones#1548
BigSimmo merged 9 commits into
mainfrom
claude/top-search-design-mockups-w53znc

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Therapy Compass's two phone filter controls were native <select>s pretending to be multi-select.value was pinned to "" so nothing was ever selected, chosen options carried a literal "✓ " prefix in their text, a disabled placeholder row did the reporting ("3 topics selected"), and the availability select carried Clear filters — an action — among its options. Assistive technology was told "combobox, nothing selected" while the visible text said three things were on, and choosing an already-chosen option silently deselected it. A listbox cannot express any of that. Both now open one sheet of aria-pressed toggles — the same controls the wide viewport has always used — so the two breakpoints describe the same state the same way. Built in Therapy Compass's own idiom (tc-btn, softControl, tc-is-selected, local SlidersIcon/CheckIcon).
  • Clear all now appears for a query-only session. The replaced select enabled its Clear filters option via … && !q; the first version of this sheet dropped that term, so a phone user who had only typed a search had no route to clearSearch. Fixed — but with the query counted separately from the badge (below).
  • The trigger badge counts filters only. The two counts are deliberately different: clearableCount (sheet) = topics + availability + query, so Clear all appears whenever clearSearch has something to reset; activeFilterCount (badge) = topics + availability, because the badge is labelled "N filters active" and a search term is not a filter. Counting the query there made the control announce "1 filter active" on a plain search with nothing filtered — the same class of defect this sheet was built to remove.
  • Two follow-ups from the CodeRabbit review on Documents: present the filter panel as a sheet #1542, which merged before they could land there: the documents sheet footer's primary action moves from sm:min-h-10 to sm:min-h-12 (ribbon controls are deliberately 40px on desktop, but a dialog's primary action is not a ribbon control — mode-nav.tsx:269), and the dialog test now pins aria-expanded and aria-controls against the rendered dialog's own id.
  • Removes PR_POLICY_BODY.md, a scratch template Claude/clinical kb design system 333a69 #1546 committed to main. ci.yml's "Sync PR policy body" job reads it from the PR head and overwrites the description with it, so every PR — including this one — was having its body replaced with Claude/clinical kb design system 333a69 #1546's content. Since pr-policy.mjs parses the description as merge-gating input, that meant governance checklists and verification claims on unrelated PRs belonged to a different change. The workflow already skips cleanly when the file is absent.

Review findings deliberately not applied

  • "Share one active-filter formula between the sheet and the trigger." This is the opposite of the fix above. The formulas must differ — a shared helper would re-merge exactly what separates the badge from Clear all, reintroducing the "1 filter active" bug.
  • "Raise the trigger's tap-target assertion to 48px." The trigger sits in the ribbon's utility row beside ResultSortControl, which is min-h-tap (44px); raising only this control leaves the row visibly ragged. The repo's min-h-12 rule exists to stop generic a11y advice pulling production down to min-h-11 (a known ui-smoke flake), not to override a row's own rhythm. The sheet's own toggles, which have the room, aremin-h-12.
  • "Unmounting an open Sheet skips focus restore" (from Documents: present the filter panel as a sheet #1542). Real in mechanism, no observable consequence: popSheet runs in the same effect cleanup as the unmount (sheet.tsx:256), so the scroll lock and background inert state are released either way, and the focus-restore target is the trigger, which unmounts in that same render. The suggested restructure was implemented, failed its own regression test on activeElement === body, and was reverted. A test pins the invariant that does matter instead.

Verification

  • npm run verify:cheap — exit 0, Test Files 460 passed (460), Tests 4797 passed | 4 skipped (4801)
  • tests/ui-accessibility.spec.ts15 passed, Chromium. The Therapy Compass case is rewritten around the actual defect rather than re-pointed: a topic toggle reports aria-pressed="false", flips to "true", and turning on an availability filter leaves the topic on — the state the old control could not represent. It also pins the end-to-end badge contract that component tests cannot reach, since SearchScreen decides what the trigger counts.
  • tests/therapy-filter-sheet.dom.test.tsx — 5 passed; tests/document-filter-panel.dom.test.tsx — 10 passed.
  • Mutation-tested: reproducing the query-only Clear-all bug fails the accessibility spec; restoring the fix passes.
  • npm run verify:pr-local — not run; the gates above are the evidence.
  • eval:*, check:production-readiness, check:supabase-project — not run and not applicable. No retrieval, ranking, ingestion, or provider surface is touched.

RAG impact: no retrieval behaviour change — this changes only which client-side controls render and how already-computed therapy search state is toggled. No file under src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture is touched, and no retrieval RPC or comparator ordering is altered.

Risk and rollout

  • Risk: UI-only. The filter semantics are unchanged — the same toggleTag / toggleReviewedOnly / toggleBriefOnly bindings are called; only the controls calling them change. The wide viewport is untouched. The PR_POLICY_BODY.md deletion restores author-written PR descriptions repo-wide and needs no workflow change.
  • Rollback: independently revertible commits — therapy sheet, the query-only Clear fix, the badge separation plus review responses, and the template removal.
  • Provider or production effects: None.

Clinical Governance Preflight

classifyPullRequestFiles returns clinicalRisk: false, operationalRisk: false, ragRanking: false, ui: true, checked against the full origin/main...HEAD diff rather than the tip commit. No ingestion, answer generation, search/ranking, source rendering, document access, privacy, or production-environment path is involved.

Notes

  • Scope correction: this was planned as three pages. Only Therapy Compass warranted it. Formulation's "Pattern" control is navigation (router.push(presetHref(...))), not a filter; specifiers' two dimensions are genuine single-selects over 4 and 5 options, which a listbox models correctly and a sheet would only add taps to. Differentials, prescribing, services and factsheets were ruled out on the same basis. The distinction that matters is not how many controls a page has but what each one does.

Summary by CodeRabbit

  • New Features

    • Added a mobile filter sheet for Therapy Compass with topic and availability toggles, active-filter counts, live result counts, and a Clear all action.
    • Added accessible dialog behavior, keyboard support, and clear completion controls.
  • Bug Fixes

    • Improved filter action sizing on small screens.
    • Fixed filter-panel cleanup during loading and refetching to prevent scroll locking.

claude added 3 commits July 31, 2026 15:44
Topics and availability were two native `<select>`s pretending to be
multi-select. `value` was pinned to `""` so nothing was ever selected, chosen
options carried a literal `"✓ "` prefix in their text, and a disabled
placeholder row did the reporting ("3 topics selected"). Assistive technology
was told "combobox, nothing selected" while the visible text said three things
were on; choosing an already-chosen option silently deselected it; and the
availability select carried "Clear filters" — an action — among its options.
A listbox cannot express any of that.
Both now open one sheet of `aria-pressed` toggles, which is what the wide
viewport has always used, so the two breakpoints finally describe the same
state the same way. Built in Therapy Compass's own idiom — `tc-btn`,
`softControl`, `tc-is-selected`, and the local `SlidersIcon`/`CheckIcon` —
rather than importing the documents panel's styling.
The accessibility spec's therapy case is rewritten around the real defect: a
topic toggle reports `aria-pressed=false`, flips to `true` when pressed, and
turning on an availability filter leaves the topic on. That is precisely the
state the old control could not represent. Focus-ring and 44px tap-target
assertions move to the trigger.
Verified: `npm run verify:cheap` exit 0, 457 files / 4780 tests passed;
`tests/ui-accessibility.spec.ts` 15 passed, chromium.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
Two of the three findings were valid and are fixed:
- The sheet footer's "Show N documents" button carried `sm:min-h-10`, copied
from the ribbon. Ribbon controls are deliberately 40px on desktop, but this
is a dialog's primary action — `mode-nav.tsx:269` shows sheet rows use
`min-h-12`. Corrected to `sm:min-h-12`.
- The dialog test asserted `role` and `aria-haspopup` but never the
`aria-controls` linkage this PR deliberately added, so a wrong or stale panel
id would have passed. It now pins `aria-expanded` and `aria-controls` against
the rendered dialog's own id, and asserts both are cleared after Escape.
The third — that unmounting an open Sheet when `loading` flips skips focus
restore — is real in mechanism but has no observable consequence, so the
suggested restructure is deliberately not made. `popSheet` runs in the same
effect cleanup as the unmount (`sheet.tsx:256`), so the body scroll lock and
background inert state are released either way; the only skipped step is focus
restore, and its target is the trigger, which `showFilterControl` unmounts in
that same render. Keeping the panel mounted to run the close path would fix
nothing visible while making the panel silently reopen once the refetch ends.
A test pins the invariant that does matter: the scroll lock is not leaked.
Verified: `npm run verify:cheap` exit 0, 457 files / 4781 tests passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
…eview ledger
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@coderabbitai

coderabbitaiBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds an accessible Therapy Compass filter sheet and trigger, integrates filter state into SearchScreen, updates document filter sizing, and adds DOM and accessibility regression coverage.

Changes

Filter accessibility updates

Layer / File(s)Summary
Therapy filter sheet implementation
src/components/therapy-compass/filter-sheet.tsx
Adds accessible topic and availability toggles, query-aware “Clear all” behavior, live result counts, and dialog trigger semantics.
Therapy Compass filter integration
src/components/therapy-compass/screens/search-screen.tsx, tests/therapy-filter-sheet.dom.test.tsx, tests/ui-accessibility.spec.ts
Replaces inline mobile selectors with the filter sheet, excludes search queries from the active-filter count, and tests toggle, clear, dialog, focus, tap-target, and dismissal behavior.
Document filter panel refinements
src/components/clinical-dashboard/document-search-results.tsx, tests/document-filter-panel.dom.test.tsx, docs/branch-review-ledger.md
Increases the small-screen footer action height and verifies dialog relationships, closed-state attributes, unmount behavior, and scroll-lock release.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
participant SearchScreen
participant TherapyFilterTrigger
participant TherapyFilterSheet
SearchScreen->>TherapyFilterTrigger: render open state and active filter count
TherapyFilterTrigger->>TherapyFilterSheet: open or close dialog
TherapyFilterSheet->>SearchScreen: submit toggles or clear action
SearchScreen->>TherapyFilterSheet: provide filtered result count
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly identifies the primary change: replacing the phone listbox controls with proper multi-select filter behavior.
Description check✅ PassedThe description covers the summary, verification results, risks, rollback, governance assessment, and notes with clear scope and applicability details.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

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 ↗︎.

@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@BigSimmo
BigSimmo marked this pull request as ready for review July 31, 2026 16:24
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

Comment threadsrc/components/therapy-compass/filter-sheet.tsx
@BigSimmo
BigSimmo enabled auto-merge (squash) July 31, 2026 16:40
@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

Phone Clear lives in the filter sheet; activeCount ignored the query so a
query-only session hid Clear all. Count a trimmed query and pass it from
SearchScreen so the trigger badge stays consistent.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@BigSimmo
BigSimmo disabled auto-merge July 31, 2026 16:46

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
tests/therapy-filter-sheet.dom.test.tsx (1)

7-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reset the shared mocks between tests.

baseProps holds module-level vi.fn() instances. Call counts accumulate across tests. Current assertions use a local onClear, so they pass today. Add a reset so later assertions on onClose or onToggleTopic stay reliable.

♻️ Proposed reset hook
-import { describe, expect, it, vi } from "vitest";+import { afterEach, describe, expect, it, vi } from "vitest";
 describe("TherapyFilterSheet Clear all", () => {
+ afterEach(() => {+ vi.clearAllMocks();+ });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/therapy-filter-sheet.dom.test.tsx` around lines 7 - 20, Reset the
module-level vi.fn() mocks in baseProps between tests so call counts do not leak
across cases. Add an appropriate test lifecycle reset near baseProps, clearing
or resetting onClose, onToggleTopic, onToggleReviewed, onToggleBrief, and
onClear while preserving each test’s existing mock implementations.
tests/ui-accessibility.spec.ts (1)

539-548: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the aria-controls linkage while the panel is open.

The trigger sets aria-controls only when the sheet is open. That linkage is a stated PR objective and is not covered. Add the assertion after the panel becomes visible.

♻️ Proposed added assertion
 const therapyFilterPanel = page.getByTestId("therapy-filter-panel");
await expect(therapyFilterPanel).toBeVisible();
+ await expect(therapyFilterTrigger).toHaveAttribute("aria-expanded", "true");+ const panelId = await therapyFilterPanel.getAttribute("id");+ expect(panelId).toBeTruthy();+ await expect(therapyFilterTrigger).toHaveAttribute("aria-controls", panelId!);

Also applies to: 580-585

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/ui-accessibility.spec.ts` around lines 539 - 548, Add coverage in the
therapy filter accessibility test after the filter panel becomes visible: assert
that therapyFilterTrigger has an aria-controls attribute pointing to the open
panel’s identifier. Apply the same assertion to the corresponding test block
around the second occurrence, preserving the existing visibility and
accessible-name checks.
src/components/therapy-compass/filter-sheet.tsx (1)

53-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Share one active-filter formula between the sheet and the trigger.

SearchScreen computes the same count at line 26 of src/components/therapy-compass/screens/search-screen.tsx and passes it to TherapyFilterTrigger. This file recomputes it at line 56. The two formulas must stay identical, or the trigger badge and the "Clear all" visibility will disagree. Export one helper, or accept activeCount as a prop.

♻️ Proposed shared helper
+export function therapyActiveFilterCount(input: {+ activeTopics: readonly string[];+ reviewedOnly: boolean;+ briefOnly: boolean;+ query: string;+}) {+ return (+ input.activeTopics.length + Number(input.reviewedOnly) + Number(input.briefOnly) + (input.query.trim() ? 1 : 0)+ );+}+
export function TherapyFilterSheet({
- const activeCount = activeTopics.length + Number(reviewedOnly) + Number(briefOnly) + (query.trim() ? 1 : 0);+ const activeCount = therapyActiveFilterCount({ activeTopics, reviewedOnly, briefOnly, query });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/therapy-compass/filter-sheet.tsx` around lines 53 - 56, Share
the active-filter count calculation currently used by the filter sheet’s
activeCount and SearchScreen’s count near TherapyFilterTrigger. Extract and
reuse one helper, or pass the already computed activeCount into the sheet,
ensuring both the trigger badge and Clear all visibility use the identical
formula including the trimmed query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/therapy-compass/filter-sheet.tsx`:
- Around line 90-133: Add the min-h-12 utility to the className of the topic and
availability filter toggle buttons in the topics map, Reviewed only, and Brief
available controls, while preserving their existing selected-state classes and
behavior.
In `@tests/document-filter-panel.dom.test.tsx`:
- Around line 203-218: Add an assertion in the “releases the scroll lock when a
refetch unmounts an open panel” test after opening the panel and before the
loading rerender, verifying that document.body.style.overflow is "hidden";
retain the existing post-rerender assertion that the overflow is no longer
hidden.
In `@tests/ui-accessibility.spec.ts`:
- Around line 558-563: Update the tap-target assertions for therapyFilterTrigger
to require both width and height to be at least 48px, ensuring the test enforces
the repository’s min-h-12 production requirement.
---
Nitpick comments:
In `@src/components/therapy-compass/filter-sheet.tsx`:
- Around line 53-56: Share the active-filter count calculation currently used by
the filter sheet’s activeCount and SearchScreen’s count near
TherapyFilterTrigger. Extract and reuse one helper, or pass the already computed
activeCount into the sheet, ensuring both the trigger badge and Clear all
visibility use the identical formula including the trimmed query.
In `@tests/therapy-filter-sheet.dom.test.tsx`:
- Around line 7-20: Reset the module-level vi.fn() mocks in baseProps between
tests so call counts do not leak across cases. Add an appropriate test lifecycle
reset near baseProps, clearing or resetting onClose, onToggleTopic,
onToggleReviewed, onToggleBrief, and onClear while preserving each test’s
existing mock implementations.
In `@tests/ui-accessibility.spec.ts`:
- Around line 539-548: Add coverage in the therapy filter accessibility test
after the filter panel becomes visible: assert that therapyFilterTrigger has an
aria-controls attribute pointing to the open panel’s identifier. Apply the same
assertion to the corresponding test block around the second occurrence,
preserving the existing visibility and accessible-name checks.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 980b57bd-90df-4752-854f-6ab0a0d7f887

📥 Commits

Reviewing files that changed from the base of the PR and between 845e12e and 42821f7.

📒 Files selected for processing (7)
  • docs/branch-review-ledger.md
  • src/components/clinical-dashboard/document-search-results.tsx
  • src/components/therapy-compass/filter-sheet.tsx
  • src/components/therapy-compass/screens/search-screen.tsx
  • tests/document-filter-panel.dom.test.tsx
  • tests/therapy-filter-sheet.dom.test.tsx
  • tests/ui-accessibility.spec.ts

Comment threadsrc/components/therapy-compass/filter-sheet.tsx
Comment threadtests/document-filter-panel.dom.test.tsx
Comment threadtests/ui-accessibility.spec.ts
…ish the review
The query-only Clear all fix on 42821f7 was right about the bug and wrong about
where to put the count. It added the trimmed query to the trigger's
`activeFilterCount`, so searching "anxiety" with nothing filtered rendered a
badge reading "1" and announced "1 filter active". A search term is not a
filter, and this screen's sheet exists precisely because its controls used to
describe a state the page was not in.
The two counts are now deliberately different, which is the point:
- `clearableCount` (sheet) = topics + availability + query. Clear all appears
whenever `clearSearch` has something to reset, matching the wide viewport's
unconditional Clear. The query belongs here because `clearSearch` resets it.
- `activeFilterCount` (trigger badge) = topics + availability. The badge is
labelled "N filters active" and must not count a search term.
Remaining review findings, all verified before acting:
- Sheet toggles were `tc-control`, which resolves to `--spacing-tap` = 44px.
Raised to `min-h-12`; a dialog has the room and these are thumb targets.
- The scroll-lock test asserted only the release, so it would have passed the
day the sheet stopped locking at all. It now asserts the lock is taken first.
- `aria-controls` is asserted against the open panel's own id.
- Module-level mocks in the therapy DOM test now reset between cases.
Two findings deliberately not applied. The suggestion to share one active-filter
formula between sheet and trigger is the opposite of the fix above — the
formulas must differ, and a shared helper would re-merge exactly what this
commit separates. And the trigger's tap-target assertion stays at 44px: it sits
in the ribbon's utility row beside `ResultSortControl`, which is `min-h-tap`, so
raising only this control would leave the row ragged. The repo's `min-h-12` rule
exists to stop generic a11y advice pulling production down to `min-h-11`, not to
override a row's own rhythm.
Verified: `npm run verify:cheap` exit 0, 460 files / 4797 tests passed;
`tests/ui-accessibility.spec.ts` 15 passed, chromium.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@BigSimmo
BigSimmo enabled auto-merge (squash) July 31, 2026 16:59
@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

…ription
`.github/workflows/ci.yml`'s "Sync PR policy body" job reads `PR_POLICY_BODY.md`
from the PR head and overwrites the pull request description with it. The file
is meant to be a per-branch scratch template, but #1546 committed its own copy
to `main` (845e12e), so every PR whose head contains it — which is now every
PR — has its description replaced with #1546's body.
That is not cosmetic. `scripts/pr-policy.mjs` parses the description as
merge-gating input, so the Clinical Governance Preflight and the `RAG impact:`
line currently shown on unrelated PRs belong to a different change, and the
verification evidence they assert was never run for the diff they sit on. This
PR's own description was replaced with #1546's `.ckb-v2` dark-cascade notes,
claiming `tests/ckb-v2-token-contract.test.ts — 35 passed` for a diff that does
not touch that file.
The workflow already does the right thing when the file is absent — it logs
"No PR_POLICY_BODY.md template on this head; skipping PR body sync" — so
deleting it restores author-written descriptions everywhere without a workflow
change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@ecc-tools

ecc-toolsBot commented Jul 31, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@ecc-tools

ecc-toolsBot commented Aug 1, 2026

Copy link
Copy Markdown

🔒 Upgrade Required

Private repository analysis requires Pro or Enterprise.

Upgrade: https://ecc.tools/pricing?plan=pro


ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Unblock the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker.

If the PR is already merged or closed: confirm outcome, post one summary, and stop.

Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix.

Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot.

Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop.

Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push.

Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip.

One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me.

Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress.

@cursor

cursorBot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR #1548 (claude/top-search-design-mockups-w53znc @ 6120e36f) — unblocked on content; merge left to you / auto-merge.

Snapshot

  • Tip matches GitHub head6120e36f (local ahead commits not treated as tip)
  • 0 behind / 9 ahead of origin/main; merge-tree clean — no sync
  • Auto-merge armed — no push (would cancel in-flight CI)
  • Local package-lock.json WIP preserved

What I did

  • Dispositioned + resolved the only open thread (CodeRabbit 48px tap-target) — intentional ribbon rhythm; sheet toggles already min-h-12
  • No code fix needed for mergeability or required CI

Required CI

  • Green: Static PR, Safety/config, Build, Unit coverage, Production UI critical, PR policy, PR mergeability
  • In progress: Production UI (1)/(2)/(3)
  • BLOCKED only while those shards finish

Ledger append is local only (not pushed — ledger-only tip rule). Summary comment is on the PR.

Open in WebOpen in Cursor

@coderabbitai

coderabbitaiBot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

The agent ran but didn't make any changes. The issues may already be fixed or require manual intervention.

@cursor

cursorBot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Unblock snapshot — PR #1548

Tip:6120e36ffc1d70d6b04485ec75b9320a88e0f537 (claude/top-search-design-mockups-w53znc)
Sync / merge-tree:0 behind / 9 ahead of origin/main; git merge-tree --write-treeclean. No sync/merge needed.
Auto-merge: armed (squash) — no push made (would only cancel in-flight required CI).

What blocked / what I did

ItemDisposition
MergeabilityAlready MERGEABLE; merge-tree clean vs main
Unresolved CodeRabbit thread (raise trigger tap-target assert to 48px)Dispositioned + resolved — intentional: trigger shares the ribbon utility row with ResultSortControl (min-h-tap / 44px); sheet toggles already use min-h-12. Same rationale is in the PR body and tests/ui-accessibility.spec.ts.
Required CINo code fix needed

Required CI on this tip

  • Pass: Static PR checks, Safety and config, Build, Unit coverage, Production UI critical, PR policy, PR mergeability, Semgrep, Gitleaks, autofix
  • In progress: Production UI (1)/(2)/(3) (full matrix after critical)
  • Advisory: Visual baselines + Lighthouse pass; no failed required checks observed

Residual

  • mergeStateStatus: BLOCKED only while Production UI shards finish (auto-merge should take it from there).
  • Local WIP preserved (package-lock.json dirty); ledger append recorded locally and not pushed (ledger-only tip rule).
  • Merge left to you / auto-merge.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/therapy-compass/screens/search-screen.tsx (1)

3-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Resolve the failed PR-local gate and correct the ledger record.

verify:pr-local reported two failed tests: tests/pdf-extractor.test.ts and tests/resolve-tsx-cli.test.ts. The appended ledger row also contains the literal placeholders <outcome> and <checks>. Resolve the failures, then append a record with the actual outcome and checks.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/therapy-compass/screens/search-screen.tsx` around lines 3 - 8,
Run verify:pr-local and fix the failures in tests/pdf-extractor.test.ts and
tests/resolve-tsx-cli.test.ts, updating the relevant implementation or test
expectations as needed. Then replace the appended ledger row’s literal <outcome>
and <checks> placeholders with the actual verification result and checks
performed.

Source: Coding guidelines

🧹 Nitpick comments (1)
tests/therapy-filter-sheet.dom.test.tsx (1)

54-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Add an integration test for SearchScreen filter wiring.

These tests render TherapyFilterSheet and TherapyFilterTrigger in isolation. They do not exercise SearchScreen lines 31-33, 47-52, or 94-107. A regression in activeFilterCount, filterPanelId, or callback wiring could pass these tests.

Add a focused SearchScreen test for query-only state and one selected filter. Assert the trigger announcement and sheet callback behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/therapy-filter-sheet.dom.test.tsx` around lines 54 - 74, Add focused
integration coverage for SearchScreen that renders its actual
TherapyFilterTrigger and TherapyFilterSheet wiring, covering query-only state
and one selected filter. Assert activeFilterCount produces the correct trigger
announcement, filterPanelId connects the trigger to the sheet, and selecting or
clearing a filter invokes the expected SearchScreen callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/therapy-compass/screens/search-screen.tsx`:
- Around line 3-8: Run verify:pr-local and fix the failures in
tests/pdf-extractor.test.ts and tests/resolve-tsx-cli.test.ts, updating the
relevant implementation or test expectations as needed. Then replace the
appended ledger row’s literal <outcome> and <checks> placeholders with the
actual verification result and checks performed.
---
Nitpick comments:
In `@tests/therapy-filter-sheet.dom.test.tsx`:
- Around line 54-74: Add focused integration coverage for SearchScreen that
renders its actual TherapyFilterTrigger and TherapyFilterSheet wiring, covering
query-only state and one selected filter. Assert activeFilterCount produces the
correct trigger announcement, filterPanelId connects the trigger to the sheet,
and selecting or clearing a filter invokes the expected SearchScreen callbacks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fcfd4402-f803-490a-a64b-a3c1bba7231d

📥 Commits

Reviewing files that changed from the base of the PR and between 42821f7 and 6120e36.

📒 Files selected for processing (6)
  • PR_POLICY_BODY.md
  • src/components/therapy-compass/filter-sheet.tsx
  • src/components/therapy-compass/screens/search-screen.tsx
  • tests/document-filter-panel.dom.test.tsx
  • tests/therapy-filter-sheet.dom.test.tsx
  • tests/ui-accessibility.spec.ts
💤 Files with no reviewable changes (1)
  • PR_POLICY_BODY.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/document-filter-panel.dom.test.tsx
  • src/components/therapy-compass/filter-sheet.tsx
  • tests/ui-accessibility.spec.ts

@BigSimmo
BigSimmo merged commit 40814b4 into mainAug 1, 2026
49 checks passed
@BigSimmo
BigSimmo deleted the claude/top-search-design-mockups-w53znc branch August 1, 2026 01:35
cursorBot pushed a commit that referenced this pull request Aug 4, 2026
…1609)
* docs: replace the search-bar handoff with a durable decisions record
`docs/handoff-search-bar.md` shipped to main in #1555. It existed to carry one
unverified commit across a session boundary, and its instructions are now false:
it tells the reader that `6917e732` is unverified and that no PR should be
opened on it, when #1555 merged exactly that work. Leaving it in the repo means
the next person to read it acts on stale gate status.
Its durable content — results-bar anatomy, why the filter shelf covers two modes
rather than eight, and the two things deliberately not done (the library button
stays until nav can preserve the query; Sort does not move into the phone sheet
from the shared band) — moves to docs/search-results-bar-decisions.md, verified
against current main rather than copied forward: `appliedFilters`/`onClearFilters`
still have exactly the two production consumers the doc claims, and the
`Open source library` control is still there.
Also records the PR-policy body defect that #1555's handoff flagged but never
captured: ci.yml's body-sync job reads PR_POLICY_BODY.md from the PR head, so
committing that scratch file to main (#1546) replaced every open PR's
description, and pr-policy.mjs parses the body as merge-gating input. #1548
deleted the file; nothing stops the next branch adding one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(ledger): record the search-bar decisions-doc review
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs: fix review findings on search-bar decisions record
Reconcile the twelve results-band modes with shelf scope, name the three
sheetless Sort consumers, tighten #230 to heads that contain
PR_POLICY_BODY.md, and update #170 so documents/therapy sheets match code.
* docs(ledger): record review-fix verification at tip
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@BigSimmo@claude@cursoragent