diff --git a/docs/outstanding-issues-inbox/45cda6a2-725f-4fc7-bbd8-2350c7280d72.json b/docs/outstanding-issues-inbox/45cda6a2-725f-4fc7-bbd8-2350c7280d72.json new file mode 100644 index 0000000000..386b83e487 --- /dev/null +++ b/docs/outstanding-issues-inbox/45cda6a2-725f-4fc7-bbd8-2350c7280d72.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "45cda6a2-725f-4fc7-bbd8-2350c7280d72", + "createdOn": "2026-09-07", + "action": "add", + "payload": { + "pri": "P2", + "type": "issue", + "summary": "PR required failed repeatedly on a UI-only branch with no cause ever identified", + "detail": "Session 2026-09-07, branch claude/jolly-keller-vy36h2 (the also-matches disclosure). The pr-required aggregate went red several times while every local gate was green: 15 static gates, whole-tree Prettier, 229 + 161 focused unit tests, 129 + 28 Chromium tests. A push-cancellation theory was offered and then disproved by a run with a quiet window. CI log access was blocked by the babysit marker for the whole session, so the failing job was never read and the branch merged with the cause unknown. Next action: on the next unexplained pr-required failure, unlock the marker first (CLAUDE_ALLOW_PR_FOLLOW=1) and read the failing job log before theorising. Two contributing factors worth ruling out: a second agent was pushing merge commits to the same branch concurrently, and main merged about twelve changes during the session.", + "source": "docs/branch-review-records + session 331c4d4d", + "issueUlid": "01M1XP1FE7K06J63XSYVBS40HA" + } +} diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index 44f39da693..2cbfef5445 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -343,9 +343,24 @@ function ClinicalDashboardContent({ (retainTarget = false) => scheduleComposerFocus(composerInputRef, retainTarget), [composerInputRef], ); - const [modeSearchSubmitted, setModeSearchSubmitted] = useState(() => + const [modeSearchSubmitted, setModeSearchSubmittedFlag] = useState(() => Boolean(autoRunSearch && initialQuery.trim() && initialSearchMode !== "tools"), ); + // The query the mode's on-screen results actually belong to, which is NOT `query`: + // editing the bottom composer calls `setQuery` alone and leaves both the results and + // the submitted flag in place. Anything keyed to the submitted search must read this, + // or a paused draft silently replaces it while the primary cards still show the last + // submitted search. Null until a submission records one. + const [submittedModeQuery, setSubmittedModeQuery] = useState(() => + autoRunSearch && initialQuery.trim() && initialSearchMode !== "tools" ? initialQuery.trim() : null, + ); + // Every submission already sets `query` to the text it submitted, so the text is passed + // here too rather than read back from state. Clearing the flag clears the query with it. + const setModeSearchSubmitted = useCallback((submitted: boolean, submittedText?: string) => { + setModeSearchSubmittedFlag(submitted); + if (!submitted) setSubmittedModeQuery(null); + else if (submittedText !== undefined) setSubmittedModeQuery(submittedText.trim()); + }, []); // focus=1 means "focus on entry", not "keep the dock focused after results". // Suppress autofocus once a mode search/answer has been submitted so hide-on- // scroll can reclaim chrome on result views (Answer and other bottom docks). @@ -1861,7 +1876,7 @@ function ClinicalDashboardContent({ if (modeSearch.resultKind !== "answer") { setQuery(trimmedQuery); } - if (modeSearch.kind !== "tools") setModeSearchSubmitted(true); + if (modeSearch.kind !== "tools") setModeSearchSubmitted(true, trimmedQuery); if (isDifferentialsMode) clearModeResultState(); if (modeSearch.kind === "tools") { @@ -2149,7 +2164,7 @@ function ClinicalDashboardContent({ if (!trimmedSearchText) return; setSearchMode("prescribing"); setQuery(trimmedSearchText); - setModeSearchSubmitted(true); + setModeSearchSubmitted(true, trimmedSearchText); setLoading(false); setError(null); setAnswerProgress(null); @@ -2467,7 +2482,7 @@ function ClinicalDashboardContent({ if (targetMode === "documents") { setQuery(trimmedSearchText); setSearchMode("documents"); - setModeSearchSubmitted(true); + setModeSearchSubmitted(true, trimmedSearchText); setLoading(false); setError(null); setAnswerProgress(null); @@ -2495,7 +2510,7 @@ function ClinicalDashboardContent({ setQuery(trimmedSearchText); setSearchMode(targetMode); - setModeSearchSubmitted(true); + setModeSearchSubmitted(true, trimmedSearchText); setLoading(true); setError(null); const targetModeSearch = appModeSearchConfig(targetMode); @@ -2990,7 +3005,13 @@ function ClinicalDashboardContent({ activeModeResultKind === "answer" && answerProgressEvents.length > 0 && (loading || (Boolean(answer) && answerProgressCompleted)); - const universalAlsoMatchesQuery = activeModeResultKind === "answer" ? (latestAnswerQuery ?? query) : query; + // Answer mode already keyed off the generated answer's query. Every other mode keys off + // the submitted query for the same reason: typing without pressing Enter must not fetch + // cross-mode matches for the draft, nor replace the tray and its count with results the + // primary cards do not share. Tools and Favourites never record a submission, so they + // fall through to `query`, which is the only query they have. + const universalAlsoMatchesQuery = + activeModeResultKind === "answer" ? (latestAnswerQuery ?? query) : (submittedModeQuery ?? query); // Answer-mode also-matches wait for a completed generation (`answer && !loading`) // so the panel never sits under the drafting skeleton/stepper. Tools/Favourites // still mount on submission. Follow-ups hide the panel while loading so stale diff --git a/src/components/answer-chat-perfected-v2-mockups.tsx b/src/components/answer-chat-perfected-v2-mockups.tsx index 7c2f9774e3..f9944a27be 100644 --- a/src/components/answer-chat-perfected-v2-mockups.tsx +++ b/src/components/answer-chat-perfected-v2-mockups.tsx @@ -1315,7 +1315,7 @@ function ConsecutiveLineSpecimen() {

Marks on consecutive lines

-

+

0 ? ` · +${rest}` : null} - {/* Visual cue only — the button's accessible name stays the label above, - so a screen reader is not read the count twice. */} + {/* The visible count is decorative — it is dropped entirely below sm, and + `hidden` hides it from assistive tech as well as from the eye. This + tray has no live region to carry the number, so without the sr-only + copy the count would never reach a screen reader at any width, and + the closed control would announce as a door onto an unknown. */} {countLabel} + {countLabel} {hasCoverUrl ? ( // Private signed covers stay unoptimized so bearer URLs never enter `/_next/image`. diff --git a/src/components/clinical-dashboard/universal-search-also-matches.tsx b/src/components/clinical-dashboard/universal-search-also-matches.tsx index 9c5f086d47..8cb85a087c 100644 --- a/src/components/clinical-dashboard/universal-search-also-matches.tsx +++ b/src/components/clinical-dashboard/universal-search-also-matches.tsx @@ -83,11 +83,10 @@ export function UniversalSearchAlsoMatches({ // between the composer and the results the search actually asked for. It used // to open itself from sm up. const [expanded, setExpanded] = useState(false); - // The sm breakpoint (640px) no longer decides whether the panel is open — the - // disclosure does, at every width. It still decides two things the disclosure - // cannot: whether the cross-mode lookup runs before the user opens anything - // (so the closed header can state a real count), and whether a no-match tray - // is dropped entirely rather than left as a header that opens onto nothing. + // The sm breakpoint (640px) no longer decides whether the panel is open, nor + // whether the lookup runs — the disclosure owns the first and the lookup is + // eager at every width. All it still decides is how many mode cards an opened + // tray may show, four on a wide viewport against three on a phone. const [isWide, setIsWide] = useState(false); const [viewportReady, setViewportReady] = useState(false); useEffect(() => { @@ -106,13 +105,14 @@ export function UniversalSearchAlsoMatches({ // arrive; a speculative phone disclosure would add dead space to short // answers that have no cross-mode matches. // - // Deliberately unchanged by the collapse: the lookup still runs on submit from - // sm up even while the tray is shut. That is what lets a closed header say - // "3 related modes" and lets the whole tray disappear when nothing matched. A - // closed control that cannot say what is behind it is a blind door, and - // making the fetch wait for the click would turn every desktop open into a - // spinner over a panel that may hold nothing. - const searchActive = submissionActive && (isWide || modeId === "answer" || expanded); + // The lookup runs on submit at every width, including phones, even while the + // tray is shut. That is what lets a closed header say "3 related modes" and + // lets the whole tray disappear when nothing matched. A closed control that + // cannot say what is behind it is a blind door, and the phone was the width + // where that bit: it showed "Tap to open" and could open onto nothing. The + // cost is one extra cross-mode lookup per phone search, accepted so that the + // closed row states a real count and an empty tray is never rendered at all. + const searchActive = submissionActive; const universal = useUniversalSearch({ query: trimmedQuery, enabled: trimmedQuery.length >= 2 && searchActive, @@ -165,18 +165,14 @@ export function UniversalSearchAlsoMatches({ : matchCount > 0 ? `${matchCountLabel(matchCount)} also match this search.` : emptyMessage; - const headerMeta = searchPending - ? "Searching…" - : !searchActive - ? "Tap to open" - : matchCount > 0 - ? matchCountLabel(matchCount) - : "No other matches"; + const headerMeta = searchPending ? "Searching…" : matchCount > 0 ? matchCountLabel(matchCount) : "No other matches"; if (!submissionActive) return null; if (!viewportReady || trimmedQuery.length < 2) return null; if (modeId === "answer" && currentGroups.length === 0) return null; - if (isWide && !searchPending && currentGroups.length === 0) return null; + // At every width now, not just from sm up: the phone lookup is eager, so a + // header that would open onto nothing is dropped instead of offered. + if (!searchPending && currentGroups.length === 0) return null; return (

>; setAnswer: Dispatch>; setSources: Dispatch>; - setModeSearchSubmitted: Dispatch>; + /** Also records the submitted query, so a later composer edit cannot pass for a submission. */ + setModeSearchSubmitted: (submitted: boolean, submittedText?: string) => void; setQuery: Dispatch>; setAnswerThreadBootstrapped: Dispatch>; }) { @@ -131,7 +132,9 @@ export function useAnswerThreadBootstrap({ latestAnswerTurnRef.current = persisted.latestTurn; setAnswer(persisted.latestTurn.answer); setSources(persisted.latestTurn.sources); - setModeSearchSubmitted(true); + // Restoring a persisted thread is a submission of that turn's query, not of + // whatever happens to be in the composer. + setModeSearchSubmitted(true, persisted.latestTurn.query); setQuery(""); autoRunSearchSignatureRef.current = persisted.latestSubmissionSignature; } diff --git a/src/components/clinical-dashboard/use-home-mode-seed.ts b/src/components/clinical-dashboard/use-home-mode-seed.ts index 0bdd9fe27e..21a677fb2b 100644 --- a/src/components/clinical-dashboard/use-home-mode-seed.ts +++ b/src/components/clinical-dashboard/use-home-mode-seed.ts @@ -42,7 +42,8 @@ export function useHomeModeSeed({ setQuery: Dispatch>; setQueryMode: Dispatch>; setScopeFilters: Dispatch>; - setModeSearchSubmitted: Dispatch>; + /** Also records the submitted query, so a later composer edit cannot pass for a submission. */ + setModeSearchSubmitted: (submitted: boolean, submittedText?: string) => void; setLoading: Dispatch>; setError: Dispatch>; setAnswerProgress: Dispatch>; diff --git a/tests/audit-navigation-auth-regressions.test.ts b/tests/audit-navigation-auth-regressions.test.ts index 0d1026b78d..7aea85eeb5 100644 --- a/tests/audit-navigation-auth-regressions.test.ts +++ b/tests/audit-navigation-auth-regressions.test.ts @@ -227,16 +227,28 @@ describe("audit navigation and auth regressions", () => { ); }); - it("defers cross-mode search on narrow screens until expansion except for completed answers", () => { + it("runs cross-mode search on submission at every width, so a closed tray can state its count", () => { // `prescribing` was excluded here while the panel mounted ABOVE the medication // results; the mount moved below them, so the mode is no longer suppressed and - // the deferral contract is the plain submission gate. tests/ui-stress.spec.ts - // pins the panel's position under those results. - expect(universalAlsoMatchesSource).toContain("const searchActive = submissionActive &&"); + // the gate is plain submission. tests/ui-stress.spec.ts pins the panel's + // position under those results. + // + // The narrow-screen deferral this contract used to pin is gone deliberately. + // Waiting for the click meant the phone header could only say "Tap to open" + // and the tray was still rendered when nothing was behind it — a blind door. + // The lookup is eager at every width and an empty tray is dropped instead. + expect(universalAlsoMatchesSource).toContain("const searchActive = submissionActive;"); expect(universalAlsoMatchesSource).not.toContain('modeId !== "prescribing"'); - expect(universalAlsoMatchesSource).toContain('(isWide || modeId === "answer" || expanded)'); + expect(universalAlsoMatchesSource).not.toContain('(isWide || modeId === "answer" || expanded)'); + // The header now says pending / a count / nothing found. The "Tap to open" + // arm it replaced survives only in the comment above the searchActive gate, + // which is why this pins the expression rather than searching for the string. + expect(universalAlsoMatchesSource).toContain( + 'const headerMeta = searchPending ? "Searching…" : matchCount > 0 ? matchCountLabel(matchCount) : "No other matches";', + ); expect(universalAlsoMatchesSource).toContain("enabled: trimmedQuery.length >= 2 && searchActive"); expect(universalAlsoMatchesSource).toContain('if (modeId === "answer" && currentGroups.length === 0) return null;'); + expect(universalAlsoMatchesSource).toContain("if (!searchPending && currentGroups.length === 0) return null;"); expect(universalAlsoMatchesSource).toContain("const [viewportReady, setViewportReady] = useState(false);"); expect(universalAlsoMatchesSource).toContain("setViewportReady(true);"); // The panel status is a three-way now — pending / a count / nothing found — diff --git a/tests/ui-stress.spec.ts b/tests/ui-stress.spec.ts index 666f8d5e06..caf6b08320 100644 --- a/tests/ui-stress.spec.ts +++ b/tests/ui-stress.spec.ts @@ -449,6 +449,47 @@ test.describe("Medication responsive stress coverage", () => { }, { storageKey: PATIENT_PROFILE_STORAGE_KEY }, ); + // The cross-mode lookup is eager at phone width, and a tray with nothing + // behind it is now dropped rather than shown as a header that opens onto + // nothing. So the panel this test is about only exists if the endpoint + // returns a match outside Medication — mocked here rather than left to + // whatever the demo corpus happens to hold for this query. + await page.route(/\/api\/search\/universal(?:\?.*)?$/, async (route) => { + const query = new URL(route.request().url()).searchParams.get("q") ?? ""; + const group = { + kind: "documents", + total: 1, + latencyMs: 2, + items: [ + { + id: "acamprosate-guideline", + kind: "documents", + title: "Acamprosate prescribing guideline", + href: "/documents/acamprosate-guideline", + score: 0.86, + }, + ], + }; + const response = { + query, + tookMs: 8, + demoMode: true, + groups: [group], + contextMode: "prescribing", + preferredDomains: ["medications"], + domainOrder: ["medications", "documents"], + }; + // The endpoint streams NDJSON, one event per line. A single JSON object + // parses to nothing and the panel then correctly drops itself. + const events = [ + { type: "group", query, group }, + { type: "complete", response }, + ]; + await route.fulfill({ + body: `${events.map((event) => JSON.stringify(event)).join("\n")}\n`, + contentType: "application/x-ndjson; charset=utf-8", + }); + }); await page.setViewportSize({ width: 320, height: 720 }); await page.goto("/?mode=prescribing&q=acamprosate%20renal%20dose&run=1", { waitUntil: "domcontentloaded" }); diff --git a/tests/ui-universal-search.spec.ts b/tests/ui-universal-search.spec.ts index f783c42524..6497cf9136 100644 --- a/tests/ui-universal-search.spec.ts +++ b/tests/ui-universal-search.spec.ts @@ -389,7 +389,7 @@ test.describe("universal search typeahead", () => { ).toBe(true); }); - test("loads submitted cross-mode matches on phones only after expansion", async ({ page }) => { + test("states the phone cross-mode count on the closed header, before any expansion", async ({ page }) => { await page.setViewportSize({ width: 390, height: 844 }); const universalRequests: string[] = []; page.on("request", (request) => { @@ -401,10 +401,18 @@ test.describe("universal search typeahead", () => { const alsoMatches = page.getByTestId("universal-also-matches"); await expect(alsoMatches).toBeVisible(); await expect(alsoMatches).toHaveCount(1); - expect(universalRequests).toHaveLength(0); - - await alsoMatches.getByRole("button", { name: /Also matches in other modes/ }).click(); + // Eager at phone width too. A closed row that says "Tap to open" is a blind + // door: it cannot promise the tray holds anything, and the empty tray was + // still rendered. The lookup runs on submit so the header states a count. await expect.poll(() => universalRequests.length).toBe(1); + + const trigger = alsoMatches.getByRole("button", { name: /Also matches in other modes/ }); + await expect(trigger).toHaveAttribute("aria-expanded", "false"); + await expect(alsoMatches).not.toContainText("Tap to open"); + await expect(alsoMatches.getByRole("link", { name: "Acamprosate", exact: true })).toBeHidden(); + + await trigger.click(); + await expect(trigger).toHaveAttribute("aria-expanded", "true"); await expect(alsoMatches.getByRole("link", { name: "Acamprosate", exact: true })).toBeVisible(); }); diff --git a/tests/universal-also-matches-submitted-query.test.ts b/tests/universal-also-matches-submitted-query.test.ts new file mode 100644 index 0000000000..8ac07c6fa5 --- /dev/null +++ b/tests/universal-also-matches-submitted-query.test.ts @@ -0,0 +1,86 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +/** + * THE DEFECT. On a phone dashboard result view, editing the bottom composer calls `setQuery` + * alone: the results and `modeSearchSubmitted` both stay as they were. The cross-mode + * also-matches lookup was keyed on `query`, so a paused draft fetched matches for a search the + * reader never ran, replaced the tray and its "N related modes" count, and left those matches + * disagreeing with the primary cards still on screen for the last submitted query. + * + * The fix keys the lookup on the submitted query instead. These assertions read the shipped + * source, because the behaviour lives in how ClinicalDashboard derives one value and cannot be + * reached without mounting the whole dashboard. + */ +const DASHBOARD = readFileSync(join(__dirname, "..", "src/components/ClinicalDashboard.tsx"), "utf8"); + +function sourceOf(pattern: RegExp): string { + const match = DASHBOARD.match(pattern); + expect(match, `expected to find ${pattern}`).toBeTruthy(); + return match![0]; +} + +describe("cross-mode also-matches follows the submitted query, not the draft", () => { + it("derives the lookup query from submittedModeQuery rather than the live composer query", () => { + const derivation = sourceOf(/const universalAlsoMatchesQuery =[\s\S]*?;\n/); + expect(derivation).toContain("submittedModeQuery ?? query"); + // The regression this guards: the whole non-answer arm used to be the bare draft. + expect(derivation).not.toMatch(/:\s*query;\s*$/); + }); + + it("keeps answer mode on the generated answer's query", () => { + const derivation = sourceOf(/const universalAlsoMatchesQuery =[\s\S]*?;\n/); + expect(derivation).toContain('activeModeResultKind === "answer" ? (latestAnswerQuery ?? query)'); + }); + + it("holds submittedModeQuery separately from query, so typing cannot move it", () => { + expect(DASHBOARD).toMatch(/const \[submittedModeQuery, setSubmittedModeQuery\] = useState/); + // Nothing may write the submitted query except the submission wrapper, or a composer edit + // would reach it after all. + const writes = [...DASHBOARD.matchAll(/setSubmittedModeQuery\(/g)]; + expect(writes.length).toBe(2); + const wrapper = sourceOf(/const setModeSearchSubmitted = useCallback\([\s\S]*?\n \}, \[\]\);/); + expect((wrapper.match(/setSubmittedModeQuery\(/g) ?? []).length).toBe(2); + }); + + it("records the submitted text at every submission, and clears it when submission is cleared", () => { + const submissions = [...DASHBOARD.matchAll(/setModeSearchSubmitted\(true[^)]*\)/g)].map((m) => m[0]); + expect(submissions.length).toBeGreaterThan(0); + // Every submission passes the text it submitted; a bare `true` would leave the previous + // query in place and reintroduce the mismatch. + for (const call of submissions) { + expect(call, `submission without its query: ${call}`).toMatch(/setModeSearchSubmitted\(true, \S+\)/); + } + const wrapper = sourceOf(/const setModeSearchSubmitted = useCallback\([\s\S]*?\n \}, \[\]\);/); + expect(wrapper).toContain("if (!submitted) setSubmittedModeQuery(null);"); + }); + + it("seeds the submitted query from an auto-run URL, so a restored result view is not blank", () => { + const seed = sourceOf(/const \[submittedModeQuery, setSubmittedModeQuery\][\s\S]*?\);\n/); + expect(seed).toContain("autoRunSearch"); + expect(seed).toContain("initialQuery.trim()"); + expect(seed).toContain('initialSearchMode !== "tools"'); + }); +}); + +/** + * The lookup itself is gated on this query, so with the draft no longer reaching it, typing + * without pressing Enter changes neither the request nor the count. + */ +describe("the also-matches lookup is gated on that query", () => { + const PANEL = readFileSync( + join(__dirname, "..", "src/components/clinical-dashboard/universal-search-also-matches.tsx"), + "utf8", + ); + + it("enables the fetch only from the query it was given", () => { + expect(PANEL).toMatch(/enabled: trimmedQuery\.length >= 2 && searchActive/); + expect(PANEL).toMatch(/query: trimmedQuery,/); + }); + + it("renders its count from the same query, so the closed row cannot describe a draft", () => { + expect(PANEL).toContain("universal.query === trimmedQuery"); + }); +});