diff --git a/docs/README.md b/docs/README.md index 4ab5f705e0..f0fd281ec3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -26,6 +26,7 @@ npm run docs:check-links - [frontend-architecture.md](frontend-architecture.md) — shell, routing, dashboard module structure - [wiring-conventions.md](wiring-conventions.md) — page/button wiring conventions and the dead-button / orphan-route gates - [search-chrome-behaviour.md](search-chrome-behaviour.md) — shared search-chrome contract: composer ownership, phone edge-to-edge dock, hide/reveal reserves +- [search-results-bar-decisions.md](search-results-bar-decisions.md) — shared results-bar anatomy, why the filter shelf is scoped to two modes, and what is deliberately not done - [deployment-architecture.md](deployment-architecture.md) — app/worker/Supabase deployment topology - [ingestion-state-machine.md](ingestion-state-machine.md) — ingestion job lifecycle and states - [design-system.md](design-system.md) — tokens, primitives, styling conventions diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 118b061c2a..3f04f1e5a6 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -618,4 +618,7 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-04 | pull/1588 | d79d19c97a918edf785c5a066e691435a3f31518 | Run PR sweep full changed scope | fixed; exact-head CI required | PASS: conflict resolved from current main; combined lock regenerated with 0 vulnerabilities; prior jsdom exact-head build, coverage, static, Lighthouse, container and scans passed. Final merged-tree CI required. | | 2026-08-04 | pull/1602 | 9b2c45c3f10ee7440e7450da7179a390dfbdf4c0 | Run PR sweep full changed scope | merged | PASS: dependency audit, build, coverage, static, Lighthouse, provider-free container smoke, HIGH/CRITICAL scan, SAST, Secret Scan and PR required. | | 2026-08-04 | pull/1603 | dab117d474224964da40af2a1edc016e5dd7ffdf | Run PR sweep full changed scope | merged | PASS: dependency audit, Knip check, typecheck, build, full unit coverage, Lighthouse, provider-free container verification, scans and PR required. | +| 2026-08-04 | claude/search-bar-decisions-doc | 7fc0dbbf48070b2944af0c632ea9aa1f0121be77 | search-bar handoff doc replacement + issues #230 | Docs-only: stale handoff deleted, decisions doc added, ledger row captured. verify:cheap did not complete; prettier/outstanding-issues/docs-links/docs-index passed with quoted output | prettier --check . ; check:outstanding-issues ; docs:check-links ; docs:check-index | | 2026-08-04 | claude/top-search-design-mockups-w53znc | b432448e4893a42d07558aff0dc04be797971231 | PR #1611 — results-band shelf Clear filter-only, memo deps, restored tests | Fixed two Qodo findings from merged #1555; mutation-tested guard added | tsc 0; eslint 0; vitest 4 files/59 tests; verify:pr-local blocked by lock parity (node 24.13 vs jsdom@30) | +| 2026-08-04 | claude/search-bar-decisions-doc | a7dea7f777255ade72878820a636413aaf9588af | search-bar handoff doc replacement + review fixes | Docs-only review fixes: mode/shelf accounting, Sort consumers, #230/#170 precision; removed unquoted-output claim from prior row | prettier --check . ; check:outstanding-issues ; docs:check-links ; docs:check-index | +| 2026-08-04 | claude/search-bar-decisions-doc | 3b4cd6e6bf1f36fb8aff098ce7d333641e0859d3 | search-bar handoff doc replacement + review fixes | Fixed CodeRabbit/Codex findings; Bugbot hosted stuck queued, local Bugbot-equivalent confirmed two P2 doc errors and rejected sheets-are-target finding. verify:pr-local PASS (docs scope). Decisive: prettier All matched files use Prettier code style!; outstanding-issues 228 rows next-id=231; docs link check passed: 1615; docs/codebase-index coverage OK | verify:pr-local (docs); prettier --check; check:outstanding-issues; docs:check-links; docs:check-index; check:branch-review-ledger | diff --git a/docs/handoff-search-bar.md b/docs/handoff-search-bar.md deleted file mode 100644 index 00829a55a2..0000000000 --- a/docs/handoff-search-bar.md +++ /dev/null @@ -1,110 +0,0 @@ -# Handoff — search results bar, `Results bar — perfected` - -Branch `claude/top-search-design-mockups-w53znc`. Four commits on top of -`origin/main` (`40814b44`). - -## Status - -All four commits are verified. The `wip(search)` commit message calls itself -UNVERIFIED and points here — that caveat is **superseded**; it was written before -the gates ran, and history was not rewritten to correct it because the branch was -already pushed. - -| Gate | Result | -| ----------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | -| `npm run verify:pr-local` | exit 0 — 460 files / 4796 tests, production build, client-bundle secret scan, RAG fixtures 36 cases / 23 suites | -| `tests/ui-tools.spec.ts` | 87 passed | -| `tests/ui-smoke.spec.ts` + `tests/ui-accessibility.spec.ts` | 108 passed, 1 failed | - -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 box runs Chromium 1194 against the -project's pinned 1228. Do not chase it. - -`ui-tools` was the one at genuine risk: it carried two assertions on the -Sort/Filter pair that this work deliberately separates, and it runs -`expectNoPageHorizontalOverflow` at 390 px straight after them. Both clear. - -## Where the work came from - -Artifact **`Results bar — perfected`** (`007a83f4-9922-4c96-ab13-47852605bdbe`), -its own seven-step build list. Current state of each: - -| # | Step | State | -| --- | --------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | -| 1 | Merge scope chips + source-type into the facet panel | Done — source-type merged in #1536; scope chips were inert, so `df8c3fb7` deletes rather than merges them | -| 2 | Counts against the same set, disable dead-end facets | Done (#1523 era) | -| 3 | Drop "of 12", use the mode's own noun | Done (#1523) | -| 4 | State tile, Filter right edge, Sort inboard | `6917e732` | -| 5 | The shelf — `Filtered by`, trailing Clear, survives zero result | Done (`cea1d1ca`) | -| 6 | Remove Sources from the results bar | **Deliberately not done** — see below | -| 7 | Decide OR-within-group | Done | - -### Step 6 is declined, not pending - -The study says corpus browsing belongs in nav and the bar button should go. It -explicitly declines to check where nav puts it. In this app the documents action -menu routes through `onSearchModeChange`, which calls `setQuery("")` and -`setModeSearchSubmitted(false)` (`ClinicalDashboard.tsx`), so reaching the -library that way **discards the search being read**. The bar button is the only -in-context route. It was renamed `Library` / `Open source library` instead. Do -not remove it without first giving nav a route that preserves the query. - -### Step 4, what is actually left - -The state tile already existed and already had its alert and search states. -`6917e732` adds the spinner and the funnel. What the study specifies and this -branch does **not** do: - -- **Sort moves into the sheet on phone**, with the sheet retitled - `Filter and sort`. Not done, and it is per-page work: only documents and - therapy-compass have sheets. The other six modes would lose Sort on phone - entirely, which is the exact defect the study itself records fixing in an - earlier round. Do not do this in the shared band. - -## Scope decisions worth not relitigating - -The shelf is on **documents** and **therapy-compass** only. Both have -multi-valued filters hidden behind a panel. The other six modes -(differentials, prescribing, specifiers, formulation, services, factsheets) -have a single-select dimension whose control is already visible in the bar, so -a shelf would restate what is on screen. - -Two related traps, both hit and corrected during this work: - -- **Count what a control does, not how many there are.** Formulation's - "Pattern" and factsheets' "Category" look like filters and are navigation - (`router.push`). Services' "quick filter" rewrites the query. None of them - belong behind a filter surface. -- **A shared component must not read filter state from context.** The old shelf - pulled `commandScopes` from a context no page populated, so it passed a DOM - test that constructed the context by hand and rendered for nobody in - production. The new one is prop-driven: the page supplies `appliedFilters` - (`id`, `label`, `onRemove`). Keep it that way. - -## Ledger - -`#182` (inert command-scope system) is closed by `df8c3fb7`, with the outcome -recorded. `docs/outstanding-issues.md` conflicted on the rebase — `main` had -added `#183`–`#185` while this branch archived `#182`; resolved keeping both -sides. `npm run check:outstanding-issues` passes: 183 rows, 62 open, 121 -archived, next-id 186. - -**Not yet captured** and worth an `/issues` row: `ci.yml`'s "Sync PR policy -body" job reads `PR_POLICY_BODY.md` from the PR head and overwrites the PR -description with it. #1546 committed that scratch file to `main`, so every open -PR had its body replaced with #1546's content — and `pr-policy.mjs` parses the -body as merge-gating input, so governance checklists and verification claims -were showing on PRs they did not belong to. #1548 deleted the file, which fixes -it, but nothing records the underlying habit. - -## Before opening the PR - -- `classifyPullRequestFiles` against the **full** `origin/main...HEAD` diff, not - the tip commit. Getting this wrong on #1536 produced a governance section that - was checked for the wrong change. -- Assemble every commit before the first push. `cancel-in-progress: true` means - a second push cancels the in-flight run and `PR required` scores the - cancellation as a failure — that pattern cost this branch four CI runs. -- If auto-merge is armed, disable it before pushing anything further, then - re-enable. diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 462d9abecd..7e90f4ac54 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -95,7 +95,7 @@ removed after current-main verification; it is not missing recommended work. | 46 | `#167` | A2 | High — verify gates | Next verify:pr-local change | 1–3 hours | `verify:pr-local` exits 0 when its own build step refuses to run — the PR-local gate cannot report success for a step that never executed. | | 47 | `#168` | A3 | High — ledger architecture | With #156 / id-scheme redesign | design first | Sequential issue ids force every concurrent append to conflict — two sessions can append to this ledger at the same time without conflicting. | | 48 | `#169` | A3 | High — git hygiene | Next branch cleanup batch | 1–2 hours | Local branches carry work that exists on no remote — committed work is not lost when a machine or worktree is reclaimed. | -| 49 | `#170` | A2 | High — phone UI | Next documents/filter phone pass | 0.5–1 day | Phone filter controls are inline in a scrolling rail; no sheet — phone filtering opens the repo's own bottom sheet instead of competing for width in the utilities rail. | +| 49 | `#170` | A2 | High — phone UI | Next documents/filter phone pass | 0.5–1 day | Documents and therapy already have page-owned phone filter sheets; remaining modes still use inline controls — shared-band Filter+Sheet adoption without regressing those two or sheetless Sort. | | 50 | `#171` | A2 | High — documents UI | With #170 / filter consolidation | 0.5–1.5 days | Documents mode has four overlapping filtering surfaces, two of them the same job — one Filter control opening one panel, so a reader learns filtering once. | | 51 | `#175` | A2 | Operator — clinical data + Standard | Next therapy catalogue curation window | 2–4 hours | Therapy modality is now null on all 205 records and needs curation or removal — the Therapy detail and recommend screens either show a curated modality or stop carrying the field at all. | | 52 | `#178` | A3 | High — PR policy | Next pr-policy change | 1–2 hours | pr-policy does not flag operational risk bundled with clinical or UI risk — a PR that mixes operational-risk paths with clinical or UI risk is called out before it merges, because squash-merging that mix destroys per-it… | @@ -130,7 +130,7 @@ removed after current-main verification; it is not missing recommended work. | 81 | `#204` | A2 | High — install/CI integrity | Next dependency or verify:pr-local change | 1–3 hours | npm 11.6.2 regenerates a lockfile its own `npm ci` rejects, reddening every CI job — add `npm ci --dry-run` (or lockfile-sync assertion) to `verify:pr-local` when package.json/lock change; do not regenerate with `npm install` to "fix". Distinct from #149. | - + ## Open items > **Merged-main canary update (2026-07-23, run `30018289898`):** the new structured report correctly recorded evaluated tree `c24f2e8f2d30d0c59fc1eba025d3dcd63478137e`, run/attempt identity and `cross-region-runner` latency context. Golden retrieval remained 36/36 with document/content recall 1.0 and no failed cases. The 44-case answer gate had grounded-supported and unsupported-correct rates of 1.0, but failed because `neuroleptic-side-effect-escalation` again returned one citation where two are required (citation-failure rate 0.0227). `admission-discharge-comparison` again omitted the specific AKG admission document after `comparison_source_extractive_fallback`; `admission-discharge-coverage-paraphrase` was advisory-only at 24,870 ms. Answer cost was reported as `$0.234736`. Do not retry immediately: retain this as the first structured datapoint, compare it with the scheduled 2026-07-26 report, and keep retrieval/ranking unchanged. @@ -196,7 +196,7 @@ removed after current-main verification; it is not missing recommended work. | #167 | P2 | issue | `verify:pr-local` exits 0 when its own build step refuses to run | **Outcome:** the PR-local gate cannot report success for a step that never executed. **Detail:** on 2026-07-30 `npm run verify:pr-local` selected the conditional production build for a UI diff; `scripts/guard-next-build.mjs` printed `Refusing to run next build while Clinical KB dev server is running. Stop the dev server first, or set ALLOW_BUILD_WITH_DEV_SERVER=1` — and the aggregate still exited **0**, so the run reported green with the build never run. Caught only by reading the tail of the log; `npm run build` was then re-run separately with the server stopped and passed. Same family as `#120` (`verify:phone-chrome` exits 0 while reporting failed browser tests) and exactly the trap `AGENTS.md` names — "exit code 0 alone is not proof". The guard itself is correct and protects the dev cache; what is wrong is the aggregate treating a refusal as a pass. **Next:** make the refusal exit non-zero, or have `verify:pr-local` list skipped-but-selected steps in its closing summary so a green exit cannot be misread as a build. | session 2026-07-30; PR #1437; `scripts/guard-next-build.mjs` | 2026-07-30 | | #168 | P2 | rec | Sequential issue ids force every concurrent append to conflict | **Outcome:** two sessions can append to this ledger at the same time without conflicting. **Detail:** ids are allocated read-modify-write against the `issues:next-id` marker inside the file being edited, so two branches both read N and both write N. Because duplicate ids are unacceptable, a union merge driver is unsafe — .gitattributes says so explicitly — which is why this file deliberately has no driver and every overlapping append conflicts by hand. Manual resolution is where rows get dropped: PR #1490 was closed during one and took the only record of four snapshots with it (#152), and ids were renumbered under in-flight work three times in one session (#154, #155). The new writer (`scripts/outstanding-issues.mjs`) removes the mechanical errors but explicitly not this one. **Next:** replace the counter with a collision-free id (ULID, timestamp+suffix, or a content hash), keeping a short display number derived at render time if `#151` reads better than 01JQ…; then a union driver becomes safe to reinstate and concurrent appends stop conflicting at all. A larger variant is one row per file under `docs/issues` with the table generated, which the repo already does for `site-map.md`. **Stop:** do not reinstate `merge=union` while ids are sequential — that combination was tried in PR #1416 and removed for duplicating rows and the marker. Renumbered from this PR's original `#159` because `main` already used `#159` for the duplicated test-file-list finding. | session 2026-07-31; .gitattributes; #154/#155; PR #1524 sync | 2026-07-31 | | #169 | P2 | issue | Local branches carry work that exists on no remote | **Outcome:** committed work is not lost when a machine or worktree is reclaimed. **Detail 2026-07-31:** six `claude/*` branches in this checkout have commits and no `origin/` counterpart. Verified real for `claude/clinical-kb-design-system-333a69` — 57 files / +4069, tip `feat(design-system): v2 token layer, 26 components, browser-crash fix` dated 2026-07-31 17:40, whose added `.design-sync/previews/*.tsx` files are absent from main. Others unverified: `design-sync-db0a54`, `fable-implementation-fc937c`, `frosty-mayer-2c6167`, `issues-133-evidence`. **How to check, because the obvious measure lies:** `git rev-list --count origin/main..` and a three-dot diff both report landed work as unmerged, since this repo squash-merges and the original commits never become ancestors — my own merged branch reported 1 commit and +476 by that measure. Test instead whether files the branch adds exist on main (`git ls-tree origin/main `). **Next:** per branch, push it for review or confirm it is superseded and delete it; do not bulk-delete on the commit count. Sibling of #152, which covers uncommitted work in worktrees rather than unpushed commits on branches. | session 2026-07-31; local branch audit | 2026-07-31 | -| #170 | P2 | task | Phone filter controls are inline in a scrolling rail; no sheet | **Outcome:** phone filtering opens the repo's own bottom sheet instead of competing for width in the utilities rail. **Detail:** `SearchResultsHeaderBand` renders `mobileControls` inline below `sm`, so each page's filter control fights the count and sort for a 390 px line. `src/components/ui/sheet.tsx` already provides exactly the right primitive — bottom sheet on mobile via the `sheet-up` keyframe, centred dialog at `sm+`, safe-area aware, focus-trapped, Escape and backdrop dismiss — so this is adoption, not new UI. Design settled in the round-7 study: one `Filter` control at both widths, badge counting applied filters, sheet titled *Filter and sort* because a phone bar cannot fit a labelled Sort beside Filter at 390 px. **Next:** replace the inline `mobileControls` render with a Filter trigger + `Sheet` containing `mobileControls ?? filterControls`, so each page keeps supplying its own filter content. **Stop:** at least four Playwright specs drive those inline controls directly (`document-source-type-select`, `search-query-ribbon-mobile-control-pair`); budget for updating them, and do not ship without `verify:ui` — PR #1523 showed `Production UI critical` does gate this surface properly. | Round-7 design study; PR #1523 notes; `src/components/ui/sheet.tsx` | 2026-07-31 | +| #170 | P2 | task | Phone filter sheets exist for documents and therapy; shared-band adoption remains | **Outcome:** phone filtering opens the repo's own bottom sheet instead of competing for width in the utilities rail. **Current state (2026-08-04):** documents already mounts a Filter documents Sheet, and therapy-compass mounts TherapyFilterSheet; both also pass appliedFilters into SearchResultsHeaderBand. The remaining modes still keep filter controls inline (or page-owned chips/nav), and the shared band has not yet adopted a Filter trigger + Sheet that wraps mobileControls for every mode. **Detail:** SearchResultsHeaderBand still renders mobileControls inline below sm for pages that supply them that way. src/components/ui/sheet.tsx already provides the primitive — bottom sheet on mobile via sheet-up, centred dialog at sm+, safe-area aware, focus-trapped, Escape and backdrop dismiss — so shared-band adoption is the remaining work, not new UI. Design settled in the round-7 study: one Filter control at both widths, badge counting applied filters, sheet titled Filter and sort because a phone bar cannot fit a labelled Sort beside Filter at 390 px. **Next:** replace the inline mobileControls render with a Filter trigger + Sheet containing mobileControls ?? filterControls for the modes that still lack a page-owned sheet, without regressing documents/therapy sheets or removing Sort from sheetless Sort consumers (differentials, forms, services). **Stop:** at least four Playwright specs drive those inline controls directly (document-source-type-select, search-query-ribbon-mobile-control-pair); budget for updating them, and do not ship without verify:ui — PR #1523 showed Production UI critical does gate this surface properly. | Round-7 design study; PR #1523 notes; `src/components/ui/sheet.tsx` | 2026-07-31 | | #171 | P2 | issue | Documents mode has four overlapping filtering surfaces, two of them the same job | **Outcome:** one Filter control opening one panel, so a reader learns filtering once. **Detail:** `/documents` currently offers (a) smart-tag facets — 11 groups from `smartDocumentFacetGroups`, built by `buildSmartDocumentTagFacetIndex`, the real system; (b) a **source-type** control over `all/tables/images/pdfs`, which duplicates the facet group already named `Document type`; (c) scope chips from `searchCommandSurfaceConfig` (`Guidelines`, `Tables`, `Quotes`, `Current only`) — note `Tables` appears in both (b) and (c); (d) the `Sources` button, which is `openDocumentsDrawer("library")` and is not a filter at all. Traced 2026-07-31; `commandScopes` filter client-side after retrieval in the pages checked (services, prescribing), so merging looks safe from the RAG-protection rules — **verify every mode before editing**, this was not exhaustively checked. **Next:** fold (b) and (c) into the facet panel as groups; take (d) out of the results bar (see `#172`). **Stop:** this is a behaviour change across pages — own PR, own reasoning; do not bundle it with copy or layout work. | `document-search-results.tsx:920-937`; `document-tags.ts:798,849`; session 2026-07-31 | 2026-07-31 | | #172 | P3 | task | `Sources` sits in the results bar but is navigation, not a filter | **Outcome:** the results bar holds only controls that act on the current results. **Detail:** `Sources` is passed as `utilityControls` from `document-search-results.tsx:1026` and calls `onOpenLibrary` -> `openDocumentsDrawer("library")`. Its own accessible name is *"Open source filters"*, which is why it reads as a second Filter beside the real one. It is also **documents-mode only**, so it can never be a fixture of a shared band. It reaches the ~2,000-document corpus; Filter narrows the ~12 a query returned — zero overlap, which is exactly why they cannot merge. Its one genuinely unique job is telling *"no results"* apart from *"not indexed"*, which matters in a clinical reference. **Next:** move to nav as **Browse library**; consider surfacing *Recently opened* separately since it is used far more often than corpus browsing but currently sits at the same depth. **Stop:** do not delete the capability — only relocate and rename it. | `document-search-results.tsx:1026`; `ClinicalDashboard.tsx:2752`; session 2026-07-31 | 2026-07-31 | | #174 | P3 | rec | Facets AND within a group, so two values from one group almost always return nothing | **Outcome:** a decision on record, either way. **Detail:** `filterDocumentsBySmartTagFacetIndex` applies `selected.every(...)` across **all** selected keys, with no notion of grouping. So ticking `Medication:Lithium` and `Medication:Clozapine` asks for documents about **both** and returns zero — demonstrated in the round-7 study, where that exact pair is the "No matches" preset. Conventional faceted search ORs within a group and ANDs across groups, which would make that pair mean "either medication". **Next:** decide. If OR-within-group is wanted, it is a small change to the same helper plus its callers, but it changes what every existing multi-select does. **Stop:** this is a product decision, not a defect to fix unilaterally — the current behaviour is self-consistent and may be intended. Flagged, not changed. | `document-tags.ts:849-858`; round-7 study; session 2026-07-31 | 2026-07-31 | @@ -247,6 +247,7 @@ removed after current-main verification; it is not missing recommended work. | #224 | P3 | issue | DSM search empty state lost its h2 when it adopted EmptyState | src/components/dsm/dsm-search-page.tsx rendered No diagnosis matches as an h2 inside a centred card. Adopting EmptyState (PR-J, Builder B) moved that copy into EmptyState's title, which renders as a p (ledger #217), so /dsm/search no longer carries a heading for its empty result state and heading navigation skips it. No test pinned the heading, which is why the adoption passed review and the gates. This is the first live instance of #217 rather than a separate defect: the fix is the same headingLevel prop, and if that lands this row closes with it. Next action: decide #217, then re-adopt this state with a heading. Stop: do not hand-roll the card back — the restyle is intended. | session 2026-08-03 (PR-J Wave 5, controller hunk read) | 2026-08-02 | | #225 | P3 | issue | Favourites renders up to three EmptyState live regions at once | src/components/clinical-dashboard/favourites-command-library-page.tsx renders its no-matches state three times in the DOM at once: two table cells (one for below-2xl, one for 2xl+) plus the phone card list, each hidden at the wrong breakpoint by CSS. That duplication predates PR-J, but PR-J converged all three onto EmptyState, whose default live=polite makes each one a role=status live region. Filtering is what empties the list, so the visible one is a genuine dynamic announcement; the risk is that a screen reader that does not exclude display:none subtrees hears it two or three times. Not observed — display:none content is normally excluded from the accessibility tree, which is why this is P3 and not a fix inside the wave. Next action: render the empty state once outside both breakpoint containers, or pass live only to the visible instance. Stop: do not remove the role from EmptyState — the announcement is the reason the primitive was adopted here. | session 2026-08-03 (PR-J Wave 5, controller hunk read) | 2026-08-02 | | #226 | P1 | task | PR-J: VerificationNotice pushes the phone short-answer runway past the in-flow activation band | CI run 30820496984 reported the real numbers: ui-smoke phantom-scroll 97 against an 8px budget, short-runway maxOffset 251 against a 200px ceiling. Local readings for the same tree were 29 and passing, confirming finding L's 41-81px offset and the rule never to pin from this machine. Re-pinned from CI per the user decision: bare phantom budget 8 to 112, maxOffset ceiling 200 to 280, postCollapseMaxOffset ceiling 72 to 160. State plainly what that means: the 72px in-flow activation band was a real contract and the post-collapse runway no longer fits inside it, because every answer now carries an unconditional verification notice above the prose. The collapse budget is untouched - chrome height did not change, only the content below it. OPEN QUESTION for the clinical owner, separate from the pin: a one-sentence answer now carries roughly 97px of notice above it on a phone, which is what the phantom-scroll guard was originally written to prevent. Widening the pin accepts that as intended product behaviour. | session 2026-08-03 (PR-J Wave 5, gate window, verify:phone-chrome) | 2026-08-03 | +| #230 | P2 | issue | CI's PR-policy body sync can overwrite PR descriptions from a committed scratch file | ci.yml's Sync PR policy body job runs only when the PR head contains PR_POLICY_BODY.md (pr_policy_body_present) and then replaces that PR's description with the file. #1546 committed the scratch file to main; any open PR whose head still contained the file (including those that merged or rebased onto that tip) inherited #1546's body — and pr-policy.mjs parses the body as merge-gating input, so governance checklists and verification claims appeared on PRs they did not belong to. Heads without the file are not rewritten. #1548 deleted the file, which removes today's symptom; nothing stops the next branch committing one. Next action: make the sync job no-op unless the file is new in that PR's own diff, or move the handoff body out of the repo entirely. | Observed during the search-results-bar work (#1555); recorded in that branch's handoff doc rather than the ledger at the time | 2026-08-04 | ## Resolved / archive diff --git a/docs/search-results-bar-decisions.md b/docs/search-results-bar-decisions.md new file mode 100644 index 0000000000..011ae24dfa --- /dev/null +++ b/docs/search-results-bar-decisions.md @@ -0,0 +1,73 @@ +# Search results bar — decisions + +The shared results bar is `SearchResultsHeaderBand` +(`src/components/clinical-dashboard/search-results-header-band.tsx`). All twelve +`results-band` modes render it (every searchable mode except answer), so a change +there lands everywhere at once. This file records the decisions that are easy to +reverse by accident — each was reached by checking what the app actually does, not +what the surface looks like it should do. + +Composer placement, phone dock reserves, and hide/reveal behaviour are a different +contract: see [search-chrome-behaviour.md](search-chrome-behaviour.md). + +## Anatomy + +- **State tile.** Carries state as shape, not only colour: alert when the search faulted or + returned partial results, spinner while one is running, funnel once the result set is + narrowed, magnifier otherwise. The funnel is driven by `appliedFilters.length`, the same + data the shelf reads, so a filtered list looks different from an unfiltered one before + any text is read. +- **Filter at the right edge, Sort inboard.** Sort is set about once a session. Filter is + the only control carrying state and the one returned to repeatedly, and on a phone the + right edge is where the thumb already is. The page filter is therefore the utility + rail's **last** child; `tests/ui-tools.spec.ts` asserts that placement and a matched + phone tap height, rather than the Sort/Filter adjacency it asserted before. +- **The shelf.** A labelled `Filtered by` row under the bar, one tap to remove each + filter, trailing `Clear` once more than one is applied. The label matters: without it a + row of accent pills reads as a second bank of buttons rather than as state. It + deliberately survives `loading` and a zero-result set — nothing matching is exactly when + a filter needs relaxing, 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. + +## The shelf is scoped to two modes, on purpose + +`documents` and `therapy-compass` only. Both have multi-valued filters hidden behind a +panel, so what is applied is not otherwise visible. Of the other ten results-band modes: + +- Differentials, prescribing, specifiers, formulation, services, and factsheets keep a + single-select dimension whose control is already on screen, so a shelf would restate it. +- Forms still ships a Filter trigger whose panel is a coming-soon placeholder, not applied + filter state, so there is nothing for a shelf to show. +- Favourites renders its own active-filter chips inside `filterControls` rather than the + shared shelf props. +- DSM filters by category through navigation links, and tools through a single category + select — neither passes `appliedFilters`. + +Two traps met while drawing that line: + +- **Count what a control does, not how many there are.** Formulation's "Pattern" and + factsheets' "Category" look like filters and are navigation (`router.push`). Services' + "quick filter" rewrites the query. None of them belong behind a filter surface. +- **A shared component must not read filter state from context.** The original shelf + pulled `commandScopes` from a context that no page populated. It passed a DOM test which + constructed that context by hand, and rendered for nobody in production. The replacement + is prop-driven — the page supplies `appliedFilters` (`id`, `label`, `onRemove`) and an + optional `onClearFilters`. Keep it that way. + +## Deliberately not done + +- **The library button stays in the bar.** The argument for removing it is that corpus + browsing belongs in nav. In this app the documents action menu routes through + `onSearchModeChange`, which calls `setQuery("")` and `setModeSearchSubmitted(false)` + (`ClinicalDashboard.tsx`) — reaching the library that way discards the search being + read. The bar button is the only in-context route, and is named after what it opens + (`Library` / `Open source library`). Do not remove it before nav has a route that + preserves the query. +- **Sort does not move into the phone filter sheet.** Only documents and therapy-compass + currently have phone filter sheets (`Filter documents` / `TherapyFilterSheet`). Of the + four production `onSortChange` consumers, documents already pairs Sort with a sheet; + therapy has a sheet but no Sort. Moving Sort into the sheet from the shared band would + remove Sort from phones in the three sheetless Sort consumers — differentials, forms, + and services — the exact defect an earlier round fixed. If this is ever wanted it is + per-page work on those consumers, not a shared-band change.