diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 598dbd1bd4..b2cc7728d7 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1145,6 +1145,7 @@ This file is append-only. Never rewrite or delete an existing review record; app | 2026-07-27 | `codex/settings-followup` | `806fcc4c3167d9e2f9fbd832c39e53d3491f270a` | Protected-main release-readiness review of settings follow-up browser reliability | APPROVE. The test-only diff waits for one settled React owner before strict answer/search interactions, makes universal-search mocks echo the requested query, and retries scroll-to-live-endpoint geometry after late dock layout. Review found no P0-P3 issue and no product, retrieval, ranking, clinical-output, or provider behavior change. Highest residual risk is physical iOS/WebKit behavior outside local Chromium coverage. | Focused integrated production Chromium PASS (5/5); exact integrated-head `verify:pr-local` PASS (runtime, formatting, lint, typecheck, 393 files, 3,538 passed / 2 skipped, 36 offline RAG fixtures); `verify:ui` PASS (323/323); `git diff --check` PASS; no non-GitHub provider-backed checks. | | 2026-07-27 | PR #1279 / `codex/phone-chrome-testing-infra-20260727` | `e1add338ae6b138ded13c44f5cc8f7eef5302669` | Automated-review follow-up for required-check selection in the final merge audit | APPROVE pending fresh exact-head hosted checks. The P1 was valid: the audit selected the required aggregate but incorrectly treated every advisory status as merge-blocking. Unsettled-state validation now applies only to the required aggregate, while missing or unsuccessful `pr-required` still fails closed. A regression snapshot proves a failed Advisory UI job cannot block a successful required aggregate. No other P0-P3 finding remains. | Focused `tests/final-merge-audit.test.ts` PASS (7/7); Prettier and `git diff --check` PASS; earlier exact-tree `verify:phone-chrome` remains the browser baseline because this follow-up changes only the audit validator and its unit test; hosted required checks must rerun on this head. | | 2026-07-27 | PR #1279 / `codex/phone-chrome-testing-infra-20260727` | `a2331fc8d1883d687d0bbb6e1b023503ab5deb1d` | Automated-review follow-up for changed Playwright journey selection | APPROVE pending fresh exact-head hosted checks. The P2 was valid: fixed title filters could omit a modified journey while the planner still reported focused coverage. Changed phone-chrome Playwright specs now run completely without `--grep`; the title-filtered matrix remains only for relevant unchanged specs, preserving focused-first feedback without hiding edited tests. Regression cases cover both `ui-phone-scroll` and `ui-tools`. No other P0-P3 finding remains. | Focused `tests/verify-phone-chrome.test.ts` PASS (7/7); smart-plan dry run selects complete changed specs before the risk-selected full UI suite; Prettier and `git diff --check` PASS; fresh hosted required checks must rerun on this head. | +| 2026-07-27 | PR #1280 / `claude/top-search-design-mockups-w53znc` | `78c7d1c7766c081d886f1abbd14fa7b3018a0d44` | CI fix: Production UI Loading-answer strict-mode race | FIXED. Hosted Production UI failed once on `answer search URL opens chat without the answer home copy` when `getByLabel("Loading answer")` matched the live skeleton plus a hidden Suspense `S:` clone (search-chrome invariant 17). Assertion now uses the suite-standard `:visible` locator. Not a product regression from the results-band rebuild. | Exact journey PASS 3/3 with system Chrome after the harden; hosted CI rerunning on this head; no provider-backed checks. | | 2026-07-27 | open-PR review + Bugbot sweep (Cursor) | multi-head | Fresh review of all open PRs + pr-bugbot on each head | Reviewed 10 open PRs after #1277 merged. No hosted `cursor[bot]` Bugbot comments existed on any PR; ran repo `pr-bugbot` triage per head instead. Cluster: close #1261/#1263 (unsafe audit lineage); salvage/rebuild only thin #1262 bits; hold Dependabot #1267→#1268→#1269 pending workflow approval; fix #1273 type-scale + mockup UX before design trust; sync #1275 and restore ledger from main; #1280 blocked by unrelated Loading-answer strict-mode flake; #1281 sound clinical follow-up with two P2 polish items. | merge-tree classification; gh PR/CI/thread inventory; pr-bugbot review-only agents; no provider-backed checks; no PR mutations. | | 2026-07-27 | PR #1261 / `apply-audit-system-remediation` | `b4dae8469024` | Bugbot + merge-tree review | DO NOT MERGE / CLOSE. Same `faa50e6e` dirty-checkpoint lineage as closed #1255/#1253; 526 behind; 18 real merge-tree conflicts including answer API/ClinicalDashboard/evidence. Confirmed Codex P1: tip adds unconditional `out_of_corpus` short-circuit that main deliberately avoids. PR policy missing RAG/governance. | merge-tree; tip vs main RAG guard compare; unresolved-thread validation; no provider checks. | @@ -1157,6 +1158,8 @@ This file is append-only. Never rewrite or delete an existing review record; app | 2026-07-27 | PR #1275 / `codex/identify-and-fix-performance-issues-during-mode-switch` | `24605b57e288` | Bugbot + merge-tree review | NOT READY. Prefetch product change looks auth-safe/correct. GitHub DIRTY is staleness (merge-tree CLEAN). Blockers: ledger rewrite/corruption (~95 historical rows) + incomplete required CI. Sync main, restore ledger from origin/main, append one row, then recheck. | merge-tree CLEAN; ledger byte/corruption inspect; unresolved Codex/CodeRabbit threads; no provider checks. | | 2026-07-27 | PR #1280 / `claude/top-search-design-mockups-w53znc` | `93a9f90ff287` | Bugbot + CI debug | NOT READY until Production UI green. Product band rebuild looks sound; Advisory UI green. Hosted failure is Answer Suspense `Loading answer` strict-mode (2 nodes / one hidden) in ui-smoke — not caused by band diff. Optional P2: `useRailOverflow` can miss child-list changes. | Production UI log job 90037898852; unique diff vs main; focused band unit 9/9 on tip; no provider checks. | | 2026-07-27 | PR #1281 / `claude/safety-planning-tools-page-tsq4vs` | `a26e95fc9ac9` | Bugbot clinical review | APPROVE pending exact-head required CI + minor P2 polish. Incomplete plans get draft banner/clipboard marking; contact reach methods required for Ready/Finalise. P2: StepBuilderCard green check still uses entries.length; clipboard DRAFT text untested. No P0/P1. | unique diff review; GraphQL no cursor[bot] threads; no provider checks. | +| 2026-07-27 | PR #1280 / `claude/top-search-design-mockups-w53znc` | `980b4298933642d134d44105b62ab0c31d39d4e3` | Hosted required CI after Loading-answer harden + main sync | GREEN. Supersedes the `78c7d1c7` pending-rerun row. Production UI and `PR required` both SUCCESS on this tip; Loading-answer `:visible` assertion retained through the later rail-overflow fix and `origin/main` merge. | Hosted CI run 30308513222: Production UI SUCCESS (11m22s), PR required SUCCESS; local exact journey PASS 3/3 earlier on the harden; no provider-backed checks. | +| 2026-07-27 | PR #1280 / `claude/top-search-design-mockups-w53znc` | `c844560da909b7f88a1ef351fc833ad45cfb0a81` vs `origin/main` `7740535a` | Thorough merge-readiness review vs main | APPROVE AFTER MAIN SYNC. No P0/P1. Presentation-only shared results band rebuild; clean merge-tree; hosted PR required + Production UI green on tip; 0 unresolved threads. Branch is 1 commit behind unrelated safety-plan #1281 — final-merge audit fails closed until tip contains origin/main. Residual P2 (non-blocking): phone overflowing-rail `mask-image` lacks forced-colors clear (pattern exists for edge-glass); ResizeObserver does not unobserve removed children. | Diff review of 6 unique files; `git merge-tree` clean; `audit:final-merge --dry-run` fails only on missing main ancestor; band DOM 9/9; hosted checks green on tip; no provider-backed app checks. | | 2026-07-27 | PR #1287 / `claude/site-audit-quick-wins-21v9gb` | `97ab067bfdca644e0750bfbc717da7d58ecd27ee` | Bugbot defect hunt (cursoragent request; no hosted cursor[bot] threads) | APPROVE pending exact-head required CI. No P0/P1. Projection ≡ live helpers (201 diagnoses / 31 presentations / 20 alias keys); `--check` compares parsed values (Prettier-safe); CI `static-pr` + `verify:cheap` wire `check:cross-mode-index`. Residual P2: re-importing `@/lib/differentials` into `cross-mode-differentials.ts` would restore the ~1.2 MB lazy-chunk weight while data gates stay green — no import-graph lock yet. P3: stale comment in `cross-mode-links.tsx`; scripts-index omits new generator. | `check:cross-mode-index` PASS; vitest `cross-mode-differentials-index` 2/2; gate-manifest PASS; drift/invalid-JSON proofs FAIL closed; import-graph grep clean today; no provider-backed checks. | | 2026-07-27 | PR #1287 / `claude/site-audit-quick-wins-21v9gb` | `09dbd2dcc5126e4ae7d6f9e99e7325853047f749` | Bugbot P2 follow-up: import-graph lock for cross-mode differentials | FIXED. Added a client-performance-boundaries source assertion that `cross-mode-differentials.ts` stays on the trimmed JSON index (no value-import of `@/lib/differentials` / snapshot / fixtures) and that `cross-mode-links.tsx` keeps the dynamic import. Updated the stale 1.2 MB comment. Residual: dual projection logic still lives in the build script and `differentials.ts` (caught by the existing deep-equal test). | Focused `tests/client-performance-boundaries.test.ts` PASS (7/7); no provider-backed checks. | | 2026-07-27 | PR #1287 / `claude/site-audit-quick-wins-21v9gb` | `18fcfae24b41cdd5caebd2780afa963a2a1335a5` | Follow-up — Bugbot/CodeRabbit residuals addressed (supersedes the 97ab067 Bugbot row) | Residual P2 closed: import-graph lock added — an allowlist test asserts `cross-mode-differentials.ts` may import ONLY the precomputed index + the (type-only) catalog type, catching direct, transitive-via-new-import, and dynamic `import()`/`require` reintroductions of `@/lib/differentials`. P3s closed: stale `cross-mode-links.tsx` comment fixed; `build-cross-mode-differentials-index.mjs` listed in `scripts-index.md`. CodeRabbit's two Minor nits (guard depth + this ledger refresh) addressed. | vitest `cross-mode-differentials-index` 3/3; typecheck + lint PASS; `docs:check-scripts` + `docs:check-links` PASS; `check:cross-mode-index` PASS; no provider-backed checks. | diff --git a/docs/search-chrome-behaviour.md b/docs/search-chrome-behaviour.md index b78b7003a4..b1ee877d48 100644 --- a/docs/search-chrome-behaviour.md +++ b/docs/search-chrome-behaviour.md @@ -38,6 +38,39 @@ This repo uses one shared search experience across the global shell, dashboard r 21. Installed standalone phones use the same normal-flow root with the final `display-mode: standalone` `100vh` bound and an internal `.phone-scroll-surface`. Keep that override after the browser contract; do not substitute `svh`, `dvh`, `visualViewport.height`, or a fixed root on this WebKit workaround path. Every phone footer uses `.phone-footer-layer`: fixed to the viewport in browser tabs and absolute to the positioned 100vh frame in standalone, so the composer and its backdrop share the repaired PWA edge. Page-owned footer layers must render through `PhoneFooterLayerPortal`; `PhoneFooterLayerFrame` provides a frame-scoped, paint-free host after the scroll surface. An absolute footer left inside `.phone-scroll-surface` still scrolls and clips with that surface. 22. Safari's status bar, collapsing address bar, and pixels outside `window.innerHeight` are native browser/system controls. Do not use negative safe-area overscan, a fixed app root, synthetic document padding, or an opaque viewport slab to make CSS appear to own those pixels. Acceptance is no contrasting **app-owned** band around the native controls, with a matching opaque root canvas. Use the labelled physical-device matrix in [phone-chrome-physical-acceptance.md](phone-chrome-physical-acceptance.md). +## Results band (`SearchResultsHeaderBand`) + +The band above every result list is not a composer and owns no dock reserve, but it is shared +chrome and changes to it land on every mode at once. Keep these rules: + +1. **The query is the only heading-weight thing in the band.** It renders at `text-lg` + `font-extrabold` with no eyebrow — the magnifier tile already says "search", and a `QUERY` / + `RESULTS FOR` label costs a line to repeat it. The query truncates; the count does not. +2. **The count is neutral text, not a success pill.** `text-muted` with the figure itself + `font-extrabold tabular-nums`. Success colour is reserved for states that were actually + achieved, so it still carries meaning where it appears. The `role="status"` / + `aria-live="polite"` announcement stays either way. +3. **Sort is a segmented control, not a select.** Two values do not justify a menu you must open + to read. `ResultSortControl` renders `sortOptions` as `aria-pressed` buttons inside a + `role="group"` named "Sort results"; add a third order only if it still fits the rail. +4. **Native selects are pinned to 16px below `sm`.** The unlayered iOS anti-zoom rule in + `globals.css` ("Interactive element defaults") deliberately beats Tailwind's `text-*` + utilities on `input`/`select`/`textarea`. Do not fight it with `!important` or a per-call-site + override — a sub-16px control zooms the viewport on focus in Safari. Any control that must + read quieter than the query steps down in **weight and colour**, never in size, and any + select carrying variable-length values must set `truncate` or it clips mid-word rather than + ellipsing (the "Current search" → "Current searcl" defect fixed 2026-07-27). +5. **The utility group is a swipe rail below `lg`, an inline row at `lg+`.** Children are + `shrink-0` so they keep their natural width; overflow scrolls instead of wrapping into a + second tinted band. The right-edge fade is applied via `data-overflowing` only while the rail + actually overflows — never as a permanent mask. +6. **Active scopes render as removable chips at the head of that group**, in accent tone, so a + constraint on the list is one tap from where it is read. Do not move them into a separate + strip; `hasUtilities` already suppresses the whole group when nothing is active. + +Coverage: `tests/search-results-header-band.dom.test.tsx` (structure, sort wiring, count tone), +`tests/ui-tools.spec.ts` (phone control pair geometry and tap heights). + ## Scroll hide/reveal The universal **top bar** (mode, new chat, menu) is the only sticky desktop chrome: it hides on a deliberate scroll down and returns on a deliberate scroll up at **every** breakpoint. Tablet search stays pinned below it. Desktop search is mounted at the top of normal page content, so it scrolls away with that content and is independent of the header's hide state. Only the phone bottom search dock scroll-hides, and that stays phone-only. The top bar and phone dock read one `useScrollHideReporter` per host, so they can never disagree about direction. diff --git a/src/components/clinical-dashboard/search-results-header-band.tsx b/src/components/clinical-dashboard/search-results-header-band.tsx index 272d19a7f9..1f41848352 100644 --- a/src/components/clinical-dashboard/search-results-header-band.tsx +++ b/src/components/clinical-dashboard/search-results-header-band.tsx @@ -1,7 +1,7 @@ "use client"; -import { Bookmark, CheckCircle2, ChevronsUpDown, LayoutList, LoaderCircle, Search, Table2, X } from "lucide-react"; -import type { ReactNode } from "react"; +import { Bookmark, ChevronsUpDown, LayoutList, LoaderCircle, Search, Table2, X } from "lucide-react"; +import { useCallback, useEffect, useRef, useState, type ReactNode } from "react"; import { searchCommandSurfaceConfig } from "@/lib/search-command-surface"; import { cn } from "@/components/ui-primitives"; @@ -12,6 +12,53 @@ import { readResultSort, type ResultSortValue } from "@/lib/result-sort"; const focusRing = "focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; +/** Sort is a two-state choice, so it reads as a segmented control rather than a + select: a dropdown over two values makes you open a menu to learn nothing. */ +const sortOptions: ReadonlyArray<{ value: ResultSortValue; label: string }> = [ + { value: "relevance", label: "Relevance" }, + { value: "alpha", label: "A–Z" }, +]; + +/** Below `lg` the utility group is a swipe rail rather than a wrapping block, so a + sixth control lands off the right edge instead of growing the band. Fade that + edge only while it actually overflows — a permanent mask would dim the last + control on the common case where everything fits. */ +function useRailOverflow() { + const ref = useRef(null); + const [overflowing, setOverflowing] = useState(false); + + const measure = useCallback(() => { + const node = ref.current; + if (!node) return; + setOverflowing(node.scrollWidth - node.clientWidth > 1); + }, []); + + useEffect(() => { + const node = ref.current; + if (!node || typeof ResizeObserver === "undefined") return; + measure(); + const resizeObserver = new ResizeObserver(measure); + resizeObserver.observe(node); + for (const child of Array.from(node.children)) resizeObserver.observe(child); + // Child add/remove (scope chips, utility controls) can change scrollWidth without + // resizing the rail box; keep the fade mask honest when the child list mutates. + const mutationObserver = + typeof MutationObserver === "undefined" + ? null + : new MutationObserver(() => { + measure(); + for (const child of Array.from(node.children)) resizeObserver.observe(child); + }); + mutationObserver?.observe(node, { childList: true }); + return () => { + resizeObserver.disconnect(); + mutationObserver?.disconnect(); + }; + }, [measure]); + + return { ref, overflowing } as const; +} + export function SearchResultsHeaderBand({ modeId, query, @@ -57,11 +104,11 @@ export function SearchResultsHeaderBand({ return scope ? [scope] : []; }); const displayQuery = query.trim() || "All"; - const statusLabel = loading ? "Searching…" : `${matchCount} ${matchCount === 1 ? "match" : "matches"}`; const hasUtilities = visibleScopes.length > 0 || Boolean(onSortChange || onViewChange || onSaveSearch || utilityControls || mobileControls); const QueryHeading = headingLevel === 1 ? "h1" : "h2"; + const { ref: railRef, overflowing: railOverflowing } = useRailOverflow(); return (
-
-
+
+
- - - Query - {loading ? "Searching for" : "Results for"} - - - {displayQuery} - - + {/* No eyebrow: the icon already says "search", and the query is the only + thing in this band set at heading weight. */} + + {displayQuery} + + + {/* Neutral, not a success pill: a count is not a state that was achieved, + and green has to keep meaning something where it does appear. */} {loading ? ( - + + + Searching… + ) : ( - + <> + {matchCount}{" "} + {matchCount === 1 ? "match" : "matches"} + )} - {statusLabel}
{hasUtilities ? (
+ {/* Scope reads as a removable chip beside the query — it is a constraint on + the list, the same kind of thing the query is. */} {visibleScopes.map((scope) => ( ))} + {/* Desktop only: pushes the controls to the trailing edge while the chips + stay next to the query. On the phone rail this collapses away. */} + {onSortChange && mobileControls ? (
- +
{mobileControls}
) : ( <> - {onSortChange ? ( - - ) : null} + {onSortChange ? : null} {mobileControls ? (
{mobileControls}
@@ -177,7 +228,7 @@ export function SearchResultsHeaderBand({ {utilityControls} {onViewChange ? (
@@ -218,7 +269,7 @@ export function SearchResultsHeaderBand({ type="button" onClick={onSaveSearch} className={cn( - "inline-flex min-h-tap items-center gap-1.5 rounded-lg border border-[color:var(--border)] bg-[color:var(--surface)] px-2.5 text-xs font-extrabold text-[color:var(--text-muted)] shadow-[var(--shadow-inset)] hover:border-[color:var(--border-strong)] hover:text-[color:var(--text)] sm:min-h-10", + "inline-flex min-h-tap shrink-0 items-center gap-1.5 rounded-lg border border-[color:var(--border)] bg-[color:var(--surface)] px-2.5 text-xs font-extrabold text-[color:var(--text-muted)] shadow-[var(--shadow-inset)] hover:border-[color:var(--border-strong)] hover:text-[color:var(--text)] sm:min-h-10", focusRing, )} > @@ -250,39 +301,42 @@ export function ResultSortControl({ value, onChange, className, - compact = false, }: { value: ResultSortValue; onChange: (value: ResultSortValue) => void; className?: string; - /** Hide the visual "Sort" label on narrow viewports; the select keeps its accessible name. */ - compact?: boolean; }) { return ( - + {sortOptions.map((option, index) => { + const selected = option.value === value; + return ( + + ); + })} +
); } @@ -312,12 +366,18 @@ export function MobileResultFilterControl({ )} > {label} + {/* Two things keep this readable. `truncate` ends a long option ("Current + search", a service name) in an ellipsis instead of the mid-word cut it used + to get. And the weight steps down to semibold because the size cannot: the + unlayered iOS anti-zoom rule in globals.css pins every native select to 16px + below `sm`, so weight and colour are the only hierarchy left against the + 18px query heading. */}