Skip to content

Search results bar: delete the inert scope system, rebuild the filter shelf, and land the bar anatomy - #1555

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

Search results bar: delete the inert scope system, rebuild the filter shelf, and land the bar anatomy#1555
BigSimmo merged 7 commits into
mainfrom
claude/top-search-design-mockups-w53znc

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

Three changes from the Results bar — perfected study (artifact 007a83f4), applied in the shared results band so all twelve search modes take them at once.

  • Deletes the inert command-scope system. universal-search-command-surface.tsx received commandScopes and onCommandScopesChange and discarded both with void, and every other write in the repo passed an empty array — so the value was permanently [] in production, on main, before this branch existed. Everything built on it was therefore unreachable: the scopes config for six modes, the three matcher helpers, the four call sites that filtered against them (services, forms, prescribing, favourites), and the band's original scope shelf. Behaviour-preserving by construction rather than by argument — each matcher early-returned true on an empty array, so every call site was already a no-op. SearchCommandContext survives with query and modeId, which applications-launcher-page.tsx still consumes.
  • Rebuilds the applied-filter shelf on live facet data. The band once carried a shelf, but it read filter state from a context nothing populated, so it rendered for nobody and passed a DOM test that constructed that context by hand. The new one is prop-driven: the page supplies appliedFilters (id, label, onRemove) and the band renders them. Filtered by labels the row as state rather than a second bank of buttons; every chip removes in one tap with the count following immediately; Clear appears past one chip. It survives loading and a zero result — nothing matching is exactly when you want to relax a filter, and dropping it mid-search would flicker the chips out and back on every keystroke. Only a fault removes it.
  • Bar anatomy. The state tile already existed but carried only two of its four states; it now shows a spinner while a search runs and a funnel once the result set is narrowed, the funnel driven by appliedFilters.length so it tracks the same live data as the shelf. Filter moves hard against the ribbon's right edge and Sort moves inboard: Sort is set about once a session, Filter is the only control carrying state and the one a thumb reaches for.

The shelf is scoped to documents and therapy-compass — the two modes whose filters are multi-valued and hidden behind a panel. Differentials, prescribing, specifiers, formulation, services and factsheets each have a single-select dimension whose control is already visible in the bar, so a shelf there would restate what is on screen.

One study step deliberately not done

The study's step 6 removes the library button from the results bar on the grounds that corpus browsing belongs in nav. It explicitly declines to check where nav puts it. In this repo the documents action menu routes through onSearchModeChange, which calls setQuery("") and setModeSearchSubmitted(false), so reaching the library that way discards the search being read. The bar button is the only in-context route to it, and was renamed Library / Open source library instead. docs/handoff-search-bar.md records this so it is not "finished" by mistake.

Verification

  • npm run verify:pr-local — exit 0. Test Files 460 passed (460), Tests 4796 passed | 4 skipped (4800), production build clean, client-bundle secret surface check passed, offline RAG fixture validation passed (36 golden cases, 23 suites).
  • npm run verify:ui scope covered by direct Chromium spec runs: tests/ui-tools.spec.ts 87 passed; tests/ui-smoke.spec.ts + tests/ui-accessibility.spec.ts 108 passed, 1 failed.
  • ui-tools was run first and separately because it was the real risk: it carried two assertions on the Sort/Filter pair that this work deliberately separates, and it runs expectNoPageHorizontalOverflow at 390 px immediately after them. Both clear. Those assertions are rewritten to pin what still has to hold — the page filter is the utility rail's last child, and both controls keep a matched phone tap height of at least 43 px — rather than the old adjacency.
  • The single 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: this environment has Chromium 1194 against the project's pinned 1228, run through the PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH hook the config already supports.
  • Mutation-tested. Widening the shelf's render condition from !faulted to !countUntrusted fails the pending-search case, so the "survives a search in flight" guard is real rather than vacuous.
  • npm run eval:retrieval:quality, eval:rag, eval:quality, check:supabase-project — not run and not applicable. No retrieval, ranking, selection, chunking, scoring, ingestion, or provider-backed surface is touched.

RAG impact: no retrieval behaviour change — this changes only which client-side controls render, and how already-returned result sets are filtered and labelled. 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. src/lib/search-command-surface.ts is edited, but only to remove the dead scope config and matchers described above; its retrieval-adjacent exports are untouched.

Risk and rollout

  • Risk: UI-only. The largest change by line count is a deletion of code that provably could not execute — every matcher early-returned true on a permanently-empty array. The behavioural change a reader will notice is the shelf appearing when filters are applied, and Filter sitting at the right edge instead of beside Sort. The services, forms, prescribing and favourites pages lose filtering calls that were no-ops, so their result sets are unchanged.
  • Rollback: four independently revertible commits — the scope deletion, the shelf, the bar anatomy, and docs. 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, checked against the full origin/main...HEAD diff rather than the tip commit. The trigger is path-based — src/lib/search-command-surface.ts plus the services and documents result pages — and the edits to all three are removals of unreachable filtering code and changes to which client-side controls render. No ingestion, answer generation, retrieval, source rendering, or document-access path is altered.

  • 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

Notes

  • The wip(search) commit message calls itself UNVERIFIED and points at docs/handoff-search-bar.md. That caveat is superseded by the gate results above; history was not rewritten to correct it in place because the branch was already pushed and a force-push is not worth a stale adjective. The doc records the correction.
  • Two traps worth keeping out of the next change on this surface, both recorded in the handoff doc. Count what a control does, not how many there are — formulation's "Pattern" and factsheets' "Category" look like filters and are navigation, and services' "quick filter" rewrites the query. A shared component must not read filter state from context — that is precisely how the previous shelf shipped dead while its test passed.
  • Still open from the study and not attempted here: moving Sort into the sheet on phone, with the sheet retitled Filter and sort. It is per-page work, because only documents and therapy-compass have sheets; doing it in the shared band would strip Sort from the other six modes on phones, which is a defect the study itself records fixing in an earlier round.

Generated by Claude Code

claude added 5 commits August 1, 2026 01:41
Nothing in the app could ever set a command scope.
`universal-search-command-surface.tsx` received `commandScopes` and
`onCommandScopesChange` and discarded both with `void`, and every other write in
the repo passed an empty array, so the value was permanently `[]` in production.
Confirmed on `origin/main` before this branch existed.

Everything built on it was therefore unreachable, and this deletes all of it:

- the two voided props, and the scope state, callbacks and context fields in
  `ClinicalDashboard`, `global-search-shell` and `master-search-header`
- the `scopes` config arrays for six modes and the `CommandScopeChip` type
- `recordMatchesCommandScopes`, `favouriteMatchesCommandScopes` and
  `medicationMatchesCommandScopes`, plus their scope-matching helpers
- the four call sites that filtered against them — services, forms,
  prescribing, favourites
- the results band's applied-filter shelf, its funnel-icon branch, and the
  empty state's "Clear scope filters" button

Behaviour-preserving by construction rather than by argument: each matcher
early-returned `true` on an empty array, so every call site was already a no-op,
and the shelf's render condition could never be true.

`SearchCommandContext` survives with `query` and `modeId` —
`applications-launcher-page.tsx` still consumes it. Only the scope fields go.

The tests that covered this exercised it from the wrong side: the band DOM test
constructed the context by hand and the surface unit test called the matchers
directly, so neither ever touched the production path. That is why the dead
wiring survived, and both are removed with the code they described.

Ledger `#182` closed with this outcome.

Verified: `npm run verify:cheap` exit 0, 460 files / 4792 tests passed;
`ui-smoke.spec.ts` 93 passed / 1 failed, the failure being the pre-existing
document-viewer PDF canvas test that fails identically without this change
(Chromium 1194 here against the pinned 1228).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
The `Results bar — perfected` study's step 5. The band did once carry a shelf,
but it read `commandScopes` from context — a value nothing could set — so it
rendered for nobody, and the previous commit deleted it with the rest of that
dead system. Deleting the wiring was right; dropping the design decision with it
was not. Facets became reachable in #1536, so the shelf now has a live source it
never had when the study was written.

It is prop-driven this time. The page supplies `appliedFilters` — id, label, and
its own `onRemove` — and the band renders them. That is the specific fix for how
the old one failed: a shared component pulling filter state out of a context no
page populated could look correct in a DOM test and be dead in production.

Applied to the two pages where filters are multi-valued and hidden behind a
panel:

- documents — smart-tag facets plus the source type, since both narrow the same
  list by the same act
- therapy-compass — topics plus the availability toggles

Not applied to differentials, prescribing, specifiers, formulation, services or
factsheets. Each has a single-select dimension whose control is already visible
in the bar, so a shelf would restate what is on screen.

Behaviour follows the study: `Filtered by` labels the row as state rather than a
second bank of buttons, every chip removes in one tap with the count following
immediately, and `Clear` appears past one chip. It survives `loading` and a zero
result — nothing matching is exactly when you want to relax a filter, and
dropping it mid-search would flicker the chips out and back on every keystroke.
Only a fault removes it, because filtering a result set that never loaded is
meaningless. That last guard is mutation-tested: widening the condition to
`!countUntrusted` fails the pending-search case.

Three rows of the study's comparison table are now true that were not: taps to
clear all is 1, applied filters are all visible at 390 px, and filters survive a
zero result.

Verified: `npm run verify:cheap` exit 0, 460 files / 4796 tests passed;
`ui-smoke` + `ui-accessibility` 108 passed / 1 failed, the failure being the
pre-existing document-viewer PDF canvas test that fails identically without this
change (Chromium 1194 here against the pinned 1228).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
UNVERIFIED. Typecheck and lint pass; `verify:cheap` and the browser gates have
NOT been run against this commit. Do not open a PR on it as-is — see
docs/handoff-search-bar.md.

Step 4 of the `Results bar — perfected` study, applied in the shared band so all
twelve search modes take it at once:

- The state tile already existed but carried only two of its four states. It now
  shows a spinner while a search runs and a funnel once the result set is
  narrowed, alongside the existing alert and search icons. The funnel is driven
  by `appliedFilters.length`, so it tracks the same live data as the shelf — the
  previous funnel branch keyed off the inert command-scope system and was removed
  with it.
- Filter moves hard against the ribbon's right edge and Sort moves inboard. Sort
  is set about once a session; Filter is the only control carrying state and the
  one a thumb reaches for.

Three tests asserted Sort and Filter as an adjacent *pair*, which this change
deliberately separates. They are rewritten to assert what still has to hold —
the page filter is the utility rail's last child, and both controls keep a
matched phone tap height of at least 43px — rather than the old adjacency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
Records what is verified, what is not, which study steps are done, and the two
decisions that should not be relitigated — why the library button stays in the
bar, and why the shelf is scoped to two pages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
Supersedes the `wip(search)` commit's UNVERIFIED caveat. History was not
rewritten to correct it in place, because the branch was already pushed and a
force-push is not worth a stale adjective.

`verify:pr-local` exit 0 — 460 files / 4796 tests, production build,
client-bundle secret scan, RAG fixtures 36 cases / 23 suites.
`ui-tools.spec.ts` 87 passed; `ui-smoke` + `ui-accessibility` 108 passed and 1
failed, the failure being the pre-existing document-viewer PDF canvas test that
fails identically with these changes stashed.

`ui-tools` was the real risk and is the reason it was run first and separately:
it carried two assertions on the Sort/Filter pair that this work separates, and
runs `expectNoPageHorizontalOverflow` at 390px straight after them.

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

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

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: 10 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: 0c7efb29-8d53-4cf6-9db1-c712bde771dc

📥 Commits

Reviewing files that changed from the base of the PR and between 6aa9427 and 3f08fa0.

📒 Files selected for processing (20)
  • docs/branch-review-ledger.md
  • docs/handoff-search-bar.md
  • docs/outstanding-issues.md
  • src/components/ClinicalDashboard.tsx
  • src/components/clinical-dashboard/document-search-results.tsx
  • src/components/clinical-dashboard/favourites-command-library-page.tsx
  • src/components/clinical-dashboard/global-search-shell.tsx
  • src/components/clinical-dashboard/master-search-header.tsx
  • src/components/clinical-dashboard/medication-prescribing-workspace.tsx
  • src/components/clinical-dashboard/search-command-context.tsx
  • src/components/clinical-dashboard/search-results-header-band.tsx
  • src/components/clinical-dashboard/universal-search-command-surface.tsx
  • src/components/forms/forms-search-results-page.tsx
  • src/components/services/services-navigator-page.tsx
  • src/components/therapy-compass/screens/search-screen.tsx
  • src/lib/search-command-surface.ts
  • tests/document-filter-panel.dom.test.tsx
  • tests/search-command-surface.test.ts
  • tests/search-results-header-band.dom.test.tsx
  • tests/ui-tools.spec.ts

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

@supabase

supabase Bot commented Aug 1, 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-tools Bot 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.

@ecc-tools

ecc-tools Bot 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
BigSimmo marked this pull request as ready for review August 1, 2026 02:10
@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.

@ecc-tools

ecc-tools Bot 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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Remove inert command scopes; add prop-driven applied-filter shelf and results bar states

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

Grey Divider

AI Description

• Delete the unreachable command-scope system and its no-op consumers across search modes.
• Rebuild the applied-filter shelf as prop-driven chips sourced from live facet state.
• Update results bar anatomy: spinner/funnel tile states and mobile Sort/Filter layout.
Diagram

graph TD
  A["Documents results page"] --> B["Build appliedFilters[]"] --> D["SearchResultsHeaderBand"] --> E["Applied-filter shelf"]
  C["Therapy Compass search"] --> B
  D --> F["Tile states (spinner/funnel)"]
  G["SearchCommandContext (query, modeId)"] --> H["UniversalSearchCommandSurface"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Wire command scopes end-to-end instead of deleting
  • ➕ Preserves the conceptual 'scope chips' feature if it’s still desired
  • ➕ Could unify certain filtering affordances across modes
  • ➖ Requires UX decisions and real interaction design (chips vs dropdown vs panel)
  • ➖ Higher risk: would change production filtering behavior that was previously inert
  • ➖ Adds ongoing maintenance surface across many modes and tests
2. Keep shelf context-driven (populate shared context from pages)
  • ➕ Centralizes applied filter state and avoids per-page prop plumbing
  • ➕ Could enable cross-mode persistence if ever needed
  • ➖ Easy to regress into the prior failure mode (context exists but no producer)
  • ➖ Harder to keep mode-specific semantics correct (facet keys, labels, remove handlers)
3. Derive applied filters inside SearchResultsHeaderBand from standard interfaces
  • ➕ Would reduce duplication if all modes implement a common filter model
  • ➕ Could standardize labels and chip behaviors
  • ➖ Not currently feasible: filter state is heterogeneous (facets vs result-type vs toggles)
  • ➖ Pushes mode-specific knowledge into a shared component

Recommendation: Keep the PR’s approach: deleting the inert scope system is the safest and clearest outcome given it was behaviorally unreachable, and moving the shelf to prop-driven appliedFilters makes ownership explicit and prevents the prior ‘context with no producer’ regression. If scope chips are ever reintroduced, treat it as a separate feature with explicit UX + end-to-end tests rather than reviving the previous scaffolding.

Files changed (20) +358 / -479

Enhancement (3) +120 / -92
document-search-results.tsxDerive applied filter chips from facet + result-type state and pass to header band +36/-4

Derive applied filter chips from facet + result-type state and pass to header band

• Builds an appliedFilters array from active facet keys and the result-type tab (when not 'all'), each with an onRemove handler. Adds a clearAllFilters helper and wires appliedFilters/onClearFilters into SearchResultsHeaderBand and the filter panel's Clear action.

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

search-results-header-band.tsxAdd prop-driven applied-filter shelf and update results bar states/layout +74/-88

Add prop-driven applied-filter shelf and update results bar states/layout

• Introduces AppliedFilterChip prop API and renders a labelled 'Filtered by' shelf with one-tap chip removal and optional Clear. Updates the tile icon logic to show spinner while busy and funnel when filtered, and changes mobile layout so Sort is inboard and page filter controls sit last at the right edge; removes scope-based empty-state affordances.

src/components/clinical-dashboard/search-results-header-band.tsx

search-screen.tsxProvide appliedFilters chips for Therapy Compass shelf +10/-0

Provide appliedFilters chips for Therapy Compass shelf

• Builds an appliedFilters array from active topics and availability toggles and passes it to SearchResultsHeaderBand, along with a clear-all handler.

src/components/therapy-compass/screens/search-screen.tsx

Refactor (10) +16 / -240
ClinicalDashboard.tsxRemove commandScopes state from ClinicalDashboard context and mode changes +1/-14

Remove commandScopes state from ClinicalDashboard context and mode changes

• Deletes commandScopes state and clear/remove callbacks from the dashboard and from SearchCommandContext value construction. Also removes mode-change resets that previously (and ineffectively) cleared scopes.

src/components/ClinicalDashboard.tsx

favourites-command-library-page.tsxRemove command-scope filtering paths from favourites mode +6/-19

Remove command-scope filtering paths from favourites mode

• Eliminates useSearchCommand and favouriteMatchesCommandScopes usage, removing scope-driven filtering and empty-state 'clear scopes' behavior. Match counts and empty states now reflect the actual filteredItems only.

src/components/clinical-dashboard/favourites-command-library-page.tsx

global-search-shell.tsxRemove commandScopes state from global search shell +1/-15

Remove commandScopes state from global search shell

• Deletes commandScopes state and callbacks and removes scope resets on mode changes. SearchCommandContext now provides only query and modeId, and scope props are no longer passed to the command surface.

src/components/clinical-dashboard/global-search-shell.tsx

master-search-header.tsxDrop commandScopes props from shared master search header wiring +0/-6

Drop commandScopes props from shared master search header wiring

• Removes commandScopes and onCommandScopesChange from the header’s props and stops passing them into UniversalSearchCommandSurface. This aligns the header API with the now-simplified command surface and context.

src/components/clinical-dashboard/master-search-header.tsx

medication-prescribing-workspace.tsxRemove command-scope filtering from prescribing results +2/-8

Remove command-scope filtering from prescribing results

• Eliminates useSearchCommand and medicationMatchesCommandScopes usage, making filter counts and displayed rows reflect the underlying search results only.

src/components/clinical-dashboard/medication-prescribing-workspace.tsx

search-command-context.tsxSimplify SearchCommandContextValue to query + modeId only +0/-3

Simplify SearchCommandContextValue to query + modeId only

• Removes commandScopes and scope mutation callbacks from the context type, reflecting the deletion of the entire scope system.

src/components/clinical-dashboard/search-command-context.tsx

universal-search-command-surface.tsxRemove unused commandScopes props from command surface +0/-6

Remove unused commandScopes props from command surface

• Deletes commandScopes and onCommandScopesChange props (and the prior 'void' discards), making the component’s interface match real behavior.

src/components/clinical-dashboard/universal-search-command-surface.tsx

forms-search-results-page.tsxRemove command-scope filtering from forms search results +2/-11

Remove command-scope filtering from forms search results

• Eliminates useSearchCommand and recordMatchesCommandScopes usage, and updates sorting to operate directly on matches. Removes empty-state 'clear scopes' hook.

src/components/forms/forms-search-results-page.tsx

services-navigator-page.tsxRemove command-scope filtering from services navigator results +4/-13

Remove command-scope filtering from services navigator results

• Deletes useSearchCommand and recordMatchesCommandScopes usage. Displayed matches, rank map, and empty state now use the unscoped matches set.

src/components/services/services-navigator-page.tsx

search-command-surface.tsDelete scope chip config and all scope matcher helpers +0/-145

Delete scope chip config and all scope matcher helpers

• Removes CommandScopeChip and per-mode scopes configuration, along with record/favourite/medication scope matching functions and helpers. Leaves suggestions/examples and cross-mode config intact.

src/lib/search-command-surface.ts

Tests (4) +110 / -146
document-filter-panel.dom.test.tsxAdd DOM tests for applied-filter shelf behavior in documents +66/-0

Add DOM tests for applied-filter shelf behavior in documents

• Introduces tests asserting shelf appearance, one-tap chip removal, Clear behavior with multiple chips (facet + source type), and shelf persistence through loading vs removal on fault.

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

search-command-surface.test.tsRemove tests for deleted scope matchers and other unrelated assertions +0/-90

Remove tests for deleted scope matchers and other unrelated assertions

• Deletes unit tests that exercised record/favourite/medication scope matchers and removes assertions that depended on reading universal-search-command-surface source. Retains command dropdown environment checks and suggestion filtering tests.

tests/search-command-surface.test.ts

search-results-header-band.dom.test.tsxUpdate header band tests for removed scopes and new mobile layout +10/-34

Update header band tests for removed scopes and new mobile layout

• Removes scope-related utility strip tests and updates wiring tests to only cover sort/view/save controls. Adjusts the mobile control test to validate Sort in utilities and page filter controls rendered last at the right edge.

tests/search-results-header-band.dom.test.tsx

ui-tools.spec.tsUpdate Playwright layout assertions for Sort/Filter separation on mobile +34/-22

Update Playwright layout assertions for Sort/Filter separation on mobile

• Replaces the prior 'mobile control pair' assertions with checks that the page filter group is the last child in the utilities rail and that Sort and Filter maintain matched tap height.

tests/ui-tools.spec.ts

Documentation (3) +112 / -1
branch-review-ledger.mdRecord PR handoff and verification status in ledger +1/-0

Record PR handoff and verification status in ledger

• Adds a new ledger row capturing the handoff summary, scope deletion rationale, shelf rebuild, bar anatomy changes, and verification results for this branch/PR.

docs/branch-review-ledger.md

handoff-search-bar.mdAdd detailed handoff doc for 'Results bar — perfected' changes +110/-0

Add detailed handoff doc for 'Results bar — perfected' changes

• Introduces a handoff document describing which study steps were implemented, which were intentionally declined, test gate status, and scoping decisions (notably shelf limited to documents and therapy-compass).

docs/handoff-search-bar.md

outstanding-issues.mdArchive issue #182 as deleted and summarize outcome +1/-1

Archive issue #182 as deleted and summarize outcome

• Moves the command-scope system issue into the resolved section with a deletion summary and verification notes, reflecting the completed removal across the codebase.

docs/outstanding-issues.md

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Clear wipes query too 🐞 Bug ≡ Correctness
Description
Therapy Compass passes clearSearch to SearchResultsHeaderBand.onClearFilters, but clearSearch
resets EMPTY_SEARCH including query: "". This makes the shelf’s trailing Clear unexpectedly
delete the user’s search term even though the shelf explicitly models only non-query filters.
Code

src/components/therapy-compass/screens/search-screen.tsx[R53-55]

+        appliedFilters={appliedFilters}
+        onClearFilters={b.clearSearch}
        filterLabel="Filter therapy results"
Evidence
The shelf Clear callback is b.clearSearch, and clearSearch resets EMPTY_SEARCH, whose query
is ""; meanwhile the shelf’s applied filters intentionally exclude the query, and the band’s prop
contract describes clearing filters (not query).

src/components/therapy-compass/screens/search-screen.tsx[24-55]
src/components/therapy-compass/bindings.tsx[390-409]
src/components/therapy-compass/data/select.ts[57-71]
src/components/clinical-dashboard/search-results-header-band.tsx[173-179]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Therapy Compass’s applied-filter shelf Clear button is wired to `b.clearSearch`, which resets the whole search state including `query`. The shelf intentionally excludes the query from `appliedFilters`, so Clear should clear only the applied filters represented by the shelf (tags + availability toggles), or the UI/contract should be updated to make “Clear” explicitly mean “clear the entire search including query”.

### Issue Context
- `SearchResultsHeaderBand` documents `onClearFilters` as clearing applied filters.
- Therapy Compass’s `clearSearch` resets `EMPTY_SEARCH` where `query` is empty.

### Fix Focus Areas
- src/components/therapy-compass/screens/search-screen.tsx[32-55]
- src/components/therapy-compass/bindings.tsx[390-409]
- src/components/therapy-compass/data/select.ts[57-71]
- src/components/clinical-dashboard/search-results-header-band.tsx[173-179]

### Suggested fix
Implement a filter-only clear handler (e.g., `clearFiltersOnly`) that preserves the current query while resetting `tags`, `reviewedOnly`, and `briefOnly`, and pass that to `onClearFilters` instead of `b.clearSearch`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Memo deps lint suppressed 🐞 Bug ⚙ Maintainability
Description
DocumentSearchResultsPanelImpl suppresses react-hooks/exhaustive-deps for the appliedFilters
memo while capturing toggleTagFacet, which is declared inline (identity changes every render).
This under-specified dependency list makes the chip callbacks fragile for future refactors and
contradicts the in-code comment claiming a “stable closure”.
Code

src/components/clinical-dashboard/document-search-results.tsx[R1162-1187]

+  const appliedFilters = useMemo(() => {
+    const selected = new Set(activeFacetKeys);
+    const chips = tagFacetGroups.flatMap((group) =>
+      group.facets
+        .filter((facet) => selected.has(facet.key))
+        .map((facet) => ({
+          id: facet.key,
+          label: facet.label,
+          onRemove: () => toggleTagFacet(facet),
+        })),
+    );
+    if (effectiveResultType !== "all") {
+      const tab = resultTabs.find((entry) => entry.key === effectiveResultType);
+      if (tab) {
+        chips.push({
+          id: `result-type-${tab.key}`,
+          label: tab.label,
+          onRemove: () => setActiveResultType("all"),
+        });
+      }
+    }
+    return chips;
+    // `toggleTagFacet` is a stable closure over `query`, which is already a
+    // dependency of `activeFacetKeys`.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [tagFacetGroups, activeFacetKeys, effectiveResultType, resultTabs]);
Evidence
The chip onRemove closures call toggleTagFacet, but the memo dependency list excludes it and the
lint rule is explicitly disabled; toggleTagFacet is defined as a normal function inside the
component (not useCallback).

src/components/clinical-dashboard/document-search-results.tsx[1113-1121]
src/components/clinical-dashboard/document-search-results.tsx[1159-1187]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `appliedFilters` `useMemo` intentionally omits `toggleTagFacet` (and suppresses the lint rule), but `toggleTagFacet` is an inline function recreated each render. This increases the risk of subtle bugs if `toggleTagFacet` later starts capturing additional mutable values or its implementation changes.

### Issue Context
Current behavior likely works because `activeFacetKeys/tagFacetGroups` change on the meaningful transitions, but the lint suppression makes this easy to regress.

### Fix Focus Areas
- src/components/clinical-dashboard/document-search-results.tsx[1113-1121]
- src/components/clinical-dashboard/document-search-results.tsx[1159-1187]

### Suggested fix
Either:
1) Wrap `toggleTagFacet` in `useCallback` with correct deps and include it in the `useMemo` dependency array, and remove the eslint disable; or
2) Avoid memoizing `appliedFilters` and just compute it inline (if perf impact is negligible), removing the lint suppression.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/components/therapy-compass/screens/search-screen.tsx
Comment thread src/components/clinical-dashboard/document-search-results.tsx
@BigSimmo
BigSimmo enabled auto-merge (squash) August 1, 2026 02:15
@BigSimmo

BigSimmo commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent fox comments and CI failing

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues.

Please try again later or Open in Web to view the status.

Open in Web Open in Cursor 

Learn more about Cursor Agents

@BigSimmo
BigSimmo merged commit b1e0117 into main Aug 1, 2026
38 checks passed
@BigSimmo
BigSimmo deleted the claude/top-search-design-mockups-w53znc branch August 1, 2026 02:25
cursor Bot 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>
BigSimmo added a commit that referenced this pull request Aug 5, 2026
* fix(ui): make the applied-filter shelf reachable and its dead ends readable

Four review findings land on one row and interact.

F4 — the shelf was a single `overflow-x-auto` row: label, chips, a `flex-1`
spacer, then `Clear`. With four or five chips on a phone the spacer collapses
and `Clear` sits past the right edge, reachable only by swiping a row whose
scrollbar is hidden. That is the same defect the shelf was built to avoid for
the chips themselves. The chips now scroll in an inner track and `Clear` is
pinned outside it.

F5 — the rail fades its edge on overflow and the shelf did not, so a sixth chip
simply stopped existing visually. The chip track now carries the same mask on
the same overflow condition, via a second instance of the existing
`useRailOverflow` hook.

F3 — `Clear` was `px-2 py-1`, about 26px beside 48px chips: the row's only
global action was also its hardest target. Matched to the chips instead, staying
quiet through weight and an underline. 48px rather than the 44px generic tap
guidance suggests, because `min-h-11` reintroduces a fixed `ui-smoke` sub-pixel
flake and `--spacing-tap` is this repo's floor.

F10 — zero-count facets in the documents sheet were dimmed with `opacity-50`,
which multiplies against an already-muted foreground and lands at 2.34:1. The
disabled state was least readable exactly when it most needed explaining.
Replaced with a real muted pair plus a dashed border: 4.72:1, and it survives
forced colors, where border-style is preserved and opacity is not. The three
facet states are now mutually exclusive branches rather than a base plus an
override — `cn` is a plain join, so competing `border-[color:…]` utilities would
both reach the DOM and stylesheet order, not intent, would pick the winner.

Also rewires the therapy-compass quick-filter row's `Clear` to
`clearSearchFilters`. It sits among the filter chips and is labelled `Clear`,
but it called `clearSearch`, which wipes the query with them — the sibling of
the defect #1611 fixed on the shelf, missed when that was reviewed. The sheet's
`Clear all` is deliberately a full reset and is untouched.

Shelf label becomes a funnel glyph below `sm` and the wordmark from `sm`: a
prefixed chip costs ~215px of a 350px bar, so every character the label spends
is a chip the reader cannot see. The group keeps `aria-label="Applied filters"`
either way, so both forms are decorative.

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

* feat(ui): give the filtered-to-zero empty state its relax-the-filter route back

F11. The release that made filters real (#1555) is also the one that removed the
only copy pointing at relaxing them. The old empty state read "Relax the scope,
try an example, or jump to another mode" with a `Clear scope filters (N)`
button; both were dead — they keyed off the inert scope system and never
rendered — so the change deleted them and left "Try an example, or jump to
another mode." A reader who has filtered to zero is offered an unrelated example
query and a different mode, never the chips sitting directly above that caused
it.

`SearchResultsEmptyState` now takes the same `appliedFilters` the shelf renders,
plus `onClearFilters` and `onBrowseAll`. When the set is non-empty it leads with
`Remove "X"` and `Clear all filters`, and demotes the example and cross-mode
routes below a rule — an example query is a different search, and the reader has
not finished this one. With nothing applied the current copy is already correct
and is untouched.

Two things it deliberately does not claim. The heading counts the filters rather
than quoting the query, because the query is not what emptied the set and saying
so sends the reader to rewrite the one thing that was working. And `Remove "X"`
names the last chip without calling it the most recent: `appliedFilters` arrives
in group order, not application order, so that would be a claim the data cannot
support.

Documents and therapy-compass rendered their own bare `EmptyState` for this
case — naming the problem while offering no route out of it — so both now use
the shared surface. Documents also passes Browse, because when narrowing this
result set is not the answer, reaching the whole corpus is.

This retires the last `clearSearch` mislabel on therapy-compass. Its empty state
had one button labelled `Clear filters` wired to `clearSearch`, which wiped the
query too; `Remove "X"` and `Clear all filters` are now separate controls, so
each label matches its own action. The guard added in the previous commit
tightens to assert exactly one full reset survives that screen — the sheet's
`Clear all`, the only control whose label says it clears everything.

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

* feat(ui): rebuild the documents filter sheet around eleven groups

F7 — facet buttons were `min-h-7`: 28px targets, packed at `gap-1.5`, on the
surface that exists for phones and whose only interactive elements they are.
Raised to the tap token, relaxing to 36px from `sm` and 32px from `lg` where a
pointer is likely. 48px rather than the 44px the design called for, because
`min-h-11` reintroduces a fixed `ui-smoke` sub-pixel flake and `--spacing-tap`
is this repo's floor; the sheet has the vertical room.

F8 — source type is a `radiogroup` and the facets below are `aria-pressed`
toggles, but both rendered as chips of near-identical size, colour and radius,
directly adjacent. Nothing said one row replaces and the next accumulates, so
the OR-within-group, AND-across-groups model had to be found by experiment.
Source type is now a joined segmented control, which reads as one-of on sight,
with a `one only` hint for the first time it is seen. The ARIA is unchanged —
this is presentation.

F9 — eleven groups stacked in one phone column with no collapsing and no search,
so reaching Document type meant scrolling past ten sections. Adds a
find-a-filter field and collapses groups by default, each carrying its selected
count. Both are gated on the same density threshold: a sheet showing two groups
that are both shut is a scroll saved that never existed and two taps added that
did, so below four groups everything stays open and the heading is not a
disclosure control at all. A group holding a selection cannot be collapsed —
a closed section silently narrowing the list is worse than the scroll it saves.

F12 — Library leaves the utility rail. It sat adjacent to Filter while answering
a different question, and it occupied the rail space the pinned Filter needs; it
is also the reason the phone rail could overflow at all. It is moved, not
removed: the requirement the old comment protected still holds, since the
documents action menu routes through `onSearchModeChange`, which clears the
query. Both new homes preserve it — the sheet footer under a rule, phrased as
reach with the corpus count beside it, and the zero-result state.

Also drops the footer's duplicate count. It printed "12 documents" beside "Show
12 documents", spending the sheet's most prominent slot on a number two
centimetres to its left; the button carries it, the new meter and readout carry
the proportion, and the live announcement moves to `sr-only` so it still speaks
as the number changes underneath. The header's `Clear all` becomes `Clear
filters`, matching a handler that was already filter-only.

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

* feat(ui): collapse the results bar to one line and pin what must not scroll away

The band was 123px on a phone to say "12 documents". The utility rail dropped to
its own row and that row was ~85% empty, so the height was spent on the layout
rather than on anything the reader needed. It is now 58px, 60px from `sm`.

F1 — the rationale for moving Filter last was that the right edge is where a
thumb already rests, but the spacer that pushed the rail right was
`hidden lg:block lg:flex-1`, so it did nothing below 1024px. On every phone the
rail was left-packed and Filter sat mid-rail: the change delivered its stated
benefit only at the width where thumbs are least relevant. The query now takes
the flexible space and the control group is edge-aligned at every width, with no
conditional spacer. `mr-auto` does it rather than moving `lg:flex-1` down —
auto margins only absorb what is left after flexible lengths resolve, so the
wide layout is byte-identical and no breakpoint had to move.

F2 — the rail was one `overflow-x-auto` region with Filter as its last child, so
the only control carrying filter state was the first to fall off the right edge
once a Retry button or a longer sort label appeared. The code's own comment
explained that applied-filter chips had been moved out of that rail for exactly
this reason. Only the optional controls scroll now; Filter and Retry are pinned
siblings outside the track. Retry too, because it is the recovery action in a
degraded state — the one control that must never need a horizontal swipe.

The state tile is deleted and the full-width accent border becomes a 2 x 18px
lead rule inside the padding: at bar height a line across the whole width read as
a divider between the composer and the results rather than as the band's accent.

That deletion is where the design as drawn had to be extended. The tile carried
state as SHAPE — alert when faulted, spinner while running, funnel once filtered
— and the mockup replaced it with colour alone, which makes a failed search
identical to a successful one for a reader who cannot separate the hues, and
contradicts a recorded decision. The tile turned out to carry three jobs and only
one needed a tile. Narrowed is now carried by the shelf, which grows the band by
a whole labelled row. Running was already inline. Faulted keeps three
independent non-chromatic channels: the lead rule doubles from one stroke to
two, a CircleAlert renders before the count for non-ready states only, and a
faulted band still renders no digit at all. The mark is a `border-left` on a
zero-width box rather than a background precisely so that forced colors, which
drops backgrounds but maps border colour to CanvasText and preserves
border-style, keeps all of it.

F12 — Library leaves the rail (moved in the previous commit), which is what makes
the arithmetic work: at 390px count + query + Library + Filter does not fit, and
without Library it fits comfortably.

One line is not safe for every mode, and the mockup was drawn for documents. Six
modes pass `MobileResultFilterControl` into `mobileControls` — a `w-full` native
select, and formulation and specifiers pass two in a two-column grid — which is
unreadable pinned into a 58px line at 320px. `mobileControlsPlacement` therefore
defaults to `row` whenever a page passes a phone control and to `inline` when it
passes none; documents and therapy-compass opt in explicitly. The default is the
safe one so a new mode that forgets the prop degrades to today's layout rather
than to an unusable one. Verified at 320/390/430/768/1024/1440 in light, dark and
forced-colors with no page horizontal overflow at any of them.

`tests/ui-accessibility.spec.ts` is rewritten, not deleted: it caught the accent
degrading to a neutral border when Tailwind's utilities layer outranked the
component layer, which has actually happened here, and the same failure now shows
up as a zero-width box. It gains the fault assertion the border-top version could
not make — a border has no style to change — and a guard that its own probe
returned real values, because a silently-null measurement reads as a pass. The
style-effect contract in `tests/helpers/style-contracts.ts` is retargeted from
`search-band` to `search-band-lead` for the same reason.

`docs/search-results-bar-decisions.md` is amended in this commit, since two of
its records go stale here: the state tile, and Library staying in the bar.

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

* docs(ui): correct two comments the redesign left describing the old band

`search-band` no longer paints the accent, and the placement note referenced a
variable name that never shipped. Also drops the claim that `refetching` is
dimmed "via CSS `data-status`": no such rule exists in globals.css and there is
no evidence one ever did, so the sentence described an intention rather than the
code. Adding the rule is a visual change across twelve modes and belongs to
whoever decides a background refresh should look different — asserting it here
while it does not exist is what let it go unnoticed.

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

* docs(issues): capture the two follow-ups the results-bar redesign surfaced

#237 — the band claimed a `refetching` dimming rule that globals.css has never
defined, so a background refresh is signalled by a pulsing dot alone against
text that is deliberately identical to `ready`. Corrected the comment in the
redesign rather than adding the rule, because adding it is a visual change
across twelve modes.

#238 — the one-line bar currently reaches two modes plus every mode that passes
no phone control. Six pass a `w-full` native select (two of them pass a pair),
which is unreadable pinned into a 58px line at 320px, so they keep their own
row by default. Widening them is per-mode work, not a shared-band change.

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

* fix(ui): reach the corpus from the sheet, and re-point the test that pinned it

Fallout from moving Library off the utility rail, caught by `verify:ui`
(`348 passed, 1 failed`) rather than by anything offline.

`ui-smoke.spec.ts` asserted the ribbon still carried an "Open source library"
button, then used that same button further down to open the Sources drawer and
check focus returned to it on Escape. Both are re-pointed at Library's new home
in the filter sheet's footer rather than deleted — the ribbon assertion is
inverted into an absence, so putting Library back on the rail re-creates the
overflow F12 removed and fails loudly instead of passing quietly.

The behaviour fix the test exposed: the footer control called `onOpenLibrary`
without dismissing the sheet, so the Sources drawer opened underneath a filter
panel still covering the results both of them describe. Browsing the corpus is
leaving the filter surface, not another thing to do on it, so it now closes the
sheet on the way out.

That in turn moves where focus lands when the drawer closes: the opener has
unmounted with the sheet, so the app falls back to the documents options button.
Asserted explicitly, with the reasoning, rather than loosened to "not body" —
it is a visible related control in the same workspace and the app's existing
restore target, but it is a fallback and the comment says so.

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

* docs(ledger): record the results-bar redesign review at 7eb723b

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

* fix(ui): keep document filter sheet chrome scoped to the active query

Reset the find-a-filter needle and expand set when the search query changes,
and keep selected facets visible while the find field narrows the list so an
active constraint cannot become unreachable inside the sheet.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

* fix: restore empty-state recovery and rail observation

* fix: restore therapy zero-result recovery

* fix: keep document filter recovery paths reachable

* test: cover announced empty-state recovery

* test: cover document filter recovery paths

* test: intercept the differential search endpoint

* fix: close filter disclosure click handler

* fix: use a valid exact search route regex

* style: format empty-state regression test

* style: format document-search-results.tsx

* fix(ui): observe overflow without effect state writes

* chore(ui): remove obsolete empty-state import

* fix(ui): stop the empty state claiming a second status region

The auto-fix for Devin's announcement finding gave `SearchResultsEmptyState` a
`role="status"` root. The band already renders one unconditionally on every
search route, so `getByRole("status")` became ambiguous everywhere the empty
state can appear — which is every mode. Devin's own prompt flagged the risk
("checking it does not collide with the band's own single-status-region
assertions"); the collision landed anyway and broke three suites.

A bare `aria-live="polite"` announces identically — `role="status"` is just
implicit polite + atomic — without adding a node to the role query. `aria-atomic`
is deliberately left off: heading and body change together, and re-reading the
whole panel on every keystroke is worse than reading what changed.

Also repairs three tests that were asserting the wrong things:

- The therapy guard counted `b.clearSearch` call sites and required exactly one.
  An agent then added a correctly-labelled `Clear search` recovery, and the count
  read it as a regression. The rule was never about head-count — it is that a
  control wired to `clearSearch` must be *labelled* for clearing the search.
  Rewritten to assert that, so `onClear`/`onClearSearch` pass and an
  `onClearFilters` or bare `onClick` still fails. A duplicate copy of the same
  test carrying the old assertion is removed.

- `states the proportion once…` expected `Show 0 documents` from selecting
  Clozapine + Suicide. The panel deliberately prevents that: once Clozapine
  narrows the set, Suicide re-counts to 0, becomes a dead end, and dead ends are
  click-guarded. The test read the feature as a bug. It now asserts the guard —
  `aria-disabled`, the description, and that the guarded click leaves the count
  alone.

- Two `getByRole("status")` queries in the empty-state tests are now by text,
  matching the role change above.

Two failures in `document-search-record-fault.dom.test.tsx` are untouched here
and are not from this commit — verified failing with these changes stashed.

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

* test: follow the zero-match state onto the shared empty state

`document-search-record-fault.dom.test.tsx` still expected "No matching
documents". That branch adopted `SearchResultsEmptyState` so Library stays
reachable from a search that returned nothing — the gap Codex and Devin both
raised — which changed the heading to the shared "No matches for <query>".

Re-pointed rather than loosened: the assertions still name the exact copy and
still distinguish the loading state from the settled one, so a state that stops
rendering its heading fails here the way it did before.

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

* fix(ui): keep the documents zero-result title a heading

Adopting the shared empty state demoted the main document-search
zero-result title from `h3` back to a paragraph, silently undoing #1612 —
the release that gave `EmptyState` an optional heading level precisely so
the two states owning their region could keep one. Nothing offline
noticed; the only signal was `ui-smoke`'s `@critical` journey going red on
`getByRole("heading", { name: "No matching documents" })`.

`SearchResultsEmptyState` now takes the same opt-in, un-defaulted
`headingLevel` as `EmptyState`, for the same reason: most of the twelve
modes rendering it sit inside a region whose heading the band already
owns, so promoting every title would insert an outline level the page
never declared. Documents passes `3` at the call site that owns its
region; the inline filtered-to-zero state inside the results grid stays a
paragraph.

The Playwright assertion follows the new copy but keeps asserting the
role and level, and `document-search-record-fault` now pins the same
level so the fast gate catches this next time instead of a Chromium
journey.

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

* fix(test): restore the differentials fault interception, and match empty-state copy to its controls

Three review findings, one of which was a live CI failure caused by acting on
a fourth without running it.

`ui-accessibility`'s two fault tests were switched from intercepting
`/api/differentials` to `/api/search` on review advice. That page's catalog
hook fetches `/api/differentials?kind=diagnosis|presentation` and never
touches `/api/search`, so the interception faulted nothing, the band stayed
healthy, and both tests hung waiting for a fault panel that could not render
— two red `Production UI` shards. Restored, with the endpoint named in a
comment so the swap is not made again.

The empty state's body told every reader to "try an example, or jump to
another mode". `searchCommandSurfaceByMode` is a `Partial<Record<…>>` with no
therapy-compass entry, so on that mode the panel renders neither control and
the copy named two affordances that were not there. The body is now derived
from what the panel actually offers.

Also drops a hardcoded neutral colour literal from the lead-rule assertion —
the line above it already compares against the probed neutral — and scopes the
style-effect contract's selector to `[data-tone="accent"]`, so a degraded
live result cannot fail a contract that exists to catch cascade regressions.

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

* fix(ui): move the new search-surface text nodes off the deprecated decoration tier

Readying this change for the v2 design system (#1616), which activates the
canonical token layer across 224 files and touches four of the same files.

`--text-soft` is a deprecated alias of `--decoration-soft`, and
`ckb-v2-token-contract` pins it *below* 4.5:1 deliberately, "so the tier cannot
be fixed away". TOKENS.md §7 forbids it on any text node. This branch added
eight new text-node usages, which under the v1 palette measure a comfortable
4.72:1 and look correct — and under the v2 palette measure 2.99:1.

The dead-end facet is the one that matters. Its entire reason to exist is that
the previous `opacity-50` treatment measured 2.34:1; putting the replacement on
the decoration tier meant the fix held only until the v2 layer activated, at
which point the same markup would have been worse than the AA floor again with
nothing reporting it.

  dead-end facet copy on --surface-subtle
    opacity-50, before this branch      2.34:1
    --text-soft   v1 / v2         4.72:1 / 2.99:1
    --text-muted  v1 / v2         7.29:1 / 5.99:1   (7.54:1 v2 dark)

Counts, the shelf label, the shelf Clear and the disabled-facet copy move to
`--text-muted`; the find-a-filter placeholder moves to `--text-placeholder`,
which is the role the existing recipe contract already requires. Icons and
glyphs keep `--text-soft` — that is what the tier is for, and v1 defines no
`--decoration-soft` to move them to.

Two of these lines are the exact lines #1616 migrates, so both sides now make
the same change and those hunks merge instead of conflicting.

`decoration-on-text.contract.test.ts` gains four cases pinning the tier per
surface, verified to fail on the regression and pass when restored. A DOM
assertion cannot do this job: jsdom sees the class, not the resolved colour, so
it would pass on either token.

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

* fix(ui): move the therapy quick-filter Clear label off the decoration tier

The last text node on this branch still using `--text-soft`. It is a button
label, so under the v2 palette it measures 2.99:1; `--text-muted` gives 5.99:1
light and 7.54:1 dark. Found by trial-merging the v2 branch rather than by
reading the diff — the conflict hunk showed v2 migrating this exact line while
this branch kept the old tier.

Matching v2's token choice also shrinks that conflict to the handler alone,
which is the part the two branches genuinely disagree about
(`clearSearchFilters` here versus `clearSearch` on v2's base).

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

* fix(ui): restore Library on the record-match path, and stop the phantom collapse

Two defects raised in review, both verified against the code and both introduced
by this branch. Neither was covered — the existing suites passed before these
fixes as well as after, which is why each gets a guard proven by inversion.

**Library was unreachable from a services or forms search that matched records
but no documents.** Moving Library off the utility rail left it three homes: the
sheet footer, the zero-result empty state, and the inline fallback. The footer
needs `matches.length > 0`; the empty state needs `recordMatchCount === 0`. The
record-match render satisfies neither and returned `null` outright, so on that
path the only route to the corpus was the documents action menu — which calls
`setQuery("")` and destroys the search being read. That is the precise thing
`docs/search-results-bar-decisions.md` requires an in-context route for. The
control is now a shared const rendered from both paths, so a fourth branch
cannot be added without one.

**A group heading reported a collapse it did not perform.** `isOpen` is forced
true while the find-a-filter field has text, but the disclosure button stayed
mounted and its handler still branched on that forced-true value: tapping it
left `aria-expanded="true"`, rotated no chevron, hid nothing — and wrote the
group into `collapsed`, so the collapse ambushed the reader later, once the
field was cleared and the tap forgotten. While searching, the needle owns what
is open, so the heading now renders in its static form. This is the rule the
comment three lines above it already stated; the needle case was simply missed.

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

* fix(ui): stabilize results-band review findings on #1615

Keep phone-control row geometry across loading, stop wide selects sharing a
shrinkable flex line with Sort, announce empty states without double-speaking
filtered zeros, and clear the stale docs/comments the redesign left behind.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

* fix(ui): keep the deferred empty-state announce out of the effect body

`788b664` added a deferred screen-reader announcement for the query-only empty
state — a live region that mounts already populated is silent in most screen
readers, so the message has to arrive on a later frame. The mechanism is right;
the implementation called `setLiveMessage("")` synchronously in the effect body
twice, which `react-hooks/set-state-in-effect` blocks. `Static PR checks` went
red on that lint error, and nothing offline caught it: lint is in neither the
unit suite nor `tsc`.

Both cleared calls were dead anyway. The region renders only when `!filtered`,
so the filtered branch had nothing mounted to clear, and that same gate unmounts
and remounts the region across the transition, so it starts empty on its own.
Only the `requestAnimationFrame` callback now sets state, which is the form the
rule allows and the one the deferral needs.

CodeRabbit reached the same fix independently on the same head.

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Sign up for free to 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.

2 participants