Skip to content

feat(search band): count names what it counted, filters get their own shelf - #1523

Merged
BigSimmo merged 3 commits into
mainfrom
claude/top-search-design-mockups-w53znc
Jul 31, 2026
Merged

feat(search band): count names what it counted, filters get their own shelf#1523
BigSimmo merged 3 commits into
mainfrom
claude/top-search-design-mockups-w53znc

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

Two commits. The second changes shipped UI, so this PR is no longer design-scratch only — the scope statement below supersedes the original mockup-only framing.

5b5fe31 — shared results band (shipped, all 12 search pages). Applies the agreed design decisions to SearchResultsHeaderBand, so every search mode picks them up at once.

  • The count leads and names its subject.12 matches matched what? Once the query stopped being the band's heading nothing on screen answered that, and appModeSearchConfig(modeId).resultHeading already carries the noun — so it now reads 12 services, 1 DSM diagnosis, 3 therapies. inlineNoun keeps the acronym in DSM diagnoses intact rather than lower-casing it; singularNoun covers the three shapes the registry actually uses (iesy, sessis, else drop s).
  • The query moves after the count, at reading weight, as its anchor. The composer above still shows it, but phone chrome scroll-hides and this band pins, so while you read results nothing otherwise says what was asked. It is always rendered rather than appearing on scroll, and it truncates before the number does. The heading element and its accessible name are unchanged — only position and weight — so anything resolving the query by role is unaffected.
  • Applied scope chips move out of the utilities rail onto their own labelled shelf. They are state, not tools: sharing a horizontally-scrolling rail with sort and view mixed the two and let a chip be pushed off the right edge, losing the only affordance for removing it. The shelf carries a Filtered by label so a row of accent pills reads as editable state rather than a second bank of buttons, and a Clear that undoes all of them — previously one tap per filter with no shortcut.
  • The shelf survives loading, unlike the page filter row. Those chips carry no counts, and dropping them mid-search made the shelf flicker out and back on every keystroke. Only a faulted search drops them, because filtering a result set that never loaded is meaningless.
  • The state tile gains a funnel once the result set is narrowed, so a filtered list looks different from an unfiltered one before any text is read.

f6f2d61/mockups/search-band-directions (design scratch; 404s in production, outside the wiring and reachability gates).

Verification

  • npm run verify:cheap — exit 0. Test Files 445 passed (445), Tests 4662 passed | 4 skipped (4666), Design-system contract passed (614 production files; raw colors 2; literal shadows 0; legacy tap classes 0).
  • npm run typecheck, npm run lint — clean. npm run format run and committed.
  • tests/search-results-header-band.dom.test.tsx — 25 passed. Count assertions moved to the new copy; the ordering test rewritten to pin count-before-query via compareDocumentPosition rather than the reverse; three cases added for the singular, the acronym, and the irregular plural.
  • Driven in the real app at /services?q=crisis after npm run ensure: HTTP 200, no console errors. Desktop renders Searching… · crisis mid-flight; phone renders 45 services · crisis with the controls below. Screenshots taken at both widths.

UI verification not run: npm run verify:ui. This container has Chromium 1194 against the project's pinned build, so a Playwright run here would not be evidence about the pinned browser. This diff changes the DOM of a band on 12 production routes, so the Chromium gate is genuinely required before merge — please run it, or tell me and I will. The specs that bind this surface are ui-smoke, ui-tools, ui-specifiers, ui-formulation, ui-accessibility, ui-route-coverage.

Risk and rollout

  • Risk: medium. The clinical invariant is untouched — a faulted search still asserts no number, still renders exactly one role="status" and one role="alert", and the fault panel is unchanged. What changes is copy (N matchesN <noun>), the DOM order of the count and the query heading, and the position of scope chips. Playwright specs that locate the count by its old text, or assert chip position within search-query-ribbon-utilities, will need updating; I could not run them here.
  • Rollback:git revert 5b5fe312 restores the previous band exactly. The mockup commit is independent.
  • Provider or production effects: None. No provider-backed command was run.

Clinical Governance Preflight

  • 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

The band is a presentation surface over an existing result set. It performs no retrieval, ranking, or source selection, and this diff changes no data path — only what the band says about a count it is already given. The conservative-failure behaviour it was built for is unchanged and covered by the DOM suite.

Notes

Deliberately not in this PR, and both were requested:

  1. The phone filter slide-up. The repo already has the right primitive (src/components/ui/sheet.tsx — bottom sheet on mobile via the sheet-up keyframe, safe-area aware, focus-trapped, Escape and backdrop dismiss), so this should use it rather than anything new. But it replaces the inline mobileControls that at least four Playwright specs drive directly, and I cannot run the Chromium gate faithfully here. Shipping that blind to 12 clinical routes was the wrong trade.
  2. Merging the four filtering surfaces into one panel. Documents mode currently has smart-tag facets (11 groups, AND-combined, document-tags.ts), a separate source-type control that duplicates the existing Document type facet group, scope chips, and the Sources library button. Collapsing those is a behaviour change across pages and deserves its own PR and its own reasoning.

Also worth recording: facet counts are built from matches while result-type counts are built from visibleMatches, so once you select a facet the rest of the panel shows stale numbers. Not fixed here — it belongs with (2).

@coderabbitai

coderabbitaiBot commented Jul 31, 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:16 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: 591fb7cc-7b91-4642-a777-a1a6475c15be

📥 Commits

Reviewing files that changed from the base of the PR and between f6f4833 and 0cd18b3.

📒 Files selected for processing (7)
  • docs/site-map.md
  • src/app/globals.css
  • src/app/mockups/mockups-layout-client.tsx
  • src/app/mockups/search-band-directions/page.tsx
  • src/components/clinical-dashboard/search-results-header-band.tsx
  • src/components/search-band-directions-mockups.tsx
  • tests/search-results-header-band.dom.test.tsx

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

Round three ("Stacked, refined") asked how to make the band's second row earn
its place. That question accepted the row, and behind it the card. Since the
"across 4 documents · 2 images" clause was removed the band has been a
container looking for contents, so both directions here delete the container
and keep exactly one assertion.
Both put the count first, name the subject it counted (the mode config already
carries the noun, and removing the query from the header left "6 matches"
matching nothing you can name), and never grow a second row. They differ on one
axis: whether sort/view/save stay visible while you read.
- Ledger: one line, always. Filters sit on the count's baseline as removable
tokens, because a filter modifies the count. Tools stay at the far edge, one
size band down. Measured 54 px in all twelve state/filter combinations, so it
pins when scrolled with nothing to collapse.
- Statement: the band is one sentence and every tool folds into a single
control that displays its own value. Success and failure share a shape, and
empty reads as prose rather than a zero. Measured 54–83 px on a phone: a
29 px reflow, which is the argument against it.
Heights are measured off this build at the shipped band's own tap sizes
(min-h-tap below sm, min-h-10 above), not at the tidier 34 px a mockup would
naturally draw — that difference is worth about 4 px of band height and 13 px
of Statement's reflow, and it flatters every direction.
Design scratch only: /mockups 404s in production and sits outside the
button-wiring and route-reachability gates. Nothing here changes the shipped
component.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@BigSimmo
BigSimmoforce-pushed the claude/top-search-design-mockups-w53znc branch from d9da7a7 to f6f2d61CompareJuly 31, 2026 04:13
… own shelf
Applies the round-7 design decisions to the shared results band, so all twelve
search pages pick them up at once.
The count leads and names its subject. "12 matches" matched what? Once the
query stopped being the band's heading there was nothing on screen answering
that, and the mode registry already carries the noun — so it reads "12
services", "1 DSM diagnosis", "3 therapies". `inlineNoun` keeps the acronym in
"DSM diagnoses" intact; `singularNoun` covers the three shapes the registry
actually uses (ies -> y, ses -> sis, else drop s).
The query moves after the count, at reading weight, as its anchor. The composer
above still shows it, but phone chrome scroll-hides and this band pins, so
while you read results nothing otherwise says what was asked. It is always
rendered rather than appearing on scroll — conditional chrome is what the last
redesign removed — and it truncates before the number does. The heading element
and accessible name are unchanged, so anything resolving the query by role is
unaffected by the reordering.
Applied scope chips move out of the utilities rail onto their own labelled
shelf. They are state, not tools: sharing a horizontally-scrolling rail with
sort and view mixed the two and let a chip be pushed off the right edge,
losing the only affordance for removing it. The shelf carries a "Filtered by"
label so a row of accent pills reads as editable state rather than a second
bank of buttons, and a "Clear" that undoes all of them — previously one tap per
filter with no shortcut. It survives `loading`, unlike the page filter row,
because these chips carry no counts and dropping them mid-search made the shelf
flicker out and back on every keystroke; only a faulted search drops them.
The state tile gains a funnel once the result set is narrowed, so a filtered
list looks different from an unfiltered one before any text is read.
Tests: the DOM suite's count assertions move to the new copy, the ordering test
is rewritten to pin count-before-query rather than the reverse, and three cases
are added for the singular, the acronym, and the irregular plural.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
@BigSimmoBigSimmo changed the title mockups(search band): two directions — Ledger and Statementfeat(search band): count names what it counted, filters get their own shelfJul 31, 2026
@BigSimmoBigSimmo reopened this Jul 31, 2026
@supabase

supabaseBot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@github-actions

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Production UI (1)needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #7515 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo
BigSimmo marked this pull request as ready for review July 31, 2026 06:11
@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.

@BigSimmo
BigSimmo enabled auto-merge July 31, 2026 06:15
@BigSimmoClaude

Copy link
Copy Markdown
OwnerAuthor

Two corrections to the PR body now that auto-merge is armed, since it currently overstates the risk.

1. The earlier PR required failure was not a real failure. Its own log:

CHANGES_RESULT: cancelled STATIC_RESULT: cancelled
SAFETY_RESULT: cancelled COVERAGE_RESULT: cancelled
BUILD_RESULT: cancelled UI_RESULT: cancelled
##[error]CANCELLED with no failing job

Every job cancelled, none failed. Marking the PR ready for review fired a fresh pull_request event and cancel-in-progress killed the run mid-flight. The aggregate stays red because a cancelled job verified nothing — the guard behaving correctly. Run 30608915299 is the superseding run on the same head (5b5fe312).

2. I said Playwright specs asserting the old count copy or chip position "will need updating". I have now checked, and none do.

  • No spec asserts the band's count text. The nearest hit, ui-stress.spec.ts:407's getByText("1 match"), is scoped to scopeContainerdocument-scope-filter, a different component that this diff does not touch.
  • No spec asserts scope-chip position or the contents of search-query-ribbon-utilities, so moving the chips onto their own shelf has no spec to break.
  • The ribbon-scoped specs (ui-smoke, ui-specifiers, ui-formulation, ui-accessibility, ui-route-coverage, ui-tools) assert visibility, roles, overflow and layout — not the count string. search-query-ribbon-mobile-control-pair is untouched by this diff.

That was a prediction in the body, not a measurement; it is now a measurement and the prediction was wrong in the safe direction.

On the UI gate: the body notes verify:ui was not run locally because this container has Chromium 1194 against the pinned build. Production UI critical is in the required set and runs Playwright on the pinned browser in CI, so the gate does run before auto-merge can fire — my local gap is covered by CI rather than waived. If it goes red on this surface I will fix it rather than let it sit.


Generated by Claude Code

@BigSimmo
BigSimmo disabled auto-merge July 31, 2026 06:19
@BigSimmo
BigSimmo enabled auto-merge (squash) July 31, 2026 06:24
@BigSimmo
BigSimmo merged commit 3b4872f into mainJul 31, 2026
25 checks passed
@BigSimmo
BigSimmo deleted the claude/top-search-design-mockups-w53znc branch July 31, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BigSimmo@claude