Documents: one reachable filter panel, and a library control named after what it opens by BigSimmo · Pull Request #1536 · BigSimmo/Database

BigSimmo · 2026-07-31T12:43:59Z

Summary

The smart-tag facet panel was unreachable in production.DocumentTagFacetRail was mounted only when activeFacetKeys.length > 0, and the only writers of that state — onToggle and onClear — lived inside the gated subtree, so no sequence of clicks could ever satisfy the gate. It has been that way on main. The three earlier commits on this branch (recount against selection, OR-within/AND-across, dead-end facets) fixed logic no user could reach.
One filter surface instead of two. Source type (All/Tables/Images/PDFs) was a separate control — a chip row inside the ribbon on desktop, a native <select> on phones. It now lives in the same DocumentFilterPanel as the tag facets, opened by a Filter trigger in the results ribbon. Source type keeps radio semantics because it is mutually exclusive; the tag facets keep aria-pressed because they are not. The panel footer carries the live result count, which is what tells a reader whether the combination they have built still returns anything.
The library control is named after what it opens. "Sources", labelled Open source filters with the title Filter and browse sources, sat next to the new Filter trigger and read as a second filter. It is now "Library" / Open source library / Browse all indexed sources. The documents action menu names the same destination the same way ("Collections" / "Open document folders" → "Browse library" / "All indexed sources"), as does the mode-home tile, whose description was literally "Filter all indexed sources."
Ledger row #182 (renumbered from #176 after merging main) records a related finding this work turned up but does not fix: the command-scope system is inert. universal-search-command-surface.tsx:392-393 voids both commandScopes and its setter, and every other write passes an empty array, so the scope chips configured for six modes, the three matcher helpers, and the scope shelf in the results band are all unreachable. Not a wiring-conventions defect — nothing is clickable — but it needs a decision before more filtering UI is built on it.

One thing deliberately not done: moving the library control out of the ribbon into the action menu. That looked correct and was wrong. The menu's handler routes through onSearchModeChange, which does setQuery("") and setModeSearchSubmitted(false) (ClinicalDashboard.tsx:2670-2677), so reaching the library that way discards the search being read. The ribbon button is the only in-context route to it. The browser run caught this; reading the code did not.

Verification

npm run verify:cheap — exit 0, Test Files 450 passed (450), Tests 4710 passed | 4 skipped (4714)
npm run verify:ui scope covered by a direct ui-smoke.spec.ts Chromium run: 93 passed, 1 failed in 2.5m. The one failure is document viewer puts the PDF preview first with pinned evidence after it on mobile, at pdfScroller.locator("canvas"). It fails identically with these changes stashed, so it is pre-existing and environmental — this box has Chromium 1194 against the project's pinned 1228, run via the PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH hook the config already supports.
Mutation-tested: restoring the old activeFacetKeys.length > 0 gate fails all six tests in the new tests/document-filter-panel.dom.test.tsx, and removing it passes all six. The tests guard the fix rather than merely accompanying it.
npm run verify:pr-local — not run; verify:cheap plus the direct Chromium spec run is the evidence above.
npm run eval:retrieval:quality, eval:rag, eval:quality, check:production-readiness — not run and not applicable. No retrieval, ranking, selection, chunking, scoring, or answer-generation code is touched.

RAG impact: no retrieval behaviour change — this changes only which client-side controls are mounted and how already-returned DocumentMatch[] results are filtered and labelled in the documents results view. 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, confined to the documents results view. The main behavioural change is that a filtering surface which previously could not be opened now can be, so filter combinations that returned nothing were unreachable before and are reachable now — the panel disables dead-end facets and reports the live count for exactly that reason. Secondary risk is the ribbon control rename, which changes two accessible names that Playwright asserts on; both assertions are updated in this PR.
Rollback: three independent commits. f92ad5ea (naming) reverts without touching the panel; 83f67187 (panel) reverts without touching the ledger row; 608c5912 is docs only. Note that a squash merge folds them, so a post-merge revert of one item means reverting its hunks by hand.
Provider or production effects: None. No provider-backed gate was run and no Supabase, OpenAI, or Railway surface is touched.

Clinical Governance Preflight

classifyPullRequestFiles returns clinicalRisk: true for this file set because src/lib/document-tags.ts matches the lib document path pattern (client-side tag facet helpers only — no retrieval, ranking, or document-access change). Governance checklist completed for that classifier hit:

Source-backed claims still require linked source verification before clinical use
No patient-identifiable document workflow was introduced or expanded without explicit governance approval
Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
Service-role keys and private document access remain server-only
Demo/synthetic content remains clearly separated from real clinical sources
Source metadata, review status, and outdated/unknown-source behavior remain conservative
Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Document results were already rendered by this component; this PR changes only which controls filter them client-side.

Notes

The Filter trigger renders into both of the ribbon's page-control slots, because the ribbon shows mobileControls below sm and filterControls from sm up. Both copies are in the DOM, so they carry distinct test ids (document-filter-trigger-phone / -wide); a shared id makes every Playwright lookup ambiguous under strict mode, which is the failure mode that took out an earlier PR on this branch.
No tap-target assertion on the wide trigger: from sm up the ribbon controls are deliberately min-h-10 (40px) for fine pointers, and the 44px floor is a phone contract asserted on the phone trigger at 390px. Asserting the phone floor at 1440px was a genuine failure in the first browser run.
After merging main, this PR's #175/#176 ledger captures were renumbered to #181/#182 so they do not collide with main's therapy/scroll findings.

…174) Every selected facet key ANDed, regardless of which group it came from. So picking two medications asked for a document about *both* lithium and clozapine, which returns nothing in almost every corpus — multi-select within a group was a dead affordance rather than a feature, and it was the main source of the zero-count dead ends the previous commit had to guard against. Two values from the same group are alternatives: "lithium or clozapine", "renal or thyroid risk". Values from different groups are constraints that stack: "lithium documents, about renal risk". That is OR within a group, AND across groups — the conventional faceted-search model, and the one that matches what the labels mean. Group membership is read from the tags each index entry already carries, not from `index.groups`: that list is sliced to `limitPerGroup`, so a selected key outside the top N of its group would have no resolvable group. A key whose group cannot be resolved is bucketed under its own identity so it still constrains rather than being silently dropped from the filter. `projectSmartTagFacetGroups` now shares the filter's predicate rather than narrowing an already-filtered subset. It has to: adding a key to a group that is already selected *widens*, so the old subset-narrowing shortcut would have under-reported every same-group sibling — the exact numbers this fix exists to correct. Migration risk is close to nil. The behaviour being replaced almost never produced results, so nothing can depend on it: under the old rules a same-group sibling counted 0 and led to an empty list. Tests: seven cases covering alternatives within a group, narrowing across groups, both rules at once, independent widening per group, a single selection unchanged, count/filter agreement under the new rules, and a same-group sibling reported as widening rather than as a dead end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

Two small things, both about not hiding information. A zero-count facet used `disabled`, which drops it out of the tab order. A keyboard or screen-reader user then loses the row entirely and never learns why it went quiet — and a `title` on a disabled control is not reliably announced. It now uses `aria-disabled` with the click guarded, so the row stays focusable and carries an sr-only explanation via `aria-describedby`. That is the disabled-affordance pattern docs/wiring-conventions.md already describes. `#175` corrects `#171`, which is already on main. `#171` claims the documents source-type control duplicates the `Document type` facet group. It does not, and the claim was made from a shared word rather than from the code: `resultTypeTabs`/`filterMatchesByResultType` filter on artefact properties — tableCount, imageCount, a .pdf extension — while the `Document type` facet comes from `document_type` labels meaning policy, guideline, form. A guideline containing a table is both. They are complementary axes. The real duplication is the scope chip `tables` against the source-type `Tables` tab. The merge `#171` recommends is still worth doing, but source-type becomes its own group rather than being absorbed, and the scope chips are the part that folds away. `#171`'s separate claim that `Sources` is navigation stands. The correction is appended rather than edited in: the ledger is append-only, and `#158` is precedent for keeping a withdrawn finding on record so it is not re-filed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

Traced while re-checking Batch B's premise. universal-search-command-surface voids both commandScopes and its setter, and every other write passes an empty array, so the scope chips configured for six modes, the three matcher helpers, and the scope shelf in the results band are all unreachable in production. Not a wiring-conventions defect — nothing is clickable — but it reads as a feature and needs a decision before more filtering UI is built on top of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

The smart-tag facets were unreachable in production. `DocumentTagFacetRail` was mounted only when `activeFacetKeys.length > 0`, and the only writers of that state — `onToggle` and `onClear` — lived inside the gated subtree, so no sequence of clicks could ever satisfy the gate. The three earlier commits on this branch (recount against selection, OR-within/AND-across, dead-end facets) fixed logic no user could reach. Source type (All/Tables/Images/PDFs) was the second filtering surface: a chip row inside the ribbon on desktop, a native `<select>` on phones. Two surfaces, one of them dead. Both now live in `DocumentFilterPanel`, opened by a Filter trigger in the ribbon. Source type keeps radio semantics because it is mutually exclusive; the tag facets keep `aria-pressed` because they are not. The panel footer carries the live result count, which is the thing that tells a reader whether the combination they have built still returns anything. The trigger renders into both of the ribbon's page-control slots, since it shows `mobileControls` below `sm` and `filterControls` from `sm` up. Both copies are in the DOM, so they carry distinct test ids — a shared one makes every Playwright lookup ambiguous under strict mode. Verified: `npm run verify:cheap` exit 0, 450 files / 4710 tests passed. Mutation-tested — restoring the old gate fails all six new DOM tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

"Sources", labelled "Open source filters" with the title "Filter and browse sources", sat next to the new Filter trigger and read as a second filter. It is not one — it opens the source library drawer. It is now "Library" / "Open source library" / "Browse all indexed sources", and the documents action menu names the same destination the same way ("Collections" / "Open document folders" -> "Browse library" / "All indexed sources"), as does the mode-home tile, whose description was literally "Filter all indexed sources." The control stays in the ribbon rather than moving into the action menu. Removing it looked right and was wrong: the menu's handler routes through `onSearchModeChange`, which does `setQuery("")` and `setModeSearchSubmitted(false)` (ClinicalDashboard.tsx:2670-2677), so reaching the library that way discards the search being read. The ribbon button is the only in-context route to it. Caught by running the browser, not by reading. Verified: `npm run verify:cheap` exit 0, 450 files / 4710 tests passed; `ui-smoke.spec.ts` 93 passed / 1 failed, the failure being the document-viewer PDF canvas test, which fails identically with these changes stashed (Chromium 1194 here against the pinned 1228). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

coderabbitai · 2026-07-31T12:44:05Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:49 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de57e135-2f8f-43c0-861d-499a6c452b19

📥 Commits

Reviewing files that changed from the base of the PR and between e2299c1 and fc80da9.

📒 Files selected for processing (9)

docs/branch-review-ledger.md
docs/outstanding-issues.md
src/components/clinical-dashboard/document-search-results.tsx
src/components/clinical-dashboard/mode-action-popup.tsx
src/lib/document-tags.ts
tests/document-filter-panel.dom.test.tsx
tests/document-search-record-fault.dom.test.tsx
tests/document-tags.test.ts
tests/ui-smoke.spec.ts

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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

supabase · 2026-07-31T12:44:06Z

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 · 2026-07-31T12:44:08Z

🔒 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.}

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY

ecc-tools · 2026-07-31T12:46:08Z

🔒 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.}

chatgpt-codex-connector · 2026-07-31T12:51:52Z

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.

BigSimmo · 2026-07-31T12:52:20Z

@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 · 2026-07-31T12:52:25Z

PR #1536 is unblocked at tip fc80da97a421ac26ee06f92f50f41801d522a43d.

Cleared

Merge conflict on docs/outstanding-issues.md (merged main, renumbered #181/#182)
PR policy Clinical Governance Preflight (document-tags.ts → clinicalRisk: true)
Sentry: invalid facet ids + panel stuck across query change
Static PR lint: replaced useEffect+setState with query-scoped panel open (same pattern as facets)

Status now

MERGEABLE, merge-tree vs origin/main clean, 0 unresolved threads
Required: PR policy / mergeability / Static / Unit / Build / Production UI critical — all green
Production UI (1)/(2)/(3) still in progress (expected; not a content conflict)
Ledger row kept local only (no ledger-only push)

Merge left to you.

qodo-code-review · 2026-07-31T12:53:07Z

PR Summary by Qodo

Documents: unify filter UI, fix unreachable facets, rename Library control

🐞 Bug fix✨ Enhancement🧪 Tests📝 Documentation🕐 40+ Minutes

AI Description

• Make document smart-tag facets reachable via a ribbon "Filter" trigger.
• Consolidate source-type + tag facets into one filter panel with clear-all + live count.
• Rename "Sources" to "Library" consistently across ribbon, menus, home, and tests.

Diagram

graph TD
A["SearchResultsHeaderBand"] --> B["DocumentFilterTrigger"] --> C["DocumentFilterPanel"] --> D["document-tags (facet filter)"] --> E["Filtered results list"]
A --> F["Library drawer"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep source-type tabs in the ribbon; only move facets behind Filter

➕ One-click access to the most common filter (All/Tables/Images/PDFs).
➕ Less UI shown/hidden behind an extra click.
➖ Leaves two separate filter surfaces, undermining the "one place to undo" goal.
➖ Still requires users to learn where each narrowing control lives per breakpoint.

2. Use a modal/sheet filter drawer (especially on mobile)

➕ More space for facets; easier to scan long lists.
➕ Natural pattern for mobile filtering and clear confirm/done affordance.
➖ Higher implementation complexity (focus trapping, transitions, layering).
➖ Slower iteration for what is currently an inline panel in the flow.

Recommendation: The PR’s approach (single ribbon trigger opening one shared filter panel) is the better fit for reducing cognitive load and preventing split-brain filtering across breakpoints. Keeping the library control in the ribbon (but renaming it) is also correct given the existing action-menu route clears the active query; relocating it would be a behavior regression.

Files changed (9) +623 / -139

Enhancement (2) +70 / -9

mode-action-popup.tsxRename documents action menu entry to "Browse library"+1/-1

Rename documents action menu entry to "Browse library"
• Updates the documents-mode action set label/description so navigation to the indexed sources destination is consistently named "Browse library" / "All indexed sources".
src/components/clinical-dashboard/mode-action-popup.tsx

document-tags.tsFacet selection semantics: OR within group, AND across groups+69/-8

Facet selection semantics: OR within group, AND across groups

• Implements grouped facet combination rules by partitioning selected keys by facet group and matching entries with OR-within-group/AND-across-groups semantics. Updates facet count projection to use the same predicate as filtering, preventing count/filter drift and handling keys whose group is not resolvable from sliced group lists.

src/lib/document-tags.ts

Bug fix (1) +240 / -102

document-search-results.tsxIntroduce single DocumentFilterPanel + trigger; rename Library control+240/-102

Introduce single DocumentFilterPanel + trigger; rename Library control

• Replaces the unreachable facet rail and separate source-type controls with a single Filter trigger that opens a unified panel containing source-type radios and smart-tag facet toggles. Adds active-filter counting, clear-all that resets both filter kinds, an aria-live result count with a Done action, and improves dead-end facet accessibility via aria-disabled + SR explanation. Renames the ribbon navigation control from "Sources" to "Library" and updates ModeHome tile copy to match.

src/components/clinical-dashboard/document-search-results.tsx

Tests (4) +309 / -27

document-filter-panel.dom.test.tsxAdd DOM tests for filter panel reachability and behavior+180/-0

Add DOM tests for filter panel reachability and behavior

• Adds a focused regression suite proving the filter panel is reachable from the ribbon trigger, contains both source-type and tag facets, filters results correctly, clears both filter kinds, and closes via the "Show N documents" action.

tests/document-filter-panel.dom.test.tsx

document-search-record-fault.dom.test.tsxUpdate record-fault test expectations for "Browse library"+1/-1

Update record-fault test expectations for "Browse library"
• Renames the expected mode-home action label from "Browse sources" to "Browse library" to match the updated UI copy.
tests/document-search-record-fault.dom.test.tsx

document-tags.test.tsTest OR-within/AND-across facet selection and count agreement+75/-0

Test OR-within/AND-across facet selection and count agreement

• Adds unit tests validating the new facet combination semantics (alternatives within a group, constraints across groups) and asserts projected facet counts remain consistent with the filter predicate.

tests/document-tags.test.ts

ui-smoke.spec.tsUpdate Playwright smoke coverage for unified filter panel + Library naming+53/-26

Update Playwright smoke coverage for unified filter panel + Library naming

• Replaces assertions for the old ribbon chip row and mobile select with interactions through the new Filter trigger and panel at both mobile and wide breakpoints. Updates expectations for the renamed "Open source library" ribbon button and "Browse library" entry points, and verifies the panel can open/close and apply source-type filtering.

tests/ui-smoke.spec.ts

Documentation (2) +4 / -1

branch-review-ledger.mdAdd ledger row for PR #1536 verification and findings+1/-0

Add ledger row for PR #1536 verification and findings

• Records the documents filter-panel reachability fix, source-type consolidation, and Library renaming. Notes verification status and logs the related (but unfixed) inert command-scope finding as ledger item #176.

docs/branch-review-ledger.md

outstanding-issues.mdAdd issues #175–#176 and advance next-id+3/-1

Add issues #175–#176 and advance next-id

• Updates the issue ledger with a correction about source-type vs document-type facets (#175) and documents the inert command-scope system (#176). Advances the issues next-id marker accordingly.

docs/outstanding-issues.md

BigSimmo · 2026-07-31T12:54:08Z