From e1ebfcf90b3fee442cd4e77881b7bb046ecd316e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 02:39:23 +0000 Subject: [PATCH 1/8] test(search): close the band adoption gate's route-coverage holes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate advertised "every production search route" and skipped the root dashboard page, so `/?mode=prescribing` and href-less Documents were never checked. Resolving those to `src/app/(search-app)/page.tsx` surfaced two more holes in the same gate that the original finding did not name, and fixing only the first would have produced a false orphan rather than coverage: - The walk was hard-capped at two import hops. The root route's real chain is four, so a fixed hop count silently under-reported reachability instead of failing loudly. - It followed neither `layout.tsx` nor `dynamic(() => import(...))`. Both matter here: `(search-app)/page.tsx` renders only a pass-through and its band arrives through the group layout's shared shell, while the dashboard code-splits its mode workspaces through `clinical-dashboard-lazy.tsx`. Replaced the hop-counted walk with a bounded BFS that resolves `@/` and relative specifiers, follows static and lazy imports, and treats a route's layouts as part of its rendered output — which is what App Router semantics actually mean. Each part was verified load-bearing rather than assumed: restoring the `pathOnly === "/"` early return drops the root route from the inventory, and capping depth back at 2 reports it as an orphan. The negative fixture now uses real files on disk and asserts both directions, since one that only ever returns false would pass against a walker that is broken outright. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- docs/outstanding-issues.md | 2 +- tests/search-results-band-adoption.test.ts | 171 ++++++++++++++++----- 2 files changed, 131 insertions(+), 42 deletions(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 42722a1b4..d376c7fae 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -134,7 +134,7 @@ removed after current-main verification; it is not missing recommended work. | #093 | P2 | issue | Next streaming `S:` clone causes Playwright strict-mode violations under CI load | **Outcome:** duplicate-element strict-mode failures stop appearing on loaded CI runs. **Detail:** under full-suite CI load Next.js leaves a hidden duplicate page root in the stream, so a `getByTestId` that is unique locally resolves to 2 elements in CI (seen as `differentials-search-results` on PR #1316, and previously noted on PR #1294 against main). It does not reproduce in isolation, on a single spec, or locally. The documented workaround is to scope the locator to the visible root. **Reproduced locally 2026-07-28** (isolated _production_ build via `run-playwright.mjs`, full `verify:ui`): `ui-tools.spec.ts:563` duplicated `forms-home` and `ui-smoke.spec.ts:3001` duplicated `favourite-row-lithium-monitoring-guideline`; in both, copy 1 is nested under `mobile-composer-reserve-pad`. Both pass when run alone, so it is load/order-dependent, not build-mode dependent — this also corrects an earlier note that CI uses `next dev`; it does not. **Strongest evidence (CI run `30345484316`, 2026-07-28): `ui-overlap.spec.ts:199` on `/` asserted `toHaveCount(1)` successfully and then the same `header#search` locator resolved to 2 a statement later, one of them hidden.** A duplicate that appears _after_ a passing count assertion is a stream/hydration artifact by construction, not a static double mount and not something a CSS or component change can cause. That makes four distinct testids across four specs with the identical shape. **Mitigated, not fixed, on `main` (2026-07-28):** `3a8edb93` rewrapped `gotoHome` in `tests/ui-overlap.spec.ts` to retry count-and-visibility together via `toPass`, so a transient second header no longer trips strict mode there — its own note says "checking count then immediately calling waitFor races that flicker into a strict-mode violation". That hardens one helper; the duplicate root itself is unchanged and other specs remain exposed. **Confirmed pre-existing:** at `631d90d2`, the commit before PR #1316's first commit, that spec already documented "two `header#search` nodes" and "a second transient `header#search` can exist briefly" — so this predates that branch. **Next:** with a full-suite repro now available, bisect the preceding specs to find the state that triggers the second mount, then either scope the shared helpers to the visible root once or fix the mount. **Stop:** do not paper over new occurrences with `.first()` before the duplicate itself is explained. | PR #1316 CI runs; PR #1294 note on main; session 2026-07-28 | 2026-07-28 | | #094 | P2 | rec | Design-system gates assert structure, not rendered effect | **Outcome:** a style contract cannot pass while the style is inert. **Detail:** PR #1316's accent rail shipped inert because `.search-band` sat in `@layer components`, which loses to Tailwind's utilities layer regardless of specificity — and the test asserted `toHaveClass("search-band")`, i.e. class presence, not effect. Computed style showed `1px rgb(229,231,235)` where `2px rgb(11,111,134)` was intended. The same shape of gap let a rail-colour assertion compare a colour against a width and pass unconditionally. **Next:** for contracts where the visual IS the requirement (rails, forced-colors thickness, tap targets), assert `getComputedStyle` in a Playwright case rather than class names in a DOM test, and add the unlayered-component convention to the design-system contract check. **Stop:** do not convert existing passing DOM tests wholesale; add computed-style proof only where the effect carries the meaning. | PR #1316 Codex P2 finding; session 2026-07-28 | 2026-07-28 | | #095 | P3 | issue | `PR required` reports failure for concurrency-cancelled jobs | **Outcome:** a red `PR required` means a real failure. **Detail:** the aggregate calls `require_success` on `coverage`/`production-ui`, so any push that supersedes an in-flight run reports `coverage result was cancelled` → exit 1, indistinguishable at a glance from a genuine failure. Eleven such reds were produced on PR #1316 in one session, and `Production UI` never once ran to completion. **Next:** in `.github/workflows/ci.yml`, either treat `cancelled` distinctly from `failure` in the aggregate, or reduce push frequency against long UI runs. **Stop:** do not relax `require_success` for genuine failures while doing so. | PR #1316 runs 30340972329 / 30341225585; session 2026-07-28 | 2026-07-28 | -| #096 | P2 | task | One PR #1316 review fix is still a live gap on `main` | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Still live on `main` (verified 2026-07-28):** the band adoption gate skips query-backed root modes — `tests/search-results-band-adoption.test.ts:101` returns null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never enter the route inventory and the root dashboard page is unchecked. **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Next:** resolve root-path and href-less modes to `src/app/(search-app)/page.tsx` in the adoption gate, with a negative fixture for a disconnected root route. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | +| #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Still live on `main` (verified 2026-07-28):** the band adoption gate skips query-backed root modes — `tests/search-results-band-adoption.test.ts:101` returns null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never enter the route inventory and the root dashboard page is unchecked. **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | | #097 | P3 | issue | Gitleaks reports a false red when the PR head moves mid-run | **Outcome:** a red `Gitleaks` means a secret was found, not that someone pushed. **Detail:** on 2026-07-28 the job triggered for head `9bace1d1` checked out that merge ref, then queried the API and built its range against head `40278453` — pushed seconds later and absent from the checkout. Git rejected the range (`fatal: Invalid revision range`), so it scanned `~0 bytes`, logged `no leaks found in partial scan`, and exited 1. The scan did not run at all, which is worse than a normal failure because the natural reading is "noise, ignore it". It cleared on its own once the head stopped moving (`23 commits scanned`, `~198 KB`, `no leaks found`). Both range endpoints resolve in any complete checkout — verified locally against the branch and the PR merge ref — so this is not a `fetch-depth` problem. **Next:** pin the scan to a range the job controls (`base.sha`..the checked-out head) instead of re-querying the API mid-run, so a concurrent push cannot invalidate it. **Related:** same push-churn family as #095. **Stop:** do not weaken the gate to a soft-pass; the fix is a stable range, not a tolerated failure. | PR #1316 runs 30344938800 / 30346797225; session 2026-07-28 | 2026-07-28 | | #098 | P2 | task | Offline round-trip budget harness for the hot routes | **Outcome:** per-scenario Supabase round-trip counts are pinned by a test, so an extra round trip on a hot path is a red gate rather than an inference. **Done 2026-07-29:** the measurement gap is closed — `Server-Timing` now covers `auth`/`ratelimit`/`scope` on `/api/answer`, `auth`/`ratelimit`/`search`/`total` on `/api/search`, and `auth`/`ratelimit` on `/api/answer/stream` (previously the route the UI actually calls emitted no header at all). Headers flush before the first SSE frame, so in-stream stages cannot reach a header and must NOT be routed through the governed `progress`/`final` contract. `tests/answer-route-preamble.test.ts` pins admission-before-scope (no scope call while the limiter is pending or after a deny) and the client-disconnect abort signal. **Next:** generalise it — wrap the Supabase client in a counting proxy and assert per-scenario query budgets over the existing offline suites — `scripts/eval-rag-offline.mjs`, `scripts/test-rag-offline.mjs`, `scripts/rag-offline-contract.mjs` and the contract fixture `scripts/fixtures/rag-offline-contract-tests.json`. **An earlier version of this row named `test-cache-path.mjs` and `check-rag-fixtures.mjs`** (corrected 2026-07-29, PR #1377 review, matching the audit's own retraction): neither exercises a RAG request — the first computes Vitest/TypeScript cache paths, the second only validates fixture manifests — so building the harness on them would have counted nothing. Sequence before #099 and #101: it is the enabler and the standing guard. No providers, no DB. | `docs/audit/latency-audit-2026-07-28.md` measurement plan; `src/lib/server-timing.ts`; `src/lib/answer-stream-contract.ts:18-21` | 2026-07-29 | | #099 | P2 | task | Remove the remaining fixed per-request round trips | **Outcome:** the answer path stops paying avoidable per-request Supabase round trips. **Done 2026-07-29:** shared-cache-hit promotion deferred off the response path with its mid-request staleness guard intact and documented (`rag.ts:3234`, `rag-cache.ts`); scope resolution overlapped with the rate-limit RPC, signal threaded so a client disconnect finally cancels its paginated queries (`answer/route.ts`). **REFUTED on PR #1377 review — do not retry:** the same pass also overlapped scope with the rate-limit RPC and aborted it on deny, claiming the limiter could "deny for free". It cannot. With caller-supplied `filters` or explicit ids, scope passes its zero-query early returns (`search-scope.ts:242,253`) into the paginated `documents` loop at `:269`, and an `AbortSignal` cancels the client request without un-executing a statement Postgres already began — so throttled traffic kept burning database capacity while collecting 429s, against `capacity-review.md:106-113`'s first-soft-failure warning. Scope is behind admission again, pinned by `tests/answer-route-preamble.test.ts`. Re-attempting the overlap requires a non-database admission gate ahead of the durable limiter first. **Remaining:** (a) the 8 `setCachedSearch` awaits — deferring changes `throwIfAborted` semantics and widens a real mutation window because the clone happens after an `await`, so each branch needs discharging individually; (b) batch the anonymous subject+global rate-limit pair, which needs a NEW atomic RPC modelled on `consume_summary_rate_limits_atomic` and cannot be called until the operator applies it — `Promise.all` is the WRONG fix because it consumes the global bucket even when the subject bucket already denied; (c) stop the proxy and route handler resolving identity twice per authenticated request — no in-process memo can do this (different `Request` objects), so the proxy must forward unspoofable verified claims via a header it controls. Cross-references #011: halving auth resolutions eases the ~10-connection Auth cap that `capacity-review.md:106-113` calls the first hard failure. | `docs/audit/latency-audit-2026-07-28.md` L1-1/L1-3/L1-4; `src/lib/api-rate-limit.ts:276-282`; `src/proxy.ts:125` | 2026-07-29 | diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index 1359e08d0..613fcddfe 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -1,4 +1,5 @@ -import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -95,34 +96,94 @@ const BAND_ROUTE_ALLOWLIST = new Map([ ], ]); -/** Resolve a mode href like `/services` to its App Router page file when present. */ -function modeHrefToPagePath(href: string): string | null { - const pathOnly = href.split("?")[0]?.trim() ?? ""; - if (!pathOnly.startsWith("/") || pathOnly === "/") return null; - const candidate = path.join(APP_DIR, "(search-app)", pathOnly.slice(1), "page.tsx"); +const ROOT_DASHBOARD_ROUTE = path.join(APP_DIR, "(search-app)", "page.tsx"); + +/** + * Resolve a mode href to its App Router page file. + * + * Query-backed modes are the reason this is not a plain path join. `prescribing` + * declares `href: "/?mode=prescribing"`, whose pathname is `/`, and the previous + * implementation returned null for that — so it, and every href-less mode such + * as Documents, stayed outside the inventory and the root dashboard page was + * never checked at all. + */ +function modeHrefToPagePath(href: string | null): string | null { + const pathOnly = (href ?? "/").split("?")[0]?.trim() || "/"; + if (!pathOnly.startsWith("/")) return null; + const candidate = + pathOnly === "/" ? ROOT_DASHBOARD_ROUTE : path.join(APP_DIR, "(search-app)", pathOnly.slice(1), "page.tsx"); if (!existsSync(candidate)) return null; return path.relative(REPO_ROOT, candidate).replaceAll(path.sep, "/"); } -/** One or two import hops from a route into `@/components/**` that mounts the band. */ -function routeReachesBand(routeSource: string, componentSources: Map) { - if (rendersBand(routeSource)) return true; - const imported = [...routeSource.matchAll(/from "@\/(components\/[^"]+)"/g)].map((match) => match[1]); - for (const specifier of imported) { - const componentPath = `src/${specifier}.tsx`; - const source = componentSources.get(componentPath); - if (!source) continue; - if (rendersBand(source)) return true; - // Page wrappers often re-export a child that mounts the band (e.g. differentials). - const nested = [...source.matchAll(/from "@\/(components\/[^"]+)"/g)].map((match) => match[1]); - if ( - nested.some((nestedSpec) => { - const nestedSource = componentSources.get(`src/${nestedSpec}.tsx`); - return nestedSource ? rendersBand(nestedSource) : false; - }) - ) { - return true; +/** Every `layout.tsx` that wraps a route, nearest first. In App Router a page's + rendered output includes its layouts, and the root dashboard route relies on + that: `(search-app)/page.tsx` renders only a pass-through, while the band + arrives through the group layout's shared shell. Ignoring layouts would + report that route as an orphan when it is correctly wired. */ +function layoutsFor(routeAbs: string): string[] { + const layouts: string[] = []; + let dir = path.dirname(routeAbs); + while (dir.startsWith(APP_DIR)) { + const candidate = path.join(dir, "layout.tsx"); + if (existsSync(candidate)) layouts.push(candidate); + if (dir === APP_DIR) break; + dir = path.dirname(dir); + } + return layouts; +} + +/** `from "x"` and `import("x")`. The lazy form is load-bearing: the dashboard + code-splits its mode workspaces through `dynamic(() => import(...))` in + `clinical-dashboard-lazy.tsx`, so a static-only walk cannot see the band + behind Differentials, Favourites or the prescribing workspace. */ +const IMPORT_SPECIFIER = /(?:from\s*"([^"]+)")|(?:import\(\s*"([^"]+)"\s*\))/g; + +function resolveSpecifier(specifier: string, fromFile: string): string | null { + let base: string; + if (specifier.startsWith("@/")) base = path.join(REPO_ROOT, "src", specifier.slice(2)); + else if (specifier.startsWith(".")) base = path.resolve(path.dirname(fromFile), specifier); + else return null; + const candidates = [`${base}.tsx`, `${base}.ts`, path.join(base, "index.tsx"), path.join(base, "index.ts")]; + return candidates.find((candidate) => existsSync(candidate)) ?? null; +} + +/** + * Breadth-first search from a route (and its layouts) to a rendered band. + * + * The previous implementation hard-coded two hops into `@/components/**`. The + * real chain on the root dashboard route is four — + * `layout → shared-search-app-shell → global-search-shell → ClinicalDashboard → + * document-search-results` — so a fixed hop count silently under-reported + * reachability rather than failing loudly. + */ +const MAX_IMPORT_DEPTH = 8; + +function routeReachesBand(routeAbs: string): boolean { + const roots = [routeAbs, ...layoutsFor(routeAbs)]; + const seen = new Set(roots); + let frontier = roots.map((file) => ({ file, depth: 0 })); + while (frontier.length > 0) { + const next: Array<{ file: string; depth: number }> = []; + for (const { file, depth } of frontier) { + let source: string; + try { + source = readFileSync(file, "utf8"); + } catch { + continue; + } + if (rendersBand(source)) return true; + if (depth >= MAX_IMPORT_DEPTH) continue; + for (const match of source.matchAll(IMPORT_SPECIFIER)) { + const specifier = match[1] ?? match[2]; + if (!specifier) continue; + const target = resolveSpecifier(specifier, file); + if (!target || seen.has(target) || isMockupPath(target)) continue; + seen.add(target); + next.push({ file: target, depth: depth + 1 }); + } } + frontier = next; } return false; } @@ -161,9 +222,11 @@ describe("search results band adoption", () => { // Top-level mode homes such as /services and /favourites also present result // lists. Restricting the inventory to `/search/` left those pages unchecked. + // An href-less mode (Documents) is served by the root dashboard route, so it + // resolves to the same page as the query-backed hrefs rather than dropping out. const modeHomeRoutes = appModeDefinitions .filter((mode) => mode.search.resultsSurface === "results-band") - .map((mode) => ("href" in mode && typeof mode.href === "string" ? modeHrefToPagePath(mode.href) : null)) + .map((mode) => modeHrefToPagePath("href" in mode && typeof mode.href === "string" ? mode.href : null)) .filter((rel): rel is string => Boolean(rel)) .map((rel) => ({ abs: path.join(REPO_ROOT, rel), rel })); @@ -180,16 +243,18 @@ describe("search results band adoption", () => { searchRoutes.some((route) => route.rel === "src/app/(search-app)/services/page.tsx"), "Mode-href discovery must include top-level results pages such as /services.", ).toBe(true); - - const componentSources = new Map( - productionComponents.map(({ abs, rel }) => [rel.replaceAll(path.sep, "/"), readFileSync(abs, "utf8")]), - ); + // Query-backed and href-less modes (`/?mode=prescribing`, Documents) are all + // served by the root dashboard route. Leaving it out is how the gate could + // report "every production search route" while never checking that page. + expect( + searchRoutes.some((route) => route.rel === "src/app/(search-app)/page.tsx"), + "Mode-href discovery must include the root dashboard route that serves query-backed modes.", + ).toBe(true); const orphans: string[] = []; for (const { abs, rel } of searchRoutes) { if (BAND_ROUTE_ALLOWLIST.has(rel)) continue; - const routeSource = readFileSync(abs, "utf8"); - if (!routeReachesBand(routeSource, componentSources)) orphans.push(rel); + if (!routeReachesBand(abs)) orphans.push(rel); } expect( @@ -250,15 +315,39 @@ describe("band adoption detection", () => { expect(mounted.includes(" { - const componentSources = new Map([ - [ - "src/components/orphan-results-page.tsx", - 'export function OrphanResultsPage() { return
; }', - ], - ]); - const routeSource = - 'import { OrphanResultsPage } from "@/components/orphan-results-page";\nexport default OrphanResultsPage;'; - expect(routeReachesBand(routeSource, componentSources)).toBe(false); + it("resolves a route to the band through imports, and reports one that never gets there", () => { + // Real files on disk, because the walker resolves specifiers rather than + // consulting a map. Both directions are asserted: a fixture that only ever + // returns false would pass against a walker that is broken outright. + const dir = mkdtempSync(path.join(tmpdir(), "band-adoption-")); + try { + writeFileSync( + path.join(dir, "child.tsx"), + "export function Child() { return
; }\n", + "utf8", + ); + writeFileSync( + path.join(dir, "orphan-route.tsx"), + 'import { Child } from "./child";\nexport default Child;\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "orphan-route.tsx"))).toBe(false); + + // Same shape, but the child mounts the band — and via a lazy import, which + // is how the dashboard code-splits its mode workspaces. + writeFileSync( + path.join(dir, "banded-child.tsx"), + `export function Banded() { return <${BAND_IDENTIFIER} modeId="documents" />; }\n`, + "utf8", + ); + writeFileSync( + path.join(dir, "wired-route.tsx"), + 'const Lazy = () => import("./banded-child");\nexport default Lazy;\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "wired-route.tsx"))).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); }); From f46d7a4e4444ba9c97fea2266d69895327c8d08e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 02:50:13 +0000 Subject: [PATCH 2/8] test(search): count layouts only for dashboard-owned routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex caught a hole I introduced one commit earlier, and it was worse than the gap I was closing. Adding the route's layouts as reachability roots applied to every route, and the (search-app) layout transitively imports ClinicalDashboard — so any page under that group passed regardless of what it rendered. Reproduced by reducing services/page.tsx to
: the gate still passed 5/5. Scope layouts to dashboard-owned routes using the repo's own statement of ownership, isAlwaysStandaloneShellPath. Namespaced routes must now reach the band through their own page; only the root dashboard route, whose page renders a pass-through and whose band arrives via the shared shell, keeps its layouts. Re-verified both directions: with services/page.tsx gutted the gate now reports it as an orphan, and passes when restored. Added a test pinning reachabilityRoots directly, so the ownership scoping cannot be widened back without failing — a unit assertion rather than a fixture page, which would have created a real production route with no inbound link. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- tests/search-results-band-adoption.test.ts | 54 ++++++++++++++++++---- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index 613fcddfe..aa5eda225 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { appModeDefinitions } from "@/lib/app-modes"; +import { isAlwaysStandaloneShellPath } from "@/lib/search-route-ownership"; /** * The shared results band is what stops a search surface asserting "0 matches" @@ -116,12 +117,31 @@ function modeHrefToPagePath(href: string | null): string | null { return path.relative(REPO_ROOT, candidate).replaceAll(path.sep, "/"); } -/** Every `layout.tsx` that wraps a route, nearest first. In App Router a page's - rendered output includes its layouts, and the root dashboard route relies on - that: `(search-app)/page.tsx` renders only a pass-through, while the band - arrives through the group layout's shared shell. Ignoring layouts would - report that route as an orphan when it is correctly wired. */ -function layoutsFor(routeAbs: string): string[] { +/** `src/app/(search-app)/services/page.tsx` -> `/services`; the group page -> `/`. */ +function routePathname(routeAbs: string): string { + const rel = path.relative(APP_DIR, routeAbs).replaceAll(path.sep, "/"); + const segments = rel + .replace(/\/page\.tsx$/, "") + .split("/") + .filter((segment) => segment && !segment.startsWith("(")); + return `/${segments.join("/")}`; +} + +/** + * Layouts count toward reachability only for dashboard-owned routes. + * + * In App Router a page's output includes its layouts, and the root dashboard + * route depends on that: `(search-app)/page.tsx` renders only a pass-through and + * its band arrives through the group layout's shared shell. + * + * But that layout transitively imports `ClinicalDashboard`, so following it for + * *every* route made the gate worthless — a namespaced page could be reduced to + * `
` and still "reach" the band. `isAlwaysStandaloneShellPath` is the + * repo's own statement of which routes never mount that dashboard, so those + * routes must reach the band through their own page. + */ +function reachabilityRoots(routeAbs: string): string[] { + if (isAlwaysStandaloneShellPath(routePathname(routeAbs))) return [routeAbs]; const layouts: string[] = []; let dir = path.dirname(routeAbs); while (dir.startsWith(APP_DIR)) { @@ -130,7 +150,7 @@ function layoutsFor(routeAbs: string): string[] { if (dir === APP_DIR) break; dir = path.dirname(dir); } - return layouts; + return [routeAbs, ...layouts]; } /** `from "x"` and `import("x")`. The lazy form is load-bearing: the dashboard @@ -160,7 +180,7 @@ function resolveSpecifier(specifier: string, fromFile: string): string | null { const MAX_IMPORT_DEPTH = 8; function routeReachesBand(routeAbs: string): boolean { - const roots = [routeAbs, ...layoutsFor(routeAbs)]; + const roots = reachabilityRoots(routeAbs); const seen = new Set(roots); let frontier = roots.map((file) => ({ file, depth: 0 })); while (frontier.length > 0) { @@ -315,6 +335,24 @@ describe("band adoption detection", () => { expect(mounted.includes(" { + // The group layout transitively imports ClinicalDashboard, so following it + // for every route would let any page under (search-app) pass while rendering + // nothing. Verified by gutting services/page.tsx to `
`: with layouts + // followed unconditionally the gate still passed; scoped to dashboard-owned + // routes it reports that page as an orphan. + const standalone = path.join(APP_DIR, "(search-app)", "services", "page.tsx"); + expect(reachabilityRoots(standalone)).toEqual([standalone]); + + // The root dashboard route is the case that genuinely needs its layout: the + // page renders only a pass-through and the band arrives via the shared shell. + const rootRoute = path.join(APP_DIR, "(search-app)", "page.tsx"); + const rootRoots = reachabilityRoots(rootRoute); + expect(rootRoots[0]).toBe(rootRoute); + expect(rootRoots.length).toBeGreaterThan(1); + expect(rootRoots.some((file) => file.endsWith(`(search-app)${path.sep}layout.tsx`))).toBe(true); + }); + it("resolves a route to the band through imports, and reports one that never gets there", () => { // Real files on disk, because the walker resolves specifiers rather than // consulting a map. Both directions are asserted: a fixture that only ever From ac2dd7c87124b267569712ec03d2615daa75ad91 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 02:56:55 +0000 Subject: [PATCH 3/8] =?UTF-8?q?issues:=20record=20#112=20=E2=80=94=20adopt?= =?UTF-8?q?ion=20gate=20treats=20a=20discovered=20import=20as=20rendered?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex is right that the walker reports adoption for a route that keeps its results import but stops rendering it; reducing services/page.tsx to
with imports retained keeps the gate green. Recorded rather than fixed here, for two reasons. It is pre-existing — the previous two-hop walker had no render check either — and it is not a live risk: the same edit fails npm run lint with five no-unused-vars warnings under --max-warnings 0, so the composite static gate catches exactly the constructed scenario. The genuinely uncaught shape needs an import referenced somewhere non-rendering, which is deliberate rather than a slip. Tightening it properly means tracking which bindings appear in JSX element position or as the default export, since a re-export mounts with no JSX at all. That is a third expansion of this walker in one PR, and the second one introduced a worse hole than it closed, so it belongs in its own change with its own review. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- docs/outstanding-issues.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index d376c7fae..4b2276aeb 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -81,7 +81,7 @@ removed after current-main verification; it is not missing recommended work. | 33 | `#103` | A3 | Operator — Supabase schema | Same window as `#102` | 30–60 minutes | Confirm whether the wide `document_table_facts` trigram index from `20260714190000` exists live, then either mirror it into `schema.sql` (retained) or drop it via a forward migration (redundant). **Not the allowlist** — it suppresses live-vs-`schema.sql` findings only and cannot make the migration chain and the mirror agree. Stop: do not drop it without live scan evidence. | | 34 | `#105` | Optional | High — browser/UI verification | When the heavy-run lock is free | 20–40 minutes | Run `verify:ui` over the ten `LoadingPanel` fallbacks and confirm the Supabase `preconnect` reaches `` on a live page. Implementation already shipped; this row is the outstanding verification only. | - + ## Open items @@ -149,6 +149,7 @@ removed after current-main verification; it is not missing recommended work. | #108 | P3 | task | Five verified-landed remote branches await deletion (blocked in-session) | **Outcome:** the five branches whose content is fully on `main` are gone. **Detail:** a full-history branch-cleanup review on 2026-07-29 verified these introduce an empty diff against `main` and back no open PR: `claude/clinical-kb-pwa-review-asi3wb` @ `df29f311b60cadf8e43bf51283a9d6f496b295e3`, `claude/dazzling-blackwell-f348d0` @ `c9bec8f9dce38cb647de9aa64ebf08bf7823a524`, `codex/document-reader-condensed-view` @ `b5cdbf301d517239ffe9ed941b9ebe809aea0bfd`, `cursor/page-anchored-search-composer-30ee` @ `7ff134ca7f614db527b8d142676640305533669d`, `cursor/pr-1379-babysit-ledger-9365` @ `be2de03f855cb7fdfccea4bb74d05eb4c9bf6c61`. **The HEADs are recorded because they are unrecoverable once the refs are deleted:** `hasCompletedCleanupReview` (`scripts/sweep-branch-ledger.mjs:83-93`) matches a completed row on branch name AND HEAD together, so without them no later operator could ever append the required `branch-cleanup` rows. Each candidate now also has its own `branch-cleanup-deletion-pending` ledger row keyed to its own HEAD. Deletion could not be performed: the session git proxy rejects ref deletion with **HTTP 403**, and the GitHub MCP toolset exposes no delete-branch capability. The remaining 87 were deliberately NOT cleared — their touched files still differ from `main`, which is the conservative direction. **Next — ORDER MATTERS:** append the completed `branch-cleanup` row for each branch FIRST, from a checkout that still has the objects, and only then delete the refs. `resolveHead` (`scripts/branch-review-ledger.mjs:155-167`) runs `git rev-parse --verify ^{commit}` and refuses to append a HEAD that is not a commit in the repository, so the reverse order is unexecutable once the refs are gone and their objects are pruned. The `n/a - ` escape hatch does not help here: `hasCompletedCleanupReview` only matches a 7-40 char hex HEAD, so an `n/a` row would leave the branch resurfacing in every future sweep. Delete the five from the GitHub UI or an interactive session once their rows are recorded (the existing row is `branch-cleanup-deletion-pending`, which by design does not count as complete). **Stop:** do not widen to the other 87 without per-branch content proof. | session 2026-07-29 branch cleanup; ledger `branch-cleanup-deletion-pending` @ 855aa291 | 2026-07-29 | | #109 | P2 | issue | Remote sessions clone shallow, silently invalidating all branch/merge analysis | **Outcome:** no session draws branch conclusions from a truncated history. **Detail:** on 2026-07-29 this repo's remote session had `git rev-parse --is-shallow-repository` = **true** with only **74** commits of `origin/main` (full history is 2829). Every merge-base, `--cherry-pick`, and ahead/behind number computed in that state was wrong: local `main` reported `ahead 52` and `refusing to merge unrelated histories` (it is actually 0 ahead with a shared base), and an all-branch sweep wrongly showed **90 of 91** branches as carrying unmerged work. Acting on that would have meant either deleting live branches or abandoning cleanup entirely. `git fetch --unshallow` corrected both. **Next:** make `is-shallow-repository` an explicit precondition check in `docs/branch-cleanup-guide.md` §Safety Rules and in `scripts/sweep-branch-ledger.mjs`, failing closed with the `--unshallow` remedy rather than silently reporting. **Stop:** never delete a branch, or report a branch as unmerged, from a shallow clone. | session 2026-07-29; `docs/branch-cleanup-guide.md`; `scripts/sweep-branch-ledger.mjs` | 2026-07-29 | | #110 | P3 | task | Design-system project token manifest lags its stylesheet | **Outcome:** the claude.ai/design token panel matches the shipped stylesheet. **Detail:** PR #1375 pushed a recompiled `_ds_bundle.css` (Clinical Sky, `--e0`–`--e4`, 4px radius grid, `--tracking-eyebrow`/`--leading-display`/`--leading-prose`) plus the four changed guideline docs to project `08d6f126`, but `_ds_manifest.json` is converter-generated and still advertises `--text-4xs: 0.5rem`, the old `--radius-lg/xl/2xl` values, and `--tw-leading`/`--tw-tracking` entries scoped to the retired `.leading-[…]` / `.tracking-[0.08em]` utilities. Rendering is correct; only the token inventory lags. Hand-editing was rejected — `kind`/`scope`/`annotation` are converter heuristics and a wrong panel is worse than a stale one. **Next:** in a session with the `/design-sync` skill, `npm ci`, then `npm install --prefix .ds-sync --no-save --package-lock=false esbuild ts-morph @types/react @tailwindcss/cli geist`, read `.design-sync/NOTES.md`, and run `resync.mjs --remote` so bundle and manifest regenerate together. **Stop:** do not hand-author `_ds_manifest.json`; the converter is not a published npm package and ships with the skill. | PR #1375; `.design-sync/NOTES.md`; project `08d6f126` (`_ds_needs_recompile` marker present) | 2026-07-29 | +| #112 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Outcome:** the gate fails when a route keeps its results import but stops rendering it. **Detail:** `tests/search-results-band-adoption.test.ts` walks imports and reports adoption if any reachable module contains a band element; it does not track whether the imported binding is used in rendered JSX. Reducing `(search-app)/services/page.tsx` to `
` while retaining its imports keeps the gate green. This is **pre-existing** — the previous two-hop walker had the same flaw — and not a live risk, because `npm run lint` fails the same edit with five `@typescript-eslint/no-unused-vars` warnings under `--max-warnings 0`, so the composite static gate does catch it. The genuinely uncaught shape is an import referenced somewhere non-rendering (a type position, or `void Binding;`), which is a deliberate act rather than a plausible slip. **Next:** if tightened, track which imported bindings appear in JSX element position or as the default export — a re-export like `export default Child` is a real mount with no JSX — and follow `dynamic()` bindings the same way; `@babel/parser` is already used by `tests/route-reachability.test.ts`. **Stop:** do not add a fixture page under `src/app` to prove it — `tests/codebase-index-coverage.test.ts` also walks that tree and Vitest runs files in parallel, so a materialising route can be observed mid-run and left behind on failure. | PR #1394 review; session 2026-07-30 | 2026-07-30 | ## Resolved / archive From 1f06cdc964f4acb7696f3659e65a7465ab10cb7e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 30 Jul 2026 03:04:16 +0000 Subject: [PATCH 4/8] docs(ledger): record PR #1394 tip review (band adoption scope) Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 2740e31d4..44ce955bd 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1305,3 +1305,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-29 | 1391 | ff4ddfabb8d8594da0c09e04dc37e9286745140b | PR #1391 merge | merged as 39f2bcea. Three review P2s fixed: both ui-smoke retries made idempotent (toPass re-runs after a late-landing click; the mode-menu one clicked a toggle and oscillated) plus a second guard so the <768px branch can recover at all, and #111 archived instead of left open-and-done. One Production UI red on ui-phone-scroll.spec.ts:574 was proven a flake before re-running: the delta from the passing head baecef05 was two ui-smoke guards and one docs row, neither reachable from that spec, and the PR touches no document-viewer phone-scroll surface. Re-run green | verify:cheap exit 0 (429 files / 4404 tests); design-token orphan guard proven red on a reintroduced class; PR required success with UI_RESULT success on re-run | | 2026-07-29 | 1391 | baecef05cac86c4d52af895d483a33ba3c40cd61 | PR #1391 review | reviewed clean — text-4xs retirement confirmed against globals.css (--text-3xs 0.625rem present, --text-4xs absent); orphan guard proven to fail on a reintroduced class; six Playwright retries all retry action-plus-effect so a genuine regression still fails. Resolved the outstanding-issues #108/#109 double-allocation (renumbered to #110/#111, marker to 112) and recorded #111 done | verify:cheap exit 0 (429 files / 4404 tests); design-token-contract 28 passed; check:branch-review-ledger passed | | 2026-07-29 | 1374 | c14edb9c6f0bdbbfb147752503e016f2543fd803 | PR #1374 review + merge | merged as 3704007c — DocumentViewer identity-bound state clear (P1) implemented and verified red without it; all 12 review threads resolved | verify:cheap exit 0 (429 files / 4403 tests); PR required success; Production UI success | +| 2026-07-30 | claude/top-search-design-mockups-w53znc | 7577a1ea60ab5f0918885f90e849bbac754234b1 | PR #1394 search-results-band-adoption + #096/#115 | No P0/P1. Disposition1 partial: isAlwaysStandaloneShellPath fixes services/etc; /tools still layout-false-positive (P2). Disposition2 verified: import-as-rendered deferred as #115 (P3). #096 closure text accurate for root-path; row still open with stale Still-live clause. | vitest tests/search-results-band-adoption.test.ts 6/6; offline gutting repro tools vs services; static read search-route-ownership + outstanding-issues | From 8c39158d99876338613d5bb3195847fd253ef5ff Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 30 Jul 2026 03:06:22 +0000 Subject: [PATCH 5/8] fix(search): treat /tools as page-only in band adoption gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isAlwaysStandaloneShellPath missed /tools (standalone mode home, but outside the Suspense always-standalone list), so gutting tools/page.tsx still reached the band via layout → ClinicalDashboard. Also clear the contradictory "Still live on main" clause in #096 now that the root-path gap is closed on this PR. Co-authored-by: BigSimmo --- docs/outstanding-issues.md | 2 +- tests/search-results-band-adoption.test.ts | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 19a71c897..e3674f5ce 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -134,7 +134,7 @@ removed after current-main verification; it is not missing recommended work. | #093 | P2 | issue | Next streaming `S:` clone causes Playwright strict-mode violations under CI load | **Outcome:** duplicate-element strict-mode failures stop appearing on loaded CI runs. **Detail:** under full-suite CI load Next.js leaves a hidden duplicate page root in the stream, so a `getByTestId` that is unique locally resolves to 2 elements in CI (seen as `differentials-search-results` on PR #1316, and previously noted on PR #1294 against main). It does not reproduce in isolation, on a single spec, or locally. The documented workaround is to scope the locator to the visible root. **Reproduced locally 2026-07-28** (isolated _production_ build via `run-playwright.mjs`, full `verify:ui`): `ui-tools.spec.ts:563` duplicated `forms-home` and `ui-smoke.spec.ts:3001` duplicated `favourite-row-lithium-monitoring-guideline`; in both, copy 1 is nested under `mobile-composer-reserve-pad`. Both pass when run alone, so it is load/order-dependent, not build-mode dependent — this also corrects an earlier note that CI uses `next dev`; it does not. **Strongest evidence (CI run `30345484316`, 2026-07-28): `ui-overlap.spec.ts:199` on `/` asserted `toHaveCount(1)` successfully and then the same `header#search` locator resolved to 2 a statement later, one of them hidden.** A duplicate that appears _after_ a passing count assertion is a stream/hydration artifact by construction, not a static double mount and not something a CSS or component change can cause. That makes four distinct testids across four specs with the identical shape. **Mitigated, not fixed, on `main` (2026-07-28):** `3a8edb93` rewrapped `gotoHome` in `tests/ui-overlap.spec.ts` to retry count-and-visibility together via `toPass`, so a transient second header no longer trips strict mode there — its own note says "checking count then immediately calling waitFor races that flicker into a strict-mode violation". That hardens one helper; the duplicate root itself is unchanged and other specs remain exposed. **Confirmed pre-existing:** at `631d90d2`, the commit before PR #1316's first commit, that spec already documented "two `header#search` nodes" and "a second transient `header#search` can exist briefly" — so this predates that branch. **Next:** with a full-suite repro now available, bisect the preceding specs to find the state that triggers the second mount, then either scope the shared helpers to the visible root once or fix the mount. **Stop:** do not paper over new occurrences with `.first()` before the duplicate itself is explained. | PR #1316 CI runs; PR #1294 note on main; session 2026-07-28 | 2026-07-28 | | #094 | P2 | rec | Design-system gates assert structure, not rendered effect | **Outcome:** a style contract cannot pass while the style is inert. **Detail:** PR #1316's accent rail shipped inert because `.search-band` sat in `@layer components`, which loses to Tailwind's utilities layer regardless of specificity — and the test asserted `toHaveClass("search-band")`, i.e. class presence, not effect. Computed style showed `1px rgb(229,231,235)` where `2px rgb(11,111,134)` was intended. The same shape of gap let a rail-colour assertion compare a colour against a width and pass unconditionally. **Next:** for contracts where the visual IS the requirement (rails, forced-colors thickness, tap targets), assert `getComputedStyle` in a Playwright case rather than class names in a DOM test, and add the unlayered-component convention to the design-system contract check. **Stop:** do not convert existing passing DOM tests wholesale; add computed-style proof only where the effect carries the meaning. | PR #1316 Codex P2 finding; session 2026-07-28 | 2026-07-28 | | #095 | P3 | issue | `PR required` reports failure for concurrency-cancelled jobs | **Outcome:** a red `PR required` means a real failure. **Detail:** the aggregate calls `require_success` on `coverage`/`production-ui`, so any push that supersedes an in-flight run reports `coverage result was cancelled` → exit 1, indistinguishable at a glance from a genuine failure. Eleven such reds were produced on PR #1316 in one session, and `Production UI` never once ran to completion. **Next:** in `.github/workflows/ci.yml`, either treat `cancelled` distinctly from `failure` in the aggregate, or reduce push frequency against long UI runs. **Stop:** do not relax `require_success` for genuine failures while doing so. | PR #1316 runs 30340972329 / 30341225585; session 2026-07-28 | 2026-07-28 | -| #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Still live on `main` (verified 2026-07-28):** the band adoption gate skips query-backed root modes — `tests/search-results-band-adoption.test.ts:101` returns null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never enter the route inventory and the root dashboard page is unchecked. **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | +| #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Was live on `main` through 2026-07-28:** the band adoption gate skipped query-backed root modes — `modeHrefToPagePath` returned null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never entered the route inventory and the root dashboard page was unchecked. Closed on PR #1394 (see Adoption-gate gap closed below). **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | | #097 | P3 | issue | Gitleaks reports a false red when the PR head moves mid-run | **Outcome:** a red `Gitleaks` means a secret was found, not that someone pushed. **Detail:** on 2026-07-28 the job triggered for head `9bace1d1` checked out that merge ref, then queried the API and built its range against head `40278453` — pushed seconds later and absent from the checkout. Git rejected the range (`fatal: Invalid revision range`), so it scanned `~0 bytes`, logged `no leaks found in partial scan`, and exited 1. The scan did not run at all, which is worse than a normal failure because the natural reading is "noise, ignore it". It cleared on its own once the head stopped moving (`23 commits scanned`, `~198 KB`, `no leaks found`). Both range endpoints resolve in any complete checkout — verified locally against the branch and the PR merge ref — so this is not a `fetch-depth` problem. **Next:** pin the scan to a range the job controls (`base.sha`..the checked-out head) instead of re-querying the API mid-run, so a concurrent push cannot invalidate it. **Related:** same push-churn family as #095. **Stop:** do not weaken the gate to a soft-pass; the fix is a stable range, not a tolerated failure. | PR #1316 runs 30344938800 / 30346797225; session 2026-07-28 | 2026-07-28 | | #098 | P2 | task | Offline round-trip budget harness for the hot routes | **Outcome:** per-scenario Supabase round-trip counts are pinned by a test, so an extra round trip on a hot path is a red gate rather than an inference. **Done 2026-07-29:** the measurement gap is closed — `Server-Timing` now covers `auth`/`ratelimit`/`scope` on `/api/answer`, `auth`/`ratelimit`/`search`/`total` on `/api/search`, and `auth`/`ratelimit` on `/api/answer/stream` (previously the route the UI actually calls emitted no header at all). Headers flush before the first SSE frame, so in-stream stages cannot reach a header and must NOT be routed through the governed `progress`/`final` contract. `tests/answer-route-preamble.test.ts` pins admission-before-scope (no scope call while the limiter is pending or after a deny) and the client-disconnect abort signal. **Next:** generalise it — wrap the Supabase client in a counting proxy and assert per-scenario query budgets over the existing offline suites — `scripts/eval-rag-offline.mjs`, `scripts/test-rag-offline.mjs`, `scripts/rag-offline-contract.mjs` and the contract fixture `scripts/fixtures/rag-offline-contract-tests.json`. **An earlier version of this row named `test-cache-path.mjs` and `check-rag-fixtures.mjs`** (corrected 2026-07-29, PR #1377 review, matching the audit's own retraction): neither exercises a RAG request — the first computes Vitest/TypeScript cache paths, the second only validates fixture manifests — so building the harness on them would have counted nothing. Sequence before #099 and #101: it is the enabler and the standing guard. No providers, no DB. | `docs/audit/latency-audit-2026-07-28.md` measurement plan; `src/lib/server-timing.ts`; `src/lib/answer-stream-contract.ts:18-21` | 2026-07-29 | | #099 | P2 | task | Remove the remaining fixed per-request round trips | **Outcome:** the answer path stops paying avoidable per-request Supabase round trips. **Done 2026-07-29:** shared-cache-hit promotion deferred off the response path with its mid-request staleness guard intact and documented (`rag.ts:3234`, `rag-cache.ts`); scope resolution overlapped with the rate-limit RPC, signal threaded so a client disconnect finally cancels its paginated queries (`answer/route.ts`). **REFUTED on PR #1377 review — do not retry:** the same pass also overlapped scope with the rate-limit RPC and aborted it on deny, claiming the limiter could "deny for free". It cannot. With caller-supplied `filters` or explicit ids, scope passes its zero-query early returns (`search-scope.ts:242,253`) into the paginated `documents` loop at `:269`, and an `AbortSignal` cancels the client request without un-executing a statement Postgres already began — so throttled traffic kept burning database capacity while collecting 429s, against `capacity-review.md:106-113`'s first-soft-failure warning. Scope is behind admission again, pinned by `tests/answer-route-preamble.test.ts`. Re-attempting the overlap requires a non-database admission gate ahead of the durable limiter first. **Remaining:** (a) the 8 `setCachedSearch` awaits — deferring changes `throwIfAborted` semantics and widens a real mutation window because the clone happens after an `await`, so each branch needs discharging individually; (b) batch the anonymous subject+global rate-limit pair, which needs a NEW atomic RPC modelled on `consume_summary_rate_limits_atomic` and cannot be called until the operator applies it — `Promise.all` is the WRONG fix because it consumes the global bucket even when the subject bucket already denied; (c) stop the proxy and route handler resolving identity twice per authenticated request — no in-process memo can do this (different `Request` objects), so the proxy must forward unspoofable verified claims via a header it controls. Cross-references #011: halving auth resolutions eases the ~10-connection Auth cap that `capacity-review.md:106-113` calls the first hard failure. | `docs/audit/latency-audit-2026-07-28.md` L1-1/L1-3/L1-4; `src/lib/api-rate-limit.ts:276-282`; `src/proxy.ts:125` | 2026-07-29 | diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index aa5eda225..0edffec31 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -5,7 +5,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { appModeDefinitions } from "@/lib/app-modes"; -import { isAlwaysStandaloneShellPath } from "@/lib/search-route-ownership"; +import { isAlwaysStandaloneShellPath, isStandaloneModeHomePath } from "@/lib/search-route-ownership"; /** * The shared results band is what stops a search surface asserting "0 matches" @@ -136,12 +136,16 @@ function routePathname(routeAbs: string): string { * * But that layout transitively imports `ClinicalDashboard`, so following it for * *every* route made the gate worthless — a namespaced page could be reduced to - * `
` and still "reach" the band. `isAlwaysStandaloneShellPath` is the - * repo's own statement of which routes never mount that dashboard, so those - * routes must reach the band through their own page. + * `
` and still "reach" the band. Page-owned result surfaces must reach + * the band through their own page: + * - `isAlwaysStandaloneShellPath` — never mounts the dashboard (services, forms, …) + * - `isStandaloneModeHomePath` — mode homes with their own results page, including + * `/tools`, which is intentionally outside the always-standalone Suspense list + * but still mounts `ApplicationsLauncherPage` rather than the dashboard body */ function reachabilityRoots(routeAbs: string): string[] { - if (isAlwaysStandaloneShellPath(routePathname(routeAbs))) return [routeAbs]; + const pathname = routePathname(routeAbs); + if (isAlwaysStandaloneShellPath(pathname) || isStandaloneModeHomePath(pathname)) return [routeAbs]; const layouts: string[] = []; let dir = path.dirname(routeAbs); while (dir.startsWith(APP_DIR)) { @@ -344,6 +348,12 @@ describe("band adoption detection", () => { const standalone = path.join(APP_DIR, "(search-app)", "services", "page.tsx"); expect(reachabilityRoots(standalone)).toEqual([standalone]); + // `/tools` is a standalone mode home but not always-standalone (Suspense + // boundary differs). It must still be page-only — otherwise gutting + // tools/page.tsx stays green via the layout → dashboard import chain. + const tools = path.join(APP_DIR, "(search-app)", "tools", "page.tsx"); + expect(reachabilityRoots(tools)).toEqual([tools]); + // The root dashboard route is the case that genuinely needs its layout: the // page renders only a pass-through and the band arrives via the shared shell. const rootRoute = path.join(APP_DIR, "(search-app)", "page.tsx"); From d658f120253ac0a2dbfe1c9b1a14c6a0744af14d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 30 Jul 2026 03:06:22 +0000 Subject: [PATCH 6/8] docs: record PR #1394 closeout after /tools adoption fix Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 44ce955bd..bf3f936d4 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1306,3 +1306,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-29 | 1391 | baecef05cac86c4d52af895d483a33ba3c40cd61 | PR #1391 review | reviewed clean — text-4xs retirement confirmed against globals.css (--text-3xs 0.625rem present, --text-4xs absent); orphan guard proven to fail on a reintroduced class; six Playwright retries all retry action-plus-effect so a genuine regression still fails. Resolved the outstanding-issues #108/#109 double-allocation (renumbered to #110/#111, marker to 112) and recorded #111 done | verify:cheap exit 0 (429 files / 4404 tests); design-token-contract 28 passed; check:branch-review-ledger passed | | 2026-07-29 | 1374 | c14edb9c6f0bdbbfb147752503e016f2543fd803 | PR #1374 review + merge | merged as 3704007c — DocumentViewer identity-bound state clear (P1) implemented and verified red without it; all 12 review threads resolved | verify:cheap exit 0 (429 files / 4403 tests); PR required success; Production UI success | | 2026-07-30 | claude/top-search-design-mockups-w53znc | 7577a1ea60ab5f0918885f90e849bbac754234b1 | PR #1394 search-results-band-adoption + #096/#115 | No P0/P1. Disposition1 partial: isAlwaysStandaloneShellPath fixes services/etc; /tools still layout-false-positive (P2). Disposition2 verified: import-as-rendered deferred as #115 (P3). #096 closure text accurate for root-path; row still open with stale Still-live clause. | vitest tests/search-results-band-adoption.test.ts 6/6; offline gutting repro tools vs services; static read search-route-ownership + outstanding-issues | +| 2026-07-30 | PR #1394 / `claude/top-search-design-mockups-w53znc` | `8c39158d99876338613d5bb3195847fd253ef5ff` | CI/review closeout: /tools page-only roots + thread disposition | FIXED. Layout false-positive for `/tools` closed via `isStandaloneModeHomePath` in reachabilityRoots. Import-as-rendered finding left as `#115` (pre-existing; lint catches the plausible slip). Both Codex threads dispositioned. Merge clean vs main. | vitest adoption 6/6; full unit 4451 passed / 4 skipped; typecheck; prettier; Bugbot pr-bugbot | From 66c5eb2cd865982041c52737744ac55b81d37664 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 30 Jul 2026 03:06:35 +0000 Subject: [PATCH 7/8] docs: pin PR #1394 closeout ledger row to tip d658f120 Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index bf3f936d4..f0a9ec7db 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1307,3 +1307,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-29 | 1374 | c14edb9c6f0bdbbfb147752503e016f2543fd803 | PR #1374 review + merge | merged as 3704007c — DocumentViewer identity-bound state clear (P1) implemented and verified red without it; all 12 review threads resolved | verify:cheap exit 0 (429 files / 4403 tests); PR required success; Production UI success | | 2026-07-30 | claude/top-search-design-mockups-w53znc | 7577a1ea60ab5f0918885f90e849bbac754234b1 | PR #1394 search-results-band-adoption + #096/#115 | No P0/P1. Disposition1 partial: isAlwaysStandaloneShellPath fixes services/etc; /tools still layout-false-positive (P2). Disposition2 verified: import-as-rendered deferred as #115 (P3). #096 closure text accurate for root-path; row still open with stale Still-live clause. | vitest tests/search-results-band-adoption.test.ts 6/6; offline gutting repro tools vs services; static read search-route-ownership + outstanding-issues | | 2026-07-30 | PR #1394 / `claude/top-search-design-mockups-w53znc` | `8c39158d99876338613d5bb3195847fd253ef5ff` | CI/review closeout: /tools page-only roots + thread disposition | FIXED. Layout false-positive for `/tools` closed via `isStandaloneModeHomePath` in reachabilityRoots. Import-as-rendered finding left as `#115` (pre-existing; lint catches the plausible slip). Both Codex threads dispositioned. Merge clean vs main. | vitest adoption 6/6; full unit 4451 passed / 4 skipped; typecheck; prettier; Bugbot pr-bugbot | +| 2026-07-30 | PR #1394 / `claude/top-search-design-mockups-w53znc` | `d658f120253ac0a2dbfe1c9b1a14c6a0744af14d` | CI/review closeout tip (supersedes 8c39158d row) | FIXED layout `/tools` false-positive; `#115` deferred with human disposition; threads resolved; merge-tree clean; CI settling | vitest adoption 6/6; full unit 4451/4; typecheck; Bugbot | From 61314887cbe2a362fe62d4bc0aff14226d01979b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 03:09:38 +0000 Subject: [PATCH 8/8] style: format the outstanding-issues ledger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prettier only, a single line. docs/outstanding-issues.md was left unformatted by the #096 wording edit on the previous tip, failing both ci/circleci: verify and Static PR checks — the third time this exact break has landed today. Recurs because format:check is not part of npm run test, typecheck, or lint, so a change can report a clean suite and a clean prettier run on the file it was focused on while the repository-wide check is red. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- docs/outstanding-issues.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index e3674f5ce..766feed53 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -134,7 +134,7 @@ removed after current-main verification; it is not missing recommended work. | #093 | P2 | issue | Next streaming `S:` clone causes Playwright strict-mode violations under CI load | **Outcome:** duplicate-element strict-mode failures stop appearing on loaded CI runs. **Detail:** under full-suite CI load Next.js leaves a hidden duplicate page root in the stream, so a `getByTestId` that is unique locally resolves to 2 elements in CI (seen as `differentials-search-results` on PR #1316, and previously noted on PR #1294 against main). It does not reproduce in isolation, on a single spec, or locally. The documented workaround is to scope the locator to the visible root. **Reproduced locally 2026-07-28** (isolated _production_ build via `run-playwright.mjs`, full `verify:ui`): `ui-tools.spec.ts:563` duplicated `forms-home` and `ui-smoke.spec.ts:3001` duplicated `favourite-row-lithium-monitoring-guideline`; in both, copy 1 is nested under `mobile-composer-reserve-pad`. Both pass when run alone, so it is load/order-dependent, not build-mode dependent — this also corrects an earlier note that CI uses `next dev`; it does not. **Strongest evidence (CI run `30345484316`, 2026-07-28): `ui-overlap.spec.ts:199` on `/` asserted `toHaveCount(1)` successfully and then the same `header#search` locator resolved to 2 a statement later, one of them hidden.** A duplicate that appears _after_ a passing count assertion is a stream/hydration artifact by construction, not a static double mount and not something a CSS or component change can cause. That makes four distinct testids across four specs with the identical shape. **Mitigated, not fixed, on `main` (2026-07-28):** `3a8edb93` rewrapped `gotoHome` in `tests/ui-overlap.spec.ts` to retry count-and-visibility together via `toPass`, so a transient second header no longer trips strict mode there — its own note says "checking count then immediately calling waitFor races that flicker into a strict-mode violation". That hardens one helper; the duplicate root itself is unchanged and other specs remain exposed. **Confirmed pre-existing:** at `631d90d2`, the commit before PR #1316's first commit, that spec already documented "two `header#search` nodes" and "a second transient `header#search` can exist briefly" — so this predates that branch. **Next:** with a full-suite repro now available, bisect the preceding specs to find the state that triggers the second mount, then either scope the shared helpers to the visible root once or fix the mount. **Stop:** do not paper over new occurrences with `.first()` before the duplicate itself is explained. | PR #1316 CI runs; PR #1294 note on main; session 2026-07-28 | 2026-07-28 | | #094 | P2 | rec | Design-system gates assert structure, not rendered effect | **Outcome:** a style contract cannot pass while the style is inert. **Detail:** PR #1316's accent rail shipped inert because `.search-band` sat in `@layer components`, which loses to Tailwind's utilities layer regardless of specificity — and the test asserted `toHaveClass("search-band")`, i.e. class presence, not effect. Computed style showed `1px rgb(229,231,235)` where `2px rgb(11,111,134)` was intended. The same shape of gap let a rail-colour assertion compare a colour against a width and pass unconditionally. **Next:** for contracts where the visual IS the requirement (rails, forced-colors thickness, tap targets), assert `getComputedStyle` in a Playwright case rather than class names in a DOM test, and add the unlayered-component convention to the design-system contract check. **Stop:** do not convert existing passing DOM tests wholesale; add computed-style proof only where the effect carries the meaning. | PR #1316 Codex P2 finding; session 2026-07-28 | 2026-07-28 | | #095 | P3 | issue | `PR required` reports failure for concurrency-cancelled jobs | **Outcome:** a red `PR required` means a real failure. **Detail:** the aggregate calls `require_success` on `coverage`/`production-ui`, so any push that supersedes an in-flight run reports `coverage result was cancelled` → exit 1, indistinguishable at a glance from a genuine failure. Eleven such reds were produced on PR #1316 in one session, and `Production UI` never once ran to completion. **Next:** in `.github/workflows/ci.yml`, either treat `cancelled` distinctly from `failure` in the aggregate, or reduce push frequency against long UI runs. **Stop:** do not relax `require_success` for genuine failures while doing so. | PR #1316 runs 30340972329 / 30341225585; session 2026-07-28 | 2026-07-28 | -| #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Was live on `main` through 2026-07-28:** the band adoption gate skipped query-backed root modes — `modeHrefToPagePath` returned null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never entered the route inventory and the root dashboard page was unchecked. Closed on PR #1394 (see Adoption-gate gap closed below). **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | +| #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Was live on `main` through 2026-07-28:** the band adoption gate skipped query-backed root modes — `modeHrefToPagePath` returned null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never entered the route inventory and the root dashboard page was unchecked. Closed on PR #1394 (see Adoption-gate gap closed below). **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | | #097 | P3 | issue | Gitleaks reports a false red when the PR head moves mid-run | **Outcome:** a red `Gitleaks` means a secret was found, not that someone pushed. **Detail:** on 2026-07-28 the job triggered for head `9bace1d1` checked out that merge ref, then queried the API and built its range against head `40278453` — pushed seconds later and absent from the checkout. Git rejected the range (`fatal: Invalid revision range`), so it scanned `~0 bytes`, logged `no leaks found in partial scan`, and exited 1. The scan did not run at all, which is worse than a normal failure because the natural reading is "noise, ignore it". It cleared on its own once the head stopped moving (`23 commits scanned`, `~198 KB`, `no leaks found`). Both range endpoints resolve in any complete checkout — verified locally against the branch and the PR merge ref — so this is not a `fetch-depth` problem. **Next:** pin the scan to a range the job controls (`base.sha`..the checked-out head) instead of re-querying the API mid-run, so a concurrent push cannot invalidate it. **Related:** same push-churn family as #095. **Stop:** do not weaken the gate to a soft-pass; the fix is a stable range, not a tolerated failure. | PR #1316 runs 30344938800 / 30346797225; session 2026-07-28 | 2026-07-28 | | #098 | P2 | task | Offline round-trip budget harness for the hot routes | **Outcome:** per-scenario Supabase round-trip counts are pinned by a test, so an extra round trip on a hot path is a red gate rather than an inference. **Done 2026-07-29:** the measurement gap is closed — `Server-Timing` now covers `auth`/`ratelimit`/`scope` on `/api/answer`, `auth`/`ratelimit`/`search`/`total` on `/api/search`, and `auth`/`ratelimit` on `/api/answer/stream` (previously the route the UI actually calls emitted no header at all). Headers flush before the first SSE frame, so in-stream stages cannot reach a header and must NOT be routed through the governed `progress`/`final` contract. `tests/answer-route-preamble.test.ts` pins admission-before-scope (no scope call while the limiter is pending or after a deny) and the client-disconnect abort signal. **Next:** generalise it — wrap the Supabase client in a counting proxy and assert per-scenario query budgets over the existing offline suites — `scripts/eval-rag-offline.mjs`, `scripts/test-rag-offline.mjs`, `scripts/rag-offline-contract.mjs` and the contract fixture `scripts/fixtures/rag-offline-contract-tests.json`. **An earlier version of this row named `test-cache-path.mjs` and `check-rag-fixtures.mjs`** (corrected 2026-07-29, PR #1377 review, matching the audit's own retraction): neither exercises a RAG request — the first computes Vitest/TypeScript cache paths, the second only validates fixture manifests — so building the harness on them would have counted nothing. Sequence before #099 and #101: it is the enabler and the standing guard. No providers, no DB. | `docs/audit/latency-audit-2026-07-28.md` measurement plan; `src/lib/server-timing.ts`; `src/lib/answer-stream-contract.ts:18-21` | 2026-07-29 | | #099 | P2 | task | Remove the remaining fixed per-request round trips | **Outcome:** the answer path stops paying avoidable per-request Supabase round trips. **Done 2026-07-29:** shared-cache-hit promotion deferred off the response path with its mid-request staleness guard intact and documented (`rag.ts:3234`, `rag-cache.ts`); scope resolution overlapped with the rate-limit RPC, signal threaded so a client disconnect finally cancels its paginated queries (`answer/route.ts`). **REFUTED on PR #1377 review — do not retry:** the same pass also overlapped scope with the rate-limit RPC and aborted it on deny, claiming the limiter could "deny for free". It cannot. With caller-supplied `filters` or explicit ids, scope passes its zero-query early returns (`search-scope.ts:242,253`) into the paginated `documents` loop at `:269`, and an `AbortSignal` cancels the client request without un-executing a statement Postgres already began — so throttled traffic kept burning database capacity while collecting 429s, against `capacity-review.md:106-113`'s first-soft-failure warning. Scope is behind admission again, pinned by `tests/answer-route-preamble.test.ts`. Re-attempting the overlap requires a non-database admission gate ahead of the durable limiter first. **Remaining:** (a) the 8 `setCachedSearch` awaits — deferring changes `throwIfAborted` semantics and widens a real mutation window because the clone happens after an `await`, so each branch needs discharging individually; (b) batch the anonymous subject+global rate-limit pair, which needs a NEW atomic RPC modelled on `consume_summary_rate_limits_atomic` and cannot be called until the operator applies it — `Promise.all` is the WRONG fix because it consumes the global bucket even when the subject bucket already denied; (c) stop the proxy and route handler resolving identity twice per authenticated request — no in-process memo can do this (different `Request` objects), so the proxy must forward unspoofable verified claims via a header it controls. Cross-references #011: halving auth resolutions eases the ~10-connection Auth cap that `capacity-review.md:106-113` calls the first hard failure. | `docs/audit/latency-audit-2026-07-28.md` L1-1/L1-3/L1-4; `src/lib/api-rate-limit.ts:276-282`; `src/proxy.ts:125` | 2026-07-29 |