Results bar: one line, one filter, nothing unreachable - #1615
Conversation
…adable 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
…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
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
…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
`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
…faced #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
…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
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe redesign updates shared search-result bars, document filtering, filtered empty states, responsive controls, forced-colors styling, Therapy Compass integration, and validation records. ChangesSearch results redesign
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SearchResultsConsumer
participant SearchResultsHeaderBand
participant DocumentFilterPanel
participant SourcesDialog
SearchResultsConsumer->>SearchResultsHeaderBand: render result state and filter controls
SearchResultsConsumer->>DocumentFilterPanel: open query-scoped filters
DocumentFilterPanel->>SearchResultsConsumer: apply facets and update counts
DocumentFilterPanel->>SourcesDialog: browse all sources
SourcesDialog-->>SearchResultsConsumer: close and restore document-options focus
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8077 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
BigSimmo
commented
Aug 5, 2026
@cursoragent run bugbot |
Bugbot triage on PR #1615 is done. No prior Findings
Fix pushed
Evidence: Branch tip on origin: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 5, 2026
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/top-search-design-mockups-fbbfuf at starting commit 86dd5dd; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/top-search-design-mockups-fbbfuf, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
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>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
PR #1615 is clear and ready to merge. Cleared
Findings re-checked (no code changes needed)
Verification
Tip unchanged at |
Resolve-all pass on |
| Area | Result |
|---|---|
CI (PR required + Production UI shards, coverage, safety, build, static) | Green on b89b4cde |
Mergeability vs main | 0 behind, merge-tree clean, mergeable=true |
mergeStateStatus=BLOCKED | Not a content conflict. Required checks are green; remaining block is review approval (no APPROVED review yet — bot reviews are COMMENTED only). Human approval still required to merge. |
| Unresolved review threads | 0 after this pass (last open thread was the outdated Codex onClick P1) |
Findings disposition
| Severity | Finding | Disposition |
|---|---|---|
| P1 | Unclosed dense-disclosure onClick JSX | Already fixed (be8a49c7); thread replied + resolved |
| P2 | Library unreachable when filter sheet absent | Already fixed — fallback document-results-browse-library when showResultsControls && !hasFilters; pinned by test |
| P2 | Therapy query-only empty state had no recovery | Already fixed — onClearSearch={b.clearSearch} + body derived from controls; pinned by test |
| P2/P3 | Dense selected groups won’t collapse (Sentry) | Disagreed — intentional; collapsed set wins; pinned by test |
| Prior CodeRabbit / Devin / Copilot items | searchText, overflow callback-ref, singular copy, /api/differentials intercept, etc. | Already addressed on head |
Verification this pass
Test Files 3 passed (3)
Tests 77 passed (77)
(document-filter-panel, search-results-header-band, therapy-compass-mode-wiring)
PR tip unchanged at b89b4cdeb143be17c7dba09fbe7f0d5065f1b7aa. Ready for human approve/merge.
…coration 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
… 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
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…om 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_01Qm3i3eLCDcwytzge1sKM4aKeep 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>
Review-findings resolve pass ( |
| Finding | Disposition |
|---|---|
.search-band-query still cited in search-chrome-behaviour.md | Fixed — retargeted to .search-band-subject / current weights |
| Placement flips loading→ready for row-mode pages | Fixed — default derives from raw mobileControls |
| Row placement compresses wide select + Sort | Fixed — flex-wrap + basis-full shrink-0 |
Empty-state aria-live mounts populated | Fixed — deferred sr-only announce for query-only |
| Filtered-to-zero double polite announce | Fixed — suppress empty-state live region when filtered (band already spoke) |
| Stale therapy filter-sheet Clear comment | Fixed |
| Find-a-filter needle if field hidden | Fixed — gate on dense/showNeedle |
Inline color-mix idiom | Fixed — --search-band-badge-bg token |
Duplicate forced-colors 3 — numbering | Fixed → 4 — |
| Partial-suffix doubled middot gap | Fixed — nested back into the count phrase |
Style-contract tautological distinct pairs | Fixed — removed; real check stays in ui-accessibility |
| Lead-mark flex-only paint | Fixed — display: block on .search-band-lead |
| Collapse header no-op while searching | Already fixed on tip (7c1861f8) |
| Callback-ref / unlayered inventory notes | Acknowledged — no further code change |
Verification: 94 passed across search-results-header-band, document-filter-panel, therapy-compass-mode-wiring, style-contract-registry.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
`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#1606#1606 is closed, but it carried the one fix nothing else in the queue provides: MobileResultFilterControl's native <select> paints a harsh system-blue highlight on phones, and #1615 keeps that native select (its change is the iOS 16px anti-zoom rule). So the fix does not survive #1615 landing. Records it as #238 with the two defects the redo must not repeat: the unresolved keyboard trap on disabled options, and the set-state-in-effect lint error that PR #1620's new pre-push guard would now catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy
…sign Four conflicts, resolved as the trial merge recorded in the PR body predicted. `search-screen.tsx` reduced to the handler alone, exactly as forecast: both sides had independently arrived at `--text-muted`, so the only disagreement left was `clearSearchFilters` (here) versus `clearSearch` (main). Kept this branch's — #1616 branched before defect #1611's sibling was fixed, and `clearSearch` on a control labelled `Clear` inside a filter row deletes the query the reader is looking at. `search-results-header-band.tsx` (2 hunks) took this branch: main's side is the pre-restructure shelf carrying only the token migration this branch had already applied. `document-search-results.tsx` (3 hunks): the two empty-state hunks took this branch's shared `SearchResultsEmptyState`. The import hunk is a genuine merge — v2 renamed `metadataPill` to `metadataPillDensity`, and the auto-merged body now calls `metadataPillDensity.roomyCompact`, so the import must follow. `EmptyState` is dropped from it because both of v2's call sites are the ones this branch replaced. `docs/outstanding-issues.md`: #1616 minted its own #237 and #238 from the same `next-id`, so both sides landed rows under both numbers. Renumbered *this* branch's to #246 and #247 and bumped the marker to 248, rather than taking one side wholesale — main's rows are already landed and referenced. This is the collision `#156` and `#168` predict; the guard caught it. Two v2 gates then failed on the merged tree and are now satisfied: - `check:design-system-contract` ratchets `textSoftConsumers` per file, and v2 had driven both of these to zero. The four remaining usages here are icons and glyphs, which this branch had deliberately left on `--text-soft` because v1 defined no decoration token. v2 defines `--decoration-soft`, so that constraint is gone and they move there — the tier v2 itself uses for the same nodes. - The generated adoption manifest and COMPONENTS.md maturity section went stale as a result; regenerated with `design-system:adoption:update`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a
Resolves the four expected conflicts. #1616's contribution to the three component files is a token migration plus a rename, so each resolution takes this branch's structure and re-applies #1616's change onto it: - search-screen.tsx: keep clearSearchFilters. Both sides had already converged on --text-muted for the className, so only the handler differed, and clearSearch also wipes the query - the defect #1611 fixed on the shelf's Clear. This branch's comment anticipated the merge and pre-matched the className so exactly this one decision was left. - search-results-header-band.tsx: keep this branch's shelf scroll-track and tap-target work; migrate the Funnel glyph this branch introduced off the deprecated --text-soft to --decoration-soft, matching what #1616 did to the equally decorative ChevronsUpDown. - document-search-results.tsx: keep SearchResultsEmptyState; adopt #1616's metadataPill -> metadataPillDensity rename. The import still said metadataPill while the auto-merged bodies already called metadataPillDensity, so taking either side wholesale would not have compiled. Drops the now-unused EmptyState import and migrates three further decorative --text-soft glyphs. - outstanding-issues.md: take main's next-id=246. Ours said 239, which is below main's highest allocated id and fails check:outstanding-issues. docs/design-system/adoption-manifest.json is regenerated, not hand-edited. document-search-results.tsx drops off EmptyState's adoption lists because this branch replaces EmptyState with the shared SearchResultsEmptyState there - a reviewed change with its own announcement rationale, not a fall back to a local panel. The manifest describes the tree, so it now describes the merged one. Verified on the merged result: tsc --noEmit exit 0 (the import break would have failed here), full unit suite 503 files / 5325 tests after the manifest regeneration, decoration-on-text contract 6 passed, lint clean on all three components. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…tap floor Two sessions resolved the #1616 merge independently. The pushed one (`5c0fe4d`) failed `Static PR checks` on the outstanding-issues guard: #1616 minted its own #237 and #238 from the same `next-id`, and that resolution kept both sides under both numbers. This merge keeps the resolution that renumbers *this* branch's rows to #246/#247 and bumps the marker to 248 — main's are already landed and referenced, so they keep their numbers. The guard now passes: 245 rows, unique ids, next-id above the highest, no ids dropped from base 08595cc. The only other conflict was a duplicated comment block on the therapy quick-filter `Clear`; both sides already agreed on `--text-muted` and `clearSearchFilters`. Also fixes a review finding, which is a real inconsistency this branch introduced: the find-a-filter field shipped at `min-h-10` — 40px — in the very commit that raised the facets, the disclosure headings, the shelf `Clear` and the sheet footer to `min-h-tap` (48px). On the surface that exists for phones, the one control added to make a long filter list usable was the smallest target in the sheet. It now matches the facets exactly (`min-h-tap sm:min-h-9`). `decoration-on-text.contract.test.ts` had pinned that field by slicing 600 characters after its testid, so documenting the line broke the guard. It now walks forward to the element's own `className`, and additionally pins the tap floor so the 40px version cannot come back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…ix review findings Reviewed the full findings list; fixed the ones I agree with and left the rest, with reasons in the PR thread. **Tap floor on the recovery actions.** `emptyStateAction` was 36px. Pre-existing, but it stopped being incidental when this branch made those controls the designated way out: on the documents zero-result and filtered-to-zero paths they are the only escape, and on therapy-compass `Clear search` is the only one for a query-only zero result. Shipping the escape hatch below the floor the same change raised the facets, find field and disclosure headings to contradicts the rule this PR argues for in its own description. Now `min-h-tap sm:min-h-9`, matching the facets, pinned by a test. **Sheet-footer Library rendered a full border, not the intended rule.** `cn` is a plain join, so `floatingControl`'s own `border` survived the added `border-t` and the result was a fully bordered button whose colour depended on Tailwind's emission order between two competing arbitrary utilities — the exact hazard the facet-chip branches in the same file are written to avoid. `border-0 border-t`. **"No matches" asserted while the registry was still loading.** On the services and forms paths `loading` covers only the document search, so this branch rendered "No matches for …" directly beneath a spine reading "Searching…", which derives its status from `recordStatus`. The band's invariant is that a search in flight asserts nothing; the body now holds to it too. **Therapy quick-filter `Clear` was a no-op with no filters active.** Rewiring it to `clearSearchFilters` fixed the label/handler mismatch and created a second one: in the ordinary "typed a query, got results" case the control did nothing at all. Gated on `activeFilterCount > 0`, as the sheet's `Clear all` already is. **Two stale docs**, both the trap #1609 records. `search-chrome-behaviour.md` item 7 still described the deleted full-width `border-top` accent and the old forced-colors values; it now describes the lead mark, `6px double` faulted, and the card's `4px double`. The `globals.css` band banner still claimed 580 as the heaviest weight and that the query and figure share it — both changed. **`SearchXIcon` deleted**: the therapy empty-state swap left it with no consumer, and `check:knip` does not gate unused exports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a
Uh oh!
There was an error while loading. Please reload this page.
* fix(gates): catch lint and type errors before push, not in CI Two open PRs burned full CI cycles this week on defects a single local command would have caught: #1606 on a react-hooks/set-state-in-effect lint error, #1618 on a TS2339 for `mode.devOnly` (a union member that lacks the property, where app-modes.ts already exports the correct `"devOnly" in mode` guard). Neither lint nor typecheck was in the pre-push path. Typecheck could not simply be added, because it was already unusable (outstanding-issues #210). tsconfig.json's `include` carries `.next/types/**/*.ts` and `.next/dev/types/**/*.ts` — gitignored build artifacts — so deleting a page leaves the stale generated validator importing a removed module. Reproduced rather than inferred: a planted `.next/dev/types/validator.ts` referencing a removed mockup page yields `error TS2307: Cannot find module .../mockups/deleted-mockup-route/page.js`, base config exit 2, source-only config exit 0. Full source typecheck is clean (71s cold, 8.8s warm). Red locally and green in CI is how the gate got abandoned, which is how the real type error then reached CI. - tsconfig.typecheck.json + `typecheck:source`: identical compiler options, minus the `.next` globs, with a separate tsbuildinfo so the two incremental caches cannot invalidate each other. Route-signature validation is not lost; `next build` still covers it in CI. - guard-push.mjs gains a fourth guard running eslint over the pushed files and this typecheck. Verified to reproduce both defects above with CI-identical messages. Scoped to the lint roots and to pushes that touch TS, skips loudly when node_modules is absent rather than pushing people to GUARD_PUSH_DISABLE=1, and overridable with SKIP_STATIC_GUARD=1. Also corrects a doc claim that made #1580 surprising: "mockups are exempt" was being read as blanket. Mockups are exempt from the wiring and reachability gates and nothing else — they are still typechecked, and their client chunks still count toward check:bundle-budget, which totals every built chunk rather than the initial production bundle. That the budget's scope contradicts ledger #13's "not an initial production bundle" position is a real unmade decision, now recorded as #237 rather than papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * docs(issues): capture the phone Category soft-menu fix salvaged from PR #1606#1606 is closed, but it carried the one fix nothing else in the queue provides: MobileResultFilterControl's native <select> paints a harsh system-blue highlight on phones, and #1615 keeps that native select (its change is the iOS 16px anti-zoom rule). So the fix does not survive #1615 landing. Records it as #238 with the two defects the redo must not repeat: the unresolved keyboard trap on disabled options, and the set-state-in-effect lint error that PR #1620's new pre-push guard would now catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * issues: capture #239 stale Cloud acceptance pin on PR #1617, #240 remote-container browser gate drift * Tighten guard coordinator test Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): shared source-typecheck lease and safer static pre-push Treat typecheck:source:internal as a shared read-only coordinator lease with a distinct per-worktree buildinfo file, drop the pinned in-repo cache path, and harden staticGuard: acquire a short exclusive lease (fail-open when busy), use a private eslint cache, escalate lint on eslint policy changes, fail closed when the push tip is not HEAD, cover eslint-rules, and add Vitest coverage. Align hook/docs wording with the fourth guard and point CLAUDE.md at #252. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): keep tsBuildInfoFile when run-heavy has no npm_execpath Pre-push invokes run-heavy via plain node, so the npm_execpath spawn path was skipped and the fallback dropped effectiveForwarded — undoing the per-worktree buildinfo injection. Also warn when staticGuard passes on a dirty working tree. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: refresh scripts-index for lint:changed:internal Keep docs:check-inventory green after adding the pre-push eslint wrapper script to package.json. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): address Devin findings on static pre-push guard - Treat "Database focused-test capacity is full" as coordinator busy so shared typecheck slot exhaustion fails open instead of faking a type error. - Skip source typecheck when every changed .ts path is excluded by tsconfig.typecheck.json (edge functions, archive, scratch, worktrees). - Restore check-github-shell-access.mjs (and its Role notes) in the scripts index. * chore(ledger): record PR #1620 babysit * fix(gates): emit structured heavy-run admission-busy signal Prefer exit 75 + DATABASE_HEAVY_RUN_ADMISSION_BUSY over prose matching so tsc/eslint output that quotes busy strings cannot false-pass the static guard. * fix(gates): tip-check only when static work runs; isolate typecheck cache Addresses follow-up Devin on PR #1620: - Reorder staticGuard so tip-vs-HEAD fails closed only when lint/typecheck will actually read the working tree; ignore tag refs in the tip check. - Pin a distinct tsBuildInfoFile on tsconfig.typecheck.json so direct tsc does not collide with the base config cache (run-heavy still overrides). * fix(gates): keep lint failures when typecheck admission is busy Addresses Devin on PR #1620 — a prior eslint failure must still block the push if the follow-up source typecheck cannot get a coordinator slot. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…#1672) * fix(search): stop the results bar clipping its own controls on phones The one-line results band (#1615) left the inline utilities group `shrink`, so an over-subscribed line paid the shortfall out of the *controls* instead of the truncating query. The sort group was severed mid-glyph by the utility track's `overflow-x-auto` and its trailing option was then washed out by the 28px overflow mask, which reads as a rendering fault rather than as "swipe for more". Measured on the live app before the fix: clipped at 320/375/390/393/402 and 430/440px, and by 41.9px at 540px with a four-word query — so this was never bounded by phone width, and the query truncated a 7-character word alongside it. - Inline utilities are `shrink-0`. The band's own contract is that the query is what gives way when the line runs out; `truncate` + `min-w-[2rem]` on the heading already exist to absorb exactly this. - Below 414px one line cannot hold count + query + sort + filter even with the query fully truncated, so the utilities take their own full-width row rather than overflow a band that is `overflow-hidden` and would clip the pinned Filter. - Sort options drop to `px-2.5` below `sm`, returning the 8px that clears the one-line widths. Tap height is untouched. - The Filter wordmark now hides only across 414–429px, the one genuinely single-line-and-short band, instead of everywhere below 430px. Also fixes the Filter trigger reading as a different component from the sort group it sits flush against. It composed `floatingControl` plus an override string, and `cn` is a plain join, not tailwind-merge — so `font-semibold` (600 against the sort group's 470/560) and `--border-lux` (a visibly darker stroke than `--border`) both reached the DOM and stylesheet order picked the winner. It now uses the band's own control recipe, with the active and resting colours as mutually exclusive branches. Gates: `ui-smoke` sweeps the rail across 320–540px as geometry, not as a class — verified to fail (`sortClipped: 16, masked: true` at 414px) with `shrink` reinstated, and to pass with the fix. A single-width check at the existing 390px viewport passes against the broken code, which is why the sweep crosses 414px. `expectNoPageHorizontalOverflow` cannot see an internal scroller, which is how this shipped green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BkKbgdhhAg8REq2wudR8V * docs(ledger): record the results-band phone clipping fix (PR #1672) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BkKbgdhhAg8REq2wudR8V * test(search): pin parent wrap + on-screen utilities for results-band fix The shrink-0 / max-[413px] child checks were green even if the parent lost max-[413px]:flex-wrap, and the ui-smoke rail-fit sweep could not see wrap failure when overflow-hidden clipped the utilities off-screen. Assert the parent wrap class in DOM, and require the utilities group to stay in the viewport below 414px. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>


Summary
Implements the published results-bar redesign.
SearchResultsHeaderBandis the shared header above every result list and 12 of the 13 app modes render it, so this lands everywhere at once.46476a3— the applied-filter shelf and the sheet's dead ends (F3, F4, F5, F10). The shelf was a singleoverflow-x-autorow, so four or five chips pushed its only global action past the right edge; chips now scroll in an inner track withClearpinned outside it. Zero-count facets in the documents sheet usedopacity-50, which multiplies against an already-muted foreground and measured 2.34:1; they now use a real muted pair plus a dashed border, which also survives forced colors where opacity does not. Also rewires therapy-compass's quick-filterCleartoclearSearchFilters— it sat among the filter chips and wiped the query, the sibling of the defect Fix the results-band shelf Clear deleting the search query #1611 fixed on the shelf.0cec1d6— the filtered-to-zero empty state (F11).Search results bar: delete the inert scope system, rebuild the filter shelf, and land the bar anatomy #1555 made filters real and deleted the only copy pointing at relaxing them, so a reader filtered to zero was offered an unrelated example query and a different mode, never the chips above that caused it.SearchResultsEmptyStatenow takes the sameappliedFiltersthe shelf renders and leads with Remove "X" and Clear all filters.4f213cf— the documents filter sheet (F7, F8, F9, F12). Facets were 28px targets packed atgap-1.5on the surface that exists for phones; they are now on the tap floor. Source type becomes a joined segmented control with aone onlyhint. Adds a find-a-filter field and collapse-by-default with selected counts, both gated on the same density threshold. Library moves off the utility rail into the sheet footer, plus a fallback control for the states that have no sheet.aa8bfc2— the one-line anatomy (F1, F2). The band was 123px on a phone to say "12 documents". It is now 58px (60px fromsm). The state tile is deleted and the full-width accent border becomes a 2 × 18px lead rule inside the padding. Filter and Retry are pinned outside the scroll track and can no longer be scrolled away.a06afd3,7eb723b— the fallout the gates caught.verify:uicaught what nothing offline could:ui-smokepinned Library to the ribbon and then used that same button to open the Sources drawer. Both assertions are re-pointed rather than deleted, with the ribbon one inverted into an absence. That test also exposed a real defect — the footer control opened the drawer underneath the still-open filter sheet.c2b174f,efc8890— v2 design-system readiness. See Design-system conformance.7c1861f— two review findings, both real, both mine. Library was unreachable from a services or forms search that matched records but no documents: that render returnednull, and Library's three homes all miss it (the sheet footer needsmatches.length > 0, the empty state needsrecordMatchCount === 0). And a filter-group heading reported a collapse it did not perform —isOpenis forced true while the find field has text, but the button stayed mounted and wrote the group intocollapsed, so the collapse ambushed the reader once the field cleared.Where this deviates from the mockup, deliberately
docs/search-results-bar-decisions.md. Faulted keeps three independent non-chromatic channels: the lead rule doubles from one stroke to two, aCircleAlertrenders for non-readystates only, and a faulted band renders no digit at all. The mark is aborder-lefton a zero-width box rather than a background precisely so forced colors, which drops backgrounds but preserves border-style, keeps all of it.min-h-11reintroduces a fixedui-smokesub-pixel flake and--spacing-tapis this repo's floor.MobileResultFilterControl— aw-fullnative select, and formulation and specifiers pass two — which is unreadable pinned into a 58px line at 320px.mobileControlsPlacementdefaults torowwhenever a page passes a phone control, so a new mode that forgets the prop degrades to today's layout rather than to an unusable one.Design-system conformance and readiness for #1616
Reviewed against
docs/design-system/and against the open v2 activation PR #1616, which touches four of this branch's files.One real defect, found and fixed.
--text-softis a deprecated alias of--decoration-soft, andtests/ckb-v2-token-contract.test.tspins it below 4.5:1 deliberately — GATES §1 words it as "so the tier cannot be 'fixed' away". TOKENS §7 forbids it on any text node. This branch had introduced nine new text-node usages:--surface-subtleopacity-50, before this branch--text-soft(as originally pushed)--text-muted(now)That middle row is the point: the accessibility fix this PR exists to make would have silently reverted — below even the 3:1 non-text floor — the moment #1616 activated the v2 layer, with nothing reporting it. Icons and glyphs keep
--text-soft— that is what the tier is for, and v1 defines no--decoration-softto move them to.tests/decoration-on-text.contract.test.tsgains four cases pinning the tier per surface, verified to fail on the exact regression and pass when restored. A DOM assertion cannot do this job: jsdom sees the class rather than the resolved colour and would pass on either token.Everything else conforms.
check:design-system-contractpasses (653 production files; raw colors 2, all pre-existing exemptions; literal shadows 0; legacy tap classes 0).check:type-scaleandcheck:icon-scalepass.Collision with #1616, for whoever merges second. Six shared files;
globals.css,ui-smoke.spec.ts,therapy-compass-mode-wiring.test.tsand the ledger merge cleanly. Three conflict, at six hunks —document-search-results.tsx(3),search-results-header-band.tsx(2),search-screen.tsx(1). The conflicts are textual, not architectural: #1616's change to these files is a mechanical--text-soft→--text-mutedmigration, while this branch restructures. Matching v2's token choice on the two lines it also edits means those now agree from both sides, and the therapy hunk narrows to the handler alone (clearSearchFiltershere,clearSearchon v2's base — take this branch's; that is defect #1611's sibling). Measured by trial merge, not inferred.Verification
Head
8ea21c0onorigin/main@9d4a28c, 32 commits, 17 files.npm run verify:ui—349 passed (10.7m), exit 0. The full Chromium suite on this exact head.npm run verify:pr-local— exit 0. The CI PR mirror, including build, client-bundle scan and RAG fixture validation.npm run lint— exit 0npx tsc --noEmit— cleancheck:design-system-contract,check:type-scale,check:icon-scale— passprettier --write, committedPR requiredincluded:Production UI critical,Production UI (1)(2)(3),Unit coverage,Build,Static PR checks,Safety and config checks,Visual baselines,Lighthouse,PR policy,PR mergeability,Semgrep×2,Gitleaks,GitGuardianOne CI flake, classified rather than quarantined.
Production UI (2)failed once onui-specifiers.spec.ts:152(/specifiers/builder, clicking a checkbox after aselectOptionre-render). It is not this diff: the branch touches zero specifiers files, the builder rendersSpecifierBuilderPagerather than the band, and the same test passes locally on this exact head in 659 ms where CI retried 24× over 11.3 s. The failed jobs were re-run and passed first time. Perdocs/testing.mdthat is one reproduction, so it correctly does not entertests/flake-ledger.json.Measured in a real browser at 320 / 390 / 430 / 768 / 1024 / 1440, light, dark and forced-colors: lead rule
2px solid rgb(29, 111, 184)(=--clinical-accent) at 18px, 20px under forced colors, andscrollWidth > clientWidthfalse at every width including 320.Not run, with reasons.
eval:*,verify:release,check:supabase-project,test:live— no retrieval, ranking or provider surface is touched.On the Clinical Governance Preflight. This repo's own gate is
scripts/pr-policy.mjs, andclassifyPullRequestFilesreturnsclinicalRisk: false,operationalRisk: false,ragRanking: false,ui: truefor the changed files. The preflight section and aRAG impact:line are therefore not required andpr-policywill not block on their absence — confirmed green. The change is presentational. Flagging this explicitly because an automated reviewer asks for the section on keyword match rather than on the classifier.Toolchain note. The container's installed tree did not match
package-lock.json(playwright1.62.0 vs locked 1.62.1,jsdom29.1.1 vs locked 30.0.1), socheck:installed-lock-parityfailed — and bothverify:uiandverify:pr-localrun it first, so either would have reported success having executed nothing. Repaired with Node 24.19.0 +npm ci --include=devbefore any gate was trusted. Environment only; no manifest or lockfile changes in this PR.Concurrent work on this branch
Several review bots pushed fixes directly to the branch (~20 commits). Five of those pushes shipped breakage reported as verified:
document-search-results.tsx— anonClickprop closed its callback but not the JSX expression, so the file did not parse at all;ui-accessibility.spec.ts—/\\/api\\/search/terminates at the first\\/;role="status"collision — the empty-state announcement fix gave it a status role, but the band renders one unconditionally on every search route, sogetByRole("status")became ambiguous across all twelve modes;d351b5f) — review advice to intercept/api/searchapplied without running the spec, but/differentialsfetches/api/differentials?kind=…, so nothing faulted and twoProduction UIshards went red waiting on a panel that could not render;react-hooks/set-state-in-effectlint error (788b664) —Static PR checkswent red, and nothing offline caught it because lint is in neither the unit suite nortsc. Fixed in8ea21c0.Two late fixes were mine to catch: adopting the shared empty state silently demoted the documents zero-result title from the
h3that #1612 had just given it back to a<p>, and the empty state's body told every mode to "try an example, or jump to another mode" when therapy-compass has no entry insearchCommandSurfaceByModeand renders neither control.Risk and rollout
ui-accessibilitytest plus the retargeted style-effect contract are what catch it.aa8bfc2alone restores the two-row anatomy without giving back the reachability or contrast fixes. The later bot commits are interleaved and no longer cleanly separable, so a full revert of the branch is the safer undo if one is needed after merge.Notes
Two pinned tests were rewritten rather than deleted, because both catch real regressions that would otherwise be silent:
tests/ui-accessibility.spec.tsasserted the band's computedborder-top-widthwas2px. The lead rule fails that by design. The rewrite asserts the mark's width, style, height and resolved colour, adds the fault-shape assertion the border version could not make, and asserts its own probe returned real values — a silently-null measurement is exactly the kind of evidence that reads as a pass.tests/helpers/style-contracts.tsis retargeted fromsearch-bandtosearch-band-lead, scoped to[data-tone="accent"]. Its same-elementdistinctpairs were removed as tautological: the selector targets the mark, which has no top border, so comparing its own left and top widths was2pxvs0px. The real cross-element check lives inui-accessibility.spec.ts.Two follow-ups are recorded in
docs/outstanding-issues.mdrather than fixed here:#237— a comment claimedrefetchingwas dimmed "via CSSdata-status", but no[data-status="refetching"]rule exists inglobals.css. Adding it is a visual change across twelve modes and belongs to whoever decides a background refresh should look different; the comment is corrected so it stops asserting something untrue.#238— widening the one-line bar to the six modes that pass a full-width phone select, which is per-mode work rather than a shared-band change.