feat(ds): give EmptyState an optional heading level, and adopt the two states that need one - #1612
Merged
Merged
Conversation
Production triage 2026-08-04 measured the live failure: OpenAI generation overruns OPENAI_ANSWER_TIMEOUT_MS and the answer degrades to source-only, while retrieval passes its gate with 12 candidates across 5 documents. The 2026-08-04 handover's four ranked hypotheses (query embedding, Supabase RPC, source governance, corpus) are all ruled out by that evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…at need one EmptyState rendered its title as a <p>, so any empty state that owns a region of the page could not adopt it without losing its heading. /dsm/search lost exactly that when it adopted the primitive (#224), and the main document-search empty state stayed hand-rolled because ui-smoke pins a heading role on it (#217). Add an opt-in headingLevel prop. It is deliberately un-defaulted: most adopted call sites sit inside a card that already owns its region's heading, and promoting every title would inject an outline level the page never declared. - /dsm/search passes headingLevel={2}, restoring the heading it had before. - The document-search empty state adopts EmptyState with headingLevel={3}, which also converts it from a bespoke centred card to the shared state surface and gives it the polite live region every other adopted empty state has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… timeout The first row named OPENAI_ANSWER_TIMEOUT_MS as the constraint. It is not, and production does not set it. The query routes fast, so answerRouteBudgetMs.fast (25s) is the ceiling and generationRequestTimeoutMs holds back the 2s recovery reserve on top; the measured 24.7s is that budget being spent. Also records the half with clinical consequence: answerRouteResultCanBeCached excludes a route-deadline-exceeded answer but not one degraded by the OpenAI request timing out, so a generation_fallback answer can be cached and re-served after the provider recovers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesEmptyState heading semantics
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SearchPage
participant EmptyState
participant DOMTest
SearchPage->>EmptyState: render search empty state
EmptyState->>DOMTest: output configured title element
DOMTest->>DOMTest: verify visible level-two heading
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:5c1c1b32c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 4, 2026
OwnerAuthor
@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/ds-v2-empty-state-heading at starting commit 5c1c1b3; 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/ds-v2-empty-state-heading, 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. |
Codex Review: Didn't find any major issues. Keep it up! 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". |
These came out of the DS V2 wave with no ledger row and would otherwise have evaporated with the session: the stale PR-J governance review record, the COMPONENTS.md section 0 drift plus the undocumented optionality-marker contract change, the undocumented answer-copy-payload module, the missing ADOPTION.md section 7 proof shots, and the two orphan builder branches and leftover dev servers. The sixth residual, visual baselines staying zero and advisory, is already ledger #118 and was deliberately not duplicated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo
commented
Aug 4, 2026
OwnerAuthor
@cursoragent run bugbot and ensure its findings are correct Please also fix any issues, address and fix any comments and resolve once done. Ensure no regression and it makes sense according to PR for any fix in context and also fix failing CI please. |
Contributor
PR #1612 is merged (squash auto-merge landed). Bugbot/review/CI work is done. BugbotNo Findings addressed
CI
Local proof
  |
PR #1612 added the opt-in headingLevel prop and adopted the DSM and document-search empty states that needed it. Close both ledger rows so /issues stops treating finished work as outstanding.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the design-system EmptyState primitive to optionally render its title as a semantic heading (opt-in), then adopts that capability in the DSM search and document-search “no matches” empty states, with DOM tests to pin the heading contract. It also includes ledger updates in docs/.
Changes:
- Add an opt-in
headingLevel?: 2 | 3 | 4 | 5 | 6prop toEmptyState, rendering the title ash2–h6when provided. - Adopt
headingLevelfor the/dsm/searchno-matches state and the document-search no-matches state. - Add/extend DOM tests that pin the default (paragraph) and heading semantics; append new rows to the outstanding-issues and branch-review ledgers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/ui-primitives.tsx | Adds headingLevel prop and renders the title as p or h2–h6 accordingly. |
| src/components/dsm/dsm-search-page.tsx | Restores DSM no-matches empty-state heading semantics via headingLevel={2}. |
| src/components/clinical-dashboard/document-search-results.tsx | Replaces a bespoke no-matches card with EmptyState and supplies headingLevel={3}. |
| tests/ui-primitives.dom.test.tsx | Pins the default paragraph title and the heading promotion behavior (including role=status preservation). |
| tests/dsm-search-empty-state.dom.test.tsx | Adds a focused jsdom test ensuring the DSM empty state exposes an h2 heading. |
| docs/outstanding-issues.md | Bumps issues:next-id and appends new outstanding-issues rows. |
| docs/branch-review-ledger.md | Appends a branch review ledger entry for this PR branch/head. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Review follow-up (description clarity): GitHub permissions here cannot edit the PR body, so recording the Copilot clarification in-thread: The append-only ledger scope on this PR is intentional bundling (not accidental):
No product-code change for the ledger commits. Please treat the Summary bullet that only named |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 617: Update the ledger entry for claude/ds-v2-empty-state-heading to
replace the bare “verify:pr-local exit 0” note with the decisive final summary
or pass line emitted by the selected verify:pr-local steps, while preserving the
recorded exit status and other verification results.
In `@docs/outstanding-issues.md`:
- Line 248: The `#230` issue title incorrectly identifies OPENAI_ANSWER_TIMEOUT_MS
as the binding timeout. Update the title in the outstanding-issues ledger to
name the fast-route answerRouteBudgetMs.fast = 25000 ceiling instead, while
preserving the existing issue details.
- Line 248: Update the issue entry in docs/outstanding-issues.md so the
src/lib/rag/** glob is enclosed in backticks wherever it appears in the
referenced sentence, preventing markdown emphasis parsing while preserving the
surrounding instruction.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4bda6e5-3951-49d9-bf40-a55190405098
📒 Files selected for processing (7)
docs/branch-review-ledger.mddocs/outstanding-issues.mdsrc/components/clinical-dashboard/document-search-results.tsxsrc/components/dsm/dsm-search-page.tsxsrc/components/ui-primitives.tsxtests/dsm-search-empty-state.dom.test.tsxtests/ui-primitives.dom.test.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Merge conflict resolved (
|
The detail already established that OPENAI_ANSWER_TIMEOUT_MS is not the binding ceiling — answerRouteBudgetMs.fast = 25000 is. Align the summary so triage does not raise a non-binding setting, and backtick the src/lib/rag/** path so markdownlint MD037 stays quiet.
BigSimmo
enabled auto-merge (squash)
August 4, 2026 15:20
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8018 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Production UI (1) failed once on ui-phone-scroll-page-owned:577 (Services results re-settle after viewport shrink) — same signature as the 2026-07-31 flake noted in the branch-review ledger. Unrelated to EmptyState headingLevel; shards 2/3 and critical passed. Cannot rerun-failed via this token (403), so retrigger the pull_request run.
BigSimmo pushed a commit
that referenced
this pull request
Aug 5, 2026
Adopting the shared empty state demoted the main document-search zero-result title from `h3` back to a paragraph, silently undoing #1612 — the release that gave `EmptyState` an optional heading level precisely so the two states owning their region could keep one. Nothing offline noticed; the only signal was `ui-smoke`'s `@critical` journey going red on `getByRole("heading", { name: "No matching documents" })`. `SearchResultsEmptyState` now takes the same opt-in, un-defaulted `headingLevel` as `EmptyState`, for the same reason: most of the twelve modes rendering it sit inside a region whose heading the band already owns, so promoting every title would insert an outline level the page never declared. Documents passes `3` at the call site that owns its region; the inline filtered-to-zero state inside the results grid stays a paragraph. The Playwright assertion follows the new copy but keeps asserting the role and level, and `document-search-record-fault` now pins the same level so the fast gate catches this next time instead of a Chromium journey. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a
9 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 5, 2026
* fix(ui): make the applied-filter shelf reachable and its dead ends readable Four review findings land on one row and interact. F4 — the shelf was a single `overflow-x-auto` row: label, chips, a `flex-1` spacer, then `Clear`. With four or five chips on a phone the spacer collapses and `Clear` sits past the right edge, reachable only by swiping a row whose scrollbar is hidden. That is the same defect the shelf was built to avoid for the chips themselves. The chips now scroll in an inner track and `Clear` is pinned outside it. F5 — the rail fades its edge on overflow and the shelf did not, so a sixth chip simply stopped existing visually. The chip track now carries the same mask on the same overflow condition, via a second instance of the existing `useRailOverflow` hook. F3 — `Clear` was `px-2 py-1`, about 26px beside 48px chips: the row's only global action was also its hardest target. Matched to the chips instead, staying quiet through weight and an underline. 48px rather than the 44px generic tap guidance suggests, because `min-h-11` reintroduces a fixed `ui-smoke` sub-pixel flake and `--spacing-tap` is this repo's floor. F10 — zero-count facets in the documents sheet were dimmed with `opacity-50`, which multiplies against an already-muted foreground and lands at 2.34:1. The disabled state was least readable exactly when it most needed explaining. Replaced with a real muted pair plus a dashed border: 4.72:1, and it survives forced colors, where border-style is preserved and opacity is not. The three facet states are now mutually exclusive branches rather than a base plus an override — `cn` is a plain join, so competing `border-[color:…]` utilities would both reach the DOM and stylesheet order, not intent, would pick the winner. Also rewires the therapy-compass quick-filter row's `Clear` to `clearSearchFilters`. It sits among the filter chips and is labelled `Clear`, but it called `clearSearch`, which wipes the query with them — the sibling of the defect #1611 fixed on the shelf, missed when that was reviewed. The sheet's `Clear all` is deliberately a full reset and is untouched. Shelf label becomes a funnel glyph below `sm` and the wordmark from `sm`: a prefixed chip costs ~215px of a 350px bar, so every character the label spends is a chip the reader cannot see. The group keeps `aria-label="Applied filters"` either way, so both forms are decorative. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * feat(ui): give the filtered-to-zero empty state its relax-the-filter route back F11. The release that made filters real (#1555) is also the one that removed the only copy pointing at relaxing them. The old empty state read "Relax the scope, try an example, or jump to another mode" with a `Clear scope filters (N)` button; both were dead — they keyed off the inert scope system and never rendered — so the change deleted them and left "Try an example, or jump to another mode." A reader who has filtered to zero is offered an unrelated example query and a different mode, never the chips sitting directly above that caused it. `SearchResultsEmptyState` now takes the same `appliedFilters` the shelf renders, plus `onClearFilters` and `onBrowseAll`. When the set is non-empty it leads with `Remove "X"` and `Clear all filters`, and demotes the example and cross-mode routes below a rule — an example query is a different search, and the reader has not finished this one. With nothing applied the current copy is already correct and is untouched. Two things it deliberately does not claim. The heading counts the filters rather than quoting the query, because the query is not what emptied the set and saying so sends the reader to rewrite the one thing that was working. And `Remove "X"` names the last chip without calling it the most recent: `appliedFilters` arrives in group order, not application order, so that would be a claim the data cannot support. Documents and therapy-compass rendered their own bare `EmptyState` for this case — naming the problem while offering no route out of it — so both now use the shared surface. Documents also passes Browse, because when narrowing this result set is not the answer, reaching the whole corpus is. This retires the last `clearSearch` mislabel on therapy-compass. Its empty state had one button labelled `Clear filters` wired to `clearSearch`, which wiped the query too; `Remove "X"` and `Clear all filters` are now separate controls, so each label matches its own action. The guard added in the previous commit tightens to assert exactly one full reset survives that screen — the sheet's `Clear all`, the only control whose label says it clears everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * feat(ui): rebuild the documents filter sheet around eleven groups F7 — facet buttons were `min-h-7`: 28px targets, packed at `gap-1.5`, on the surface that exists for phones and whose only interactive elements they are. Raised to the tap token, relaxing to 36px from `sm` and 32px from `lg` where a pointer is likely. 48px rather than the 44px the design called for, because `min-h-11` reintroduces a fixed `ui-smoke` sub-pixel flake and `--spacing-tap` is this repo's floor; the sheet has the vertical room. F8 — source type is a `radiogroup` and the facets below are `aria-pressed` toggles, but both rendered as chips of near-identical size, colour and radius, directly adjacent. Nothing said one row replaces and the next accumulates, so the OR-within-group, AND-across-groups model had to be found by experiment. Source type is now a joined segmented control, which reads as one-of on sight, with a `one only` hint for the first time it is seen. The ARIA is unchanged — this is presentation. F9 — eleven groups stacked in one phone column with no collapsing and no search, so reaching Document type meant scrolling past ten sections. Adds a find-a-filter field and collapses groups by default, each carrying its selected count. Both are gated on the same density threshold: a sheet showing two groups that are both shut is a scroll saved that never existed and two taps added that did, so below four groups everything stays open and the heading is not a disclosure control at all. A group holding a selection cannot be collapsed — a closed section silently narrowing the list is worse than the scroll it saves. F12 — Library leaves the utility rail. It sat adjacent to Filter while answering a different question, and it occupied the rail space the pinned Filter needs; it is also the reason the phone rail could overflow at all. It is moved, not removed: the requirement the old comment protected still holds, since the documents action menu routes through `onSearchModeChange`, which clears the query. Both new homes preserve it — the sheet footer under a rule, phrased as reach with the corpus count beside it, and the zero-result state. Also drops the footer's duplicate count. It printed "12 documents" beside "Show 12 documents", spending the sheet's most prominent slot on a number two centimetres to its left; the button carries it, the new meter and readout carry the proportion, and the live announcement moves to `sr-only` so it still speaks as the number changes underneath. The header's `Clear all` becomes `Clear filters`, matching a handler that was already filter-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * feat(ui): collapse the results bar to one line and pin what must not scroll away The band was 123px on a phone to say "12 documents". The utility rail dropped to its own row and that row was ~85% empty, so the height was spent on the layout rather than on anything the reader needed. It is now 58px, 60px from `sm`. F1 — the rationale for moving Filter last was that the right edge is where a thumb already rests, but the spacer that pushed the rail right was `hidden lg:block lg:flex-1`, so it did nothing below 1024px. On every phone the rail was left-packed and Filter sat mid-rail: the change delivered its stated benefit only at the width where thumbs are least relevant. The query now takes the flexible space and the control group is edge-aligned at every width, with no conditional spacer. `mr-auto` does it rather than moving `lg:flex-1` down — auto margins only absorb what is left after flexible lengths resolve, so the wide layout is byte-identical and no breakpoint had to move. F2 — the rail was one `overflow-x-auto` region with Filter as its last child, so the only control carrying filter state was the first to fall off the right edge once a Retry button or a longer sort label appeared. The code's own comment explained that applied-filter chips had been moved out of that rail for exactly this reason. Only the optional controls scroll now; Filter and Retry are pinned siblings outside the track. Retry too, because it is the recovery action in a degraded state — the one control that must never need a horizontal swipe. The state tile is deleted and the full-width accent border becomes a 2 x 18px lead rule inside the padding: at bar height a line across the whole width read as a divider between the composer and the results rather than as the band's accent. That deletion is where the design as drawn had to be extended. The tile carried state as SHAPE — alert when faulted, spinner while running, funnel once filtered — and the mockup replaced it with colour alone, which makes a failed search identical to a successful one for a reader who cannot separate the hues, and contradicts a recorded decision. The tile turned out to carry three jobs and only one needed a tile. Narrowed is now carried by the shelf, which grows the band by a whole labelled row. Running was already inline. Faulted keeps three independent non-chromatic channels: the lead rule doubles from one stroke to two, a CircleAlert renders before the count for non-ready states only, and a faulted band still renders no digit at all. The mark is a `border-left` on a zero-width box rather than a background precisely so that forced colors, which drops backgrounds but maps border colour to CanvasText and preserves border-style, keeps all of it. F12 — Library leaves the rail (moved in the previous commit), which is what makes the arithmetic work: at 390px count + query + Library + Filter does not fit, and without Library it fits comfortably. One line is not safe for every mode, and the mockup was drawn for documents. Six modes pass `MobileResultFilterControl` into `mobileControls` — a `w-full` native select, and formulation and specifiers pass two in a two-column grid — which is unreadable pinned into a 58px line at 320px. `mobileControlsPlacement` therefore defaults to `row` whenever a page passes a phone control and to `inline` when it passes none; documents and therapy-compass opt in explicitly. The default is the safe one so a new mode that forgets the prop degrades to today's layout rather than to an unusable one. Verified at 320/390/430/768/1024/1440 in light, dark and forced-colors with no page horizontal overflow at any of them. `tests/ui-accessibility.spec.ts` is rewritten, not deleted: it caught the accent degrading to a neutral border when Tailwind's utilities layer outranked the component layer, which has actually happened here, and the same failure now shows up as a zero-width box. It gains the fault assertion the border-top version could not make — a border has no style to change — and a guard that its own probe returned real values, because a silently-null measurement reads as a pass. The style-effect contract in `tests/helpers/style-contracts.ts` is retargeted from `search-band` to `search-band-lead` for the same reason. `docs/search-results-bar-decisions.md` is amended in this commit, since two of its records go stale here: the state tile, and Library staying in the bar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * docs(ui): correct two comments the redesign left describing the old band `search-band` no longer paints the accent, and the placement note referenced a variable name that never shipped. Also drops the claim that `refetching` is dimmed "via CSS `data-status`": no such rule exists in globals.css and there is no evidence one ever did, so the sentence described an intention rather than the code. Adding the rule is a visual change across twelve modes and belongs to whoever decides a background refresh should look different — asserting it here while it does not exist is what let it go unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * docs(issues): capture the two follow-ups the results-bar redesign surfaced #237 — the band claimed a `refetching` dimming rule that globals.css has never defined, so a background refresh is signalled by a pulsing dot alone against text that is deliberately identical to `ready`. Corrected the comment in the redesign rather than adding the rule, because adding it is a visual change across twelve modes. #238 — the one-line bar currently reaches two modes plus every mode that passes no phone control. Six pass a `w-full` native select (two of them pass a pair), which is unreadable pinned into a 58px line at 320px, so they keep their own row by default. Widening them is per-mode work, not a shared-band change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): reach the corpus from the sheet, and re-point the test that pinned it Fallout from moving Library off the utility rail, caught by `verify:ui` (`348 passed, 1 failed`) rather than by anything offline. `ui-smoke.spec.ts` asserted the ribbon still carried an "Open source library" button, then used that same button further down to open the Sources drawer and check focus returned to it on Escape. Both are re-pointed at Library's new home in the filter sheet's footer rather than deleted — the ribbon assertion is inverted into an absence, so putting Library back on the rail re-creates the overflow F12 removed and fails loudly instead of passing quietly. The behaviour fix the test exposed: the footer control called `onOpenLibrary` without dismissing the sheet, so the Sources drawer opened underneath a filter panel still covering the results both of them describe. Browsing the corpus is leaving the filter surface, not another thing to do on it, so it now closes the sheet on the way out. That in turn moves where focus lands when the drawer closes: the opener has unmounted with the sheet, so the app falls back to the documents options button. Asserted explicitly, with the reasoning, rather than loosened to "not body" — it is a visible related control in the same workspace and the app's existing restore target, but it is a fallback and the comment says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * docs(ledger): record the results-bar redesign review at 7eb723b Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): keep document filter sheet chrome scoped to the active query Reset the find-a-filter needle and expand set when the search query changes, and keep selected facets visible while the find field narrows the list so an active constraint cannot become unreachable inside the sheet. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: restore empty-state recovery and rail observation * fix: restore therapy zero-result recovery * fix: keep document filter recovery paths reachable * test: cover announced empty-state recovery * test: cover document filter recovery paths * test: intercept the differential search endpoint * fix: close filter disclosure click handler * fix: use a valid exact search route regex * style: format empty-state regression test * style: format document-search-results.tsx * fix(ui): observe overflow without effect state writes * chore(ui): remove obsolete empty-state import * fix(ui): stop the empty state claiming a second status region The auto-fix for Devin's announcement finding gave `SearchResultsEmptyState` a `role="status"` root. The band already renders one unconditionally on every search route, so `getByRole("status")` became ambiguous everywhere the empty state can appear — which is every mode. Devin's own prompt flagged the risk ("checking it does not collide with the band's own single-status-region assertions"); the collision landed anyway and broke three suites. A bare `aria-live="polite"` announces identically — `role="status"` is just implicit polite + atomic — without adding a node to the role query. `aria-atomic` is deliberately left off: heading and body change together, and re-reading the whole panel on every keystroke is worse than reading what changed. Also repairs three tests that were asserting the wrong things: - The therapy guard counted `b.clearSearch` call sites and required exactly one. An agent then added a correctly-labelled `Clear search` recovery, and the count read it as a regression. The rule was never about head-count — it is that a control wired to `clearSearch` must be *labelled* for clearing the search. Rewritten to assert that, so `onClear`/`onClearSearch` pass and an `onClearFilters` or bare `onClick` still fails. A duplicate copy of the same test carrying the old assertion is removed. - `states the proportion once…` expected `Show 0 documents` from selecting Clozapine + Suicide. The panel deliberately prevents that: once Clozapine narrows the set, Suicide re-counts to 0, becomes a dead end, and dead ends are click-guarded. The test read the feature as a bug. It now asserts the guard — `aria-disabled`, the description, and that the guarded click leaves the count alone. - Two `getByRole("status")` queries in the empty-state tests are now by text, matching the role change above. Two failures in `document-search-record-fault.dom.test.tsx` are untouched here and are not from this commit — verified failing with these changes stashed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * test: follow the zero-match state onto the shared empty state `document-search-record-fault.dom.test.tsx` still expected "No matching documents". That branch adopted `SearchResultsEmptyState` so Library stays reachable from a search that returned nothing — the gap Codex and Devin both raised — which changed the heading to the shared "No matches for <query>". Re-pointed rather than loosened: the assertions still name the exact copy and still distinguish the loading state from the settled one, so a state that stops rendering its heading fails here the way it did before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): keep the documents zero-result title a heading Adopting the shared empty state demoted the main document-search zero-result title from `h3` back to a paragraph, silently undoing #1612 — the release that gave `EmptyState` an optional heading level precisely so the two states owning their region could keep one. Nothing offline noticed; the only signal was `ui-smoke`'s `@critical` journey going red on `getByRole("heading", { name: "No matching documents" })`. `SearchResultsEmptyState` now takes the same opt-in, un-defaulted `headingLevel` as `EmptyState`, for the same reason: most of the twelve modes rendering it sit inside a region whose heading the band already owns, so promoting every title would insert an outline level the page never declared. Documents passes `3` at the call site that owns its region; the inline filtered-to-zero state inside the results grid stays a paragraph. The Playwright assertion follows the new copy but keeps asserting the role and level, and `document-search-record-fault` now pins the same level so the fast gate catches this next time instead of a Chromium journey. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(test): restore the differentials fault interception, and match empty-state copy to its controls Three review findings, one of which was a live CI failure caused by acting on a fourth without running it. `ui-accessibility`'s two fault tests were switched from intercepting `/api/differentials` to `/api/search` on review advice. That page's catalog hook fetches `/api/differentials?kind=diagnosis|presentation` and never touches `/api/search`, so the interception faulted nothing, the band stayed healthy, and both tests hung waiting for a fault panel that could not render — two red `Production UI` shards. Restored, with the endpoint named in a comment so the swap is not made again. The empty state's body told every reader to "try an example, or jump to another mode". `searchCommandSurfaceByMode` is a `Partial<Record<…>>` with no therapy-compass entry, so on that mode the panel renders neither control and the copy named two affordances that were not there. The body is now derived from what the panel actually offers. Also drops a hardcoded neutral colour literal from the lead-rule assertion — the line above it already compares against the probed neutral — and scopes the style-effect contract's selector to `[data-tone="accent"]`, so a degraded live result cannot fail a contract that exists to catch cascade regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): move the new search-surface text nodes off the deprecated decoration tier Readying this change for the v2 design system (#1616), which activates the canonical token layer across 224 files and touches four of the same files. `--text-soft` is a deprecated alias of `--decoration-soft`, and `ckb-v2-token-contract` pins it *below* 4.5:1 deliberately, "so the tier cannot be fixed away". TOKENS.md §7 forbids it on any text node. This branch added eight new text-node usages, which under the v1 palette measure a comfortable 4.72:1 and look correct — and under the v2 palette measure 2.99:1. The dead-end facet is the one that matters. Its entire reason to exist is that the previous `opacity-50` treatment measured 2.34:1; putting the replacement on the decoration tier meant the fix held only until the v2 layer activated, at which point the same markup would have been worse than the AA floor again with nothing reporting it. dead-end facet copy on --surface-subtle opacity-50, before this branch 2.34:1 --text-soft v1 / v2 4.72:1 / 2.99:1 --text-muted v1 / v2 7.29:1 / 5.99:1 (7.54:1 v2 dark) Counts, the shelf label, the shelf Clear and the disabled-facet copy move to `--text-muted`; the find-a-filter placeholder moves to `--text-placeholder`, which is the role the existing recipe contract already requires. Icons and glyphs keep `--text-soft` — that is what the tier is for, and v1 defines no `--decoration-soft` to move them to. Two of these lines are the exact lines #1616 migrates, so both sides now make the same change and those hunks merge instead of conflicting. `decoration-on-text.contract.test.ts` gains four cases pinning the tier per surface, verified to fail on the regression and pass when restored. A DOM assertion cannot do this job: jsdom sees the class, not the resolved colour, so it would pass on either token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): move the therapy quick-filter Clear label off the decoration tier The last text node on this branch still using `--text-soft`. It is a button label, so under the v2 palette it measures 2.99:1; `--text-muted` gives 5.99:1 light and 7.54:1 dark. Found by trial-merging the v2 branch rather than by reading the diff — the conflict hunk showed v2 migrating this exact line while this branch kept the old tier. Matching v2's token choice also shrinks that conflict to the handler alone, which is the part the two branches genuinely disagree about (`clearSearchFilters` here versus `clearSearch` on v2's base). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): restore Library on the record-match path, and stop the phantom collapse Two defects raised in review, both verified against the code and both introduced by this branch. Neither was covered — the existing suites passed before these fixes as well as after, which is why each gets a guard proven by inversion. **Library was unreachable from a services or forms search that matched records but no documents.** Moving Library off the utility rail left it three homes: the sheet footer, the zero-result empty state, and the inline fallback. The footer needs `matches.length > 0`; the empty state needs `recordMatchCount === 0`. The record-match render satisfies neither and returned `null` outright, so on that path the only route to the corpus was the documents action menu — which calls `setQuery("")` and destroys the search being read. That is the precise thing `docs/search-results-bar-decisions.md` requires an in-context route for. The control is now a shared const rendered from both paths, so a fourth branch cannot be added without one. **A group heading reported a collapse it did not perform.** `isOpen` is forced true while the find-a-filter field has text, but the disclosure button stayed mounted and its handler still branched on that forced-true value: tapping it left `aria-expanded="true"`, rotated no chevron, hid nothing — and wrote the group into `collapsed`, so the collapse ambushed the reader later, once the field was cleared and the tap forgotten. While searching, the needle owns what is open, so the heading now renders in its static form. This is the rule the comment three lines above it already stated; the needle case was simply missed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a * fix(ui): stabilize results-band review findings on #1615 Keep phone-control row geometry across loading, stop wide selects sharing a shrinkable flex line with Sort, announce empty states without double-speaking filtered zeros, and clear the stale docs/comments the redesign left behind. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(ui): keep the deferred empty-state announce out of the effect body `788b664` added a deferred screen-reader announcement for the query-only empty state — a live region that mounts already populated is silent in most screen readers, so the message has to arrive on a later frame. The mechanism is right; the implementation called `setLiveMessage("")` synchronously in the effect body twice, which `react-hooks/set-state-in-effect` blocks. `Static PR checks` went red on that lint error, and nothing offline caught it: lint is in neither the unit suite nor `tsc`. Both cleared calls were dead anyway. The region renders only when `!filtered`, so the filtered branch had nothing mounted to clear, and that same gate unmounts and remounts the region across the transition, so it starts empty on its own. Only the `requestAnimationFrame` callback now sets state, which is the form the rule allows and the one the deferral needs. CodeRabbit reached the same fix independently on the same head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qm3i3eLCDcwytzge1sKM4a --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EmptyStatecan now carry a heading (#217). The primitive rendered its title as a<p>, so any empty state that owns a region of the page could not adopt it without losing its heading. A new optionalheadingLevel?: 2 | 3 | 4 | 5 | 6renders the title as that heading element with identical classes. It is deliberately un-defaulted: most adopted call sites sit inside a card that already owns its region's heading, and promoting every title would inject an outline level the surrounding page never declared. Existing callers are unchanged./dsm/searchgets its heading back (#224). The DSM no-matches state renderedNo diagnosis matchesas an<h2>until it adoptedEmptyStatein PR-J; the copy moved into a<p>and heading navigation started skipping the state. It now passesheadingLevel={2}. The restyle from that adoption is intended and is kept — only the semantics are restored.tests/ui-smoke.spec.tspinsgetByRole("heading", { name: "No matching documents" })on it. It now rendersEmptyStatewithheadingLevel={3}and the existingui-smokeheading assertion passes unmodified.#230in the outstanding-issues ledger. Separate commit, append-only, no code impact — see Notes.Two behaviour deltas on the document-search surface worth stating rather than leaving for review to find. Visually, a bespoke centred tinted card becomes the shared left-aligned dashed state surface every other adopted empty state uses. For assistive technology, the state gains the primitive's default polite live region (
role="status"), which it did not have before; the state is introduced dynamically when a search returns nothing, so the announcement is correct there.Verification
npm run verify:pr-localnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsAlso run, before the PR-local preflight:
npm run typecheck(exit 0),npm run lint(exit 0), and the full offline unit suitenpm run test—Test Files 488 passed (488),Tests 5085 passed | 3 skipped (5088). The focused DS run over the two touched test files reportedTest Files 2 passed (2),Tests 12 passed (12).The retrieval, ranking, selection, chunking, source-rendering and answer-contract gates do not apply: this change touches no file under
src/lib/**.classifyPullRequestFilesreturnsclinicalRisk: falseand no RAG-ranking-surface path for every file here — they match the UI patterns only.Risk and rollout
EmptyStatecaller renders byte-identically without it. The one real change is the document-search empty state's appearance and its new polite live region; both are pinned by tests, and the state is only reachable when a document search returns no matches.git revertthe feature commit. It is self-contained — one primitive, two call sites, two test files — and does not depend on the ledger commit beside it.Notes
Test naming: the new DSM pin is
tests/dsm-search-empty-state.dom.test.tsx.vitest.config.mtscollectstests/**/*.test.tsin the node project andtests/**/*.dom.test.tsxin the jsdom project, so a plain*.test.tsxwould have matched neither project and run nowhere. The heading is pinned this time precisely because nothing pinned it when#224regressed.The second commit records ledger
#230from production triage in the same session: live answers onpsychiatry.toolsdegrade to source-only because OpenAI generation overrunsOPENAI_ANSWER_TIMEOUT_MS, while retrieval passes its gate with 12 candidates across 5 documents. It is an append-onlydocs/outstanding-issues.mdrow with no code impact, bundled here per the PR-bundling guidance rather than minting its own branch and CI run.Summary by CodeRabbit
Accessibility
User Experience