From 12cce795db7fa08aac0565ccb74b8277cf32a384 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 03:22:14 +0000 Subject: [PATCH 01/11] docs(agents): require npm run format before pushing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three CI failures on 2026-07-30 came from the same cause — an unformatted file reaching Static PR checks and ci/circleci: verify, which both run prettier --check . Each time the author had run the unit suite, typecheck, lint, and a focused prettier check on the file they were working on, and reported green. The loop cannot catch it: format:check is in none of test, typecheck or lint. A format-before-push guard already exists in .githooks/pre-push, and it works — but core.hooksPath is configured by this checkout's npm install, so an agent pushing from its own environment never runs it. Hooks are client-side, so this cannot be fixed for other environments in tooling; the instruction surface is the only lever. Records the specific trap as well as the rule: the missed file was twice a doc or ledger edit made alongside a source change, which a per-file prettier check on the source file does not cover. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 1bfd7cc38b..bcf63bf5c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,6 +166,7 @@ When a branch or PR review completes, record it with `npm run ledger:append -- - - For non-trivial source/config/test changes, prefer `npm run verify:cheap` as the first broad gate and `npm run verify:pr-local` before PR handoff when the change is ready. The PR-local gate runs the full unit suite once, then conditionally adds the production build/client-bundle scan and RAG fixture/manifest validation. Browser, dependency-audit, Docker/Supabase replay, and provider-backed checks remain separate gates. Use `npm run verify:pr-local -- --dry-run --files ` to inspect selection without running commands. The broader `--extended` plan is dry-run only unless explicit approval is reflected by `ALLOW_EXTENDED_PR_LOCAL=true`. - Let the repository run coordinator control cross-worktree verification. It permits at most two focused Vitest/read-only typecheck leases from different worktrees; full Vitest, coverage, lint, build, Playwright, and live-provider tests remain exclusive. Do not install while a repository test, build, lint, typecheck, or server command is active. Avoid aggressive short-interval polling, and do not repeat an unchanged full gate after it passes. - For UI, frontend, browser, routing, styling, reduced-motion, or forced-colors changes, run `npm run ensure` before browser work and use `npm run verify:ui` as the Chromium UI gate. For phone-chrome changes, run `npm run verify:phone-chrome` first: it checks installed-lock parity, selects the affected browser/PWA owners and exact journeys, and adds `verify:ui` last only when shared chrome foundations make the broad gate necessary. Inspect uncertain scope with `-- --dry-run`. +- **Run `npm run format` before every push.** `format:check` is in neither `npm run test`, `npm run typecheck`, nor `npm run lint`, so the ordinary loop reports green while `Static PR checks` and `ci/circleci: verify` both fail on `prettier --check .`. A focused `prettier --check ` on the file you edited is not enough — the repository-wide check covers every file in the push, and a ledger or doc edit made alongside a source change is the usual thing that gets missed. `.githooks/pre-push` already carries a `format-before-push` guard, but `core.hooksPath` is set by this checkout's `npm install`, so an agent pushing from its own environment bypasses it and only CI catches the break. Three CI failures on 2026-07-30 came from exactly this, every one from a push made outside a hook-configured checkout. - For release or handoff confidence, use `npm run verify:release`; this includes the full Playwright project set. - For clinical ingestion, answer generation, source governance, privacy, production-readiness, or environment changes, run the smallest relevant domain check plus `npm run check:production-readiness`. - For pull requests that touch ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output, complete the clinical governance preflight in `.github/pull_request_template.md`. From 9b7cec230871a4cfefdfca826386032f1d386c54 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 30 Jul 2026 03:19:47 +0000 Subject: [PATCH 02/11] docs(ledger): record PR #1394 CI green after format + main sync Tip 66c5eb2c failed Static PR on prettier padding in #096; fixed on 61314887, then main sync landed as 4a001efa. Closeout confirms required checks green with no open review threads. 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 f0a9ec7db1..fd6bce7ceb 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1308,3 +1308,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 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 | +| 2026-07-30 | PR #1394 / `claude/top-search-design-mockups-w53znc` | `4a001efadedea6e8f8ad59ac7374ff9293cf7e14` | CI/review closeout after format + main sync | FIXED. Tip `66c5eb2c` failed Static PR / CircleCI solely on prettier padding in `#096` row; fixed on `61314887`. Main synced via `4a001efa` (shallow-clone inventory refusal from #1392). No open review threads; layout/`/tools` false-positive already fixed; `#115` remains deferred. No product-code change this pass. | format:check pass; Static PR pass; Unit coverage pass; PR required pass; CircleCI pass; vitest adoption 6/6; typecheck; Bugbot no open P0/P1; merge-tree clean vs main | From 2ac85d5b11a441f8dac3a1a145968053a89375f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 03:43:19 +0000 Subject: [PATCH 03/11] docs(agents): require committing the formatter output, not just running it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex found my own instruction incomplete, and in a way that could mislead: a push sends commits, not the working tree, so running the formatter after committing leaves the unformatted blob on the branch. Worse, guard-push.mjs runs prettier --check against working-tree paths, so it reports clean while the pushed commit is still unformatted. Proved it in a scratch repo — commit `const a = 1`, format, and the working tree reads `const a = 1;` while `git show HEAD:f.ts` still has the original; --check on the path passes, --check on the committed blob fails. A green guard is therefore not evidence. Rewrote the bullet to require committing the formatter output and to name both traps explicitly: that one, and the per-file check that passes on the source file you edited while a doc or ledger edit in the same push fails — which was the missed file in two of the three CI breaks. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- AGENTS.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index bcf63bf5c1..d3f195289f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,7 +166,12 @@ When a branch or PR review completes, record it with `npm run ledger:append -- - - For non-trivial source/config/test changes, prefer `npm run verify:cheap` as the first broad gate and `npm run verify:pr-local` before PR handoff when the change is ready. The PR-local gate runs the full unit suite once, then conditionally adds the production build/client-bundle scan and RAG fixture/manifest validation. Browser, dependency-audit, Docker/Supabase replay, and provider-backed checks remain separate gates. Use `npm run verify:pr-local -- --dry-run --files ` to inspect selection without running commands. The broader `--extended` plan is dry-run only unless explicit approval is reflected by `ALLOW_EXTENDED_PR_LOCAL=true`. - Let the repository run coordinator control cross-worktree verification. It permits at most two focused Vitest/read-only typecheck leases from different worktrees; full Vitest, coverage, lint, build, Playwright, and live-provider tests remain exclusive. Do not install while a repository test, build, lint, typecheck, or server command is active. Avoid aggressive short-interval polling, and do not repeat an unchanged full gate after it passes. - For UI, frontend, browser, routing, styling, reduced-motion, or forced-colors changes, run `npm run ensure` before browser work and use `npm run verify:ui` as the Chromium UI gate. For phone-chrome changes, run `npm run verify:phone-chrome` first: it checks installed-lock parity, selects the affected browser/PWA owners and exact journeys, and adds `verify:ui` last only when shared chrome foundations make the broad gate necessary. Inspect uncertain scope with `-- --dry-run`. -- **Run `npm run format` before every push.** `format:check` is in neither `npm run test`, `npm run typecheck`, nor `npm run lint`, so the ordinary loop reports green while `Static PR checks` and `ci/circleci: verify` both fail on `prettier --check .`. A focused `prettier --check ` on the file you edited is not enough — the repository-wide check covers every file in the push, and a ledger or doc edit made alongside a source change is the usual thing that gets missed. `.githooks/pre-push` already carries a `format-before-push` guard, but `core.hooksPath` is set by this checkout's `npm install`, so an agent pushing from its own environment bypasses it and only CI catches the break. Three CI failures on 2026-07-30 came from exactly this, every one from a push made outside a hook-configured checkout. +- **Run `npm run format` and commit the result before every push.** `format:check` is in neither `npm run test`, `npm run typecheck`, nor `npm run lint`, so the ordinary loop reports green while `Static PR checks` and `ci/circleci: verify` both fail on `prettier --check .`. Three CI failures on 2026-07-30 came from exactly this. Two traps beyond simply running it: + - **Formatting without committing does nothing for the push.** A push sends commits, not your working tree, so formatting after committing leaves the unformatted blob on the branch. Worse, `guard-push.mjs` runs `prettier --check` against working-tree **paths**, so it reports clean while the pushed commit is still unformatted — the guard passing is not evidence. Amend or add a follow-up commit. + - **A per-file check is not the repository-wide check.** `prettier --check ` on the source file you edited passes while a doc or ledger edit in the same push fails; that was the missed file twice out of three. + + `.githooks/pre-push` carries the guard, but `core.hooksPath` is set by this checkout's `npm install`, so an agent pushing from its own environment bypasses it entirely and only CI catches the break. + - For release or handoff confidence, use `npm run verify:release`; this includes the full Playwright project set. - For clinical ingestion, answer generation, source governance, privacy, production-readiness, or environment changes, run the smallest relevant domain check plus `npm run check:production-readiness`. - For pull requests that touch ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output, complete the clinical governance preflight in `.github/pull_request_template.md`. From c0647aea974f96c945f6a774a4d8b07d6e335e77 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 04:01:25 +0000 Subject: [PATCH 04/11] fix(guard,test): check pushed blobs, and require a rendered mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps, both proven before fixing and both re-proven after. guard-push's format guard ran prettier against working-tree paths, but a push sends commits. Commit `const a = 1`, run npm run format, and --check on the path passed while git show HEAD: was still unformatted — the guard went green and CI failed. It now reads the pushed blob via git show :, fed through --stdin-filepath so Prettier still picks the parser from the real path and still honours .prettierignore for it. Verified: an unformatted commit with a formatted working tree now exits 1, a formatted commit exits 0, and SKIP_FORMAT_GUARD=1 still overrides. Unknown verdicts (deleted blob, no parser) stay fail-open so nothing new can block a push. The band adoption gate followed every import specifier, so a route that kept its results import but stopped rendering it still counted as adoption — the exact regression the gate exists to catch. It now parses with @babel/parser and follows a static import only when the binding is mounted: JSX element, default re-export, or named re-export. import(), export-from and side-effect imports stay unconditional, since each is a mount mechanism rather than a binding that can go unused, and the lazy form is the only route to the code-split dashboard workspaces. Verified by gutting (search-app)/services/page.tsx to
with its imports intact: previously green, now an orphan; restored, it passes. Pinned by temp-dir fixtures for imported-but-unrendered, default re-export, and lazy import. Closes #115. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- docs/outstanding-issues.md | 2 +- scripts/guard-push.mjs | 102 +++++++++++----- tests/search-results-band-adoption.test.ts | 135 +++++++++++++++++++-- 3 files changed, 203 insertions(+), 36 deletions(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 0bdfc446a6..1fb32fc8f7 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -152,7 +152,7 @@ removed after current-main verification; it is not missing recommended work. | #112 | P2 | issue | `issues:next-id` has no concurrency protection | **Outcome:** two agents working the same hour cannot allocate the same ledger id. **Detail:** the marker at the top of this file is a plain HTML comment read-modify-written by whoever edits next, with no lock and no post-merge check. On 2026-07-29 it collided **twice in one hour**: PR #1391 claimed `#096`/`#097`, lost them, claimed `#098`/`#099`, lost those too, and its final pair `#108`/`#109` collided a third time with the branch-cleanup work that reached `main` first — resolved during the #1391 merge by renumbering to `#110`/`#111` and bumping the marker to 112. Each collision is silent: `docs/outstanding-issues.md` has NO union merge driver (unlike `docs/branch-review-ledger.md`), so it presents as an ordinary content conflict that a hurried resolution can settle by dropping one side's rows entirely. Nothing checks for duplicate ids afterwards. **Next:** add a duplicate-id and marker-consistency check to the verification gates — the cheapest useful form is a test asserting every `#NNN` id appears exactly once across both tables and that the marker exceeds the max, which turns a silent loss into a red gate. Consider`merge=union`in`.gitattributes` as well, though row-level union does not by itself prevent two rows sharing an id. **Stop:** do not resolve one of these conflicts by taking one side wholesale without diffing the id sets first; that is how rows get dropped. | session 2026-07-29 PR sweep; PR #1391 conflict resolution; `.gitattributes` | 2026-07-29 | | #113 | P2 | issue | `ModeNav` clips its labels at every phone width on `main` | **Outcome:** the mode navigation bar honours its own "labels are never abbreviated" contract, or does not render. **Detail:** measured in Chromium against the running app on 2026-07-29 (PR #1390 review, landed as `8f861bb0`), `span.truncate` `scrollWidth` vs `clientWidth` on `/therapy-compass/search`: 320px viewport clips `Compare` to **15 of 56px**; 360px → 29px; 390px → 39px; 412px → 46px; **430px is the worst case at 3 of 4 labels clipped** (`Compare` 17px, `Recommend` 57 of 77, `Pathways` 57 of 60) because crossing the 26rem band adds a fourth slot to the same space; clean only from ~35rem. Root cause is `grid-auto-columns: 1fr` (`globals.css:2373`) in both lower bands — equal tracks make the WIDEST slot set what every slot needs — combined with `truncate`, so the shortfall is silent. The CSS block's stated budgets are wrong: it claims four labels need 394px, measured intrinsic widths are Search 92.4 / Compare 144.3 / Recommend 125.6 / Pathways 108.5 = **471px**, short by roughly the `0/4` badge plus its gap. **Next:** pick one — (a) content-size every band (adopt the ≥34rem `display: flex` shape throughout) and move thresholds to ~21rem for three slots and ~31rem for four; (b) drop the count badge below the top band, worth ~38px; (c) raise the thresholds so phones keep the collapsed control. (a) is the smallest change and deletes a layout mode. Add the band-boundary browser assertion review asked for, covering the widest label in the set. **Stop:** a hardcoded `rem` threshold cannot guarantee fit for an arbitrary item list, and `ModeNav` is shared — do not treat a Therapy-tuned number as a general solution. | PR #1390 thread `PRRT_kwDOSh5Fis6UyB5X` (open at merge); measured session 2026-07-29 | 2026-07-29 | | #114 | P2 | issue | The live Web-Vitals instrument cannot measure its own noise | **Outcome:** the `#017` baseline rests on evidence whose reproducibility can be checked. **Detail:** `live-web-vitals.yml` (landed `8dbfc5d1`) runs Lighthouse **once** per route/strategy. `#017` asks for reproducible evidence and says to stop when the evidence is too noisy — one sample can neither establish reproducibility nor recognise noise, so the instrument cannot detect the condition its own governing item tells the operator to stop on. The rule is a hard threshold (LCP < 2500ms), so a route near the line resolves to a pass or a breach on run-to-run variance alone, invisibly, and a favourable sample would mark **seven** gated findings WONTFIX. Related and also unclosed: Chrome ships with the `ubuntu-24.04` runner image and is NOT pinned by `LIGHTHOUSE_VERSION`, so a metric shift between a baseline and its follow-up can originate in the browser; the build is now recorded per report in `summary.json` (`chromeVersions`) so a cross-version comparison is visibly invalid, but nothing prevents one. **Next:** N runs per route/strategy with a sample-indexed report name, `expectedRuns`/`incompleteEvidence` reworked to expect N per cell, median as the graded aggregate (the Lighthouse and `lighthouse-ci` convention), and — most important — a spread that straddles a threshold treated as INCOMPLETE EVIDENCE rather than resolved either way. Costs N× dispatch time. Pinning Chrome needs a container or a setup action. **Stop:** do not record an `#017` verdict from a single-sample run, and do not compare baselines whose `chromeVersions` differ. | PR #1385 thread `PRRT_kwDOSh5Fis6U0Zq0` (open at merge); `scripts/summarise-web-vitals.mjs` | 2026-07-29 | -| #115 | 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 | +| #115 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` now parses each file with `@babel/parser` and follows a static `import { X } from "…"` only when `X` is actually mounted — rendered as JSX, re-exported as the default (a wrapper page mounts with no JSX of its own), or re-exported by name. `import("…")`, `export … from "…"` and side-effect imports are followed unconditionally, because each is itself a mount mechanism rather than a binding that can go unused; the lazy form is required for the band behind the code-split dashboard workspaces. Verified by reducing `(search-app)/services/page.tsx` to `
` with its imports intact: previously green, now reported as an orphan; restored, it passes. Pinned by temp-dir fixtures covering imported-but-unrendered (false), default re-export (true) and lazy `import()` (true). The residual `void Binding` / type-only-reference shape is unchanged and still caught by `no-unused-vars` under `--max-warnings 0`. | PR #1400; session 2026-07-30 | 2026-07-30 | ## Resolved / archive diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index 432f4925b1..4bc7e2c5fa 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -15,8 +15,10 @@ * * 2. Format-before-push * verify:cheap does NOT run format:check but CI requires it, so unformatted - * files reach CI and fail there. Runs `prettier --check` on the files in the - * push range. Override: SKIP_FORMAT_GUARD=1. + * files reach CI and fail there. Checks the *pushed blobs* (`git show + * :`) for the files in the push range — not the working copies, + * because a push sends commits: formatting after committing left the guard + * green and CI red. Override: SKIP_FORMAT_GUARD=1. * * 3. Drift-manifest freshness * Editing supabase/schema.sql without regenerating supabase/drift-manifest.json @@ -108,6 +110,25 @@ function collectChangedFiles(ranges) { return [...files]; } +/** + * Changed files paired with the commit they are being pushed at. + * + * A push sends commits, not the working tree. Checking `` on disk lets a + * formatted working copy vouch for an unformatted committed blob: commit + * `const a = 1`, run `npm run format`, and `prettier --check ` passes + * while `git show HEAD:` is still unformatted — so the guard went green and + * CI failed anyway. Carry the sha so the guard can read what is actually pushed. + */ +function collectChangedBlobs(ranges) { + const seen = new Map(); + for (const range of ranges) { + for (const file of changedFilesForRange(range)) { + seen.set(`${range.localSha}:${file}`, { sha: range.localSha, file }); + } + } + return [...seen.values()]; +} + // --------------------------------------------------------------------------- // Guard 1: auto-merge race sentinel // --------------------------------------------------------------------------- @@ -174,40 +195,66 @@ function resolvePrettierBin() { return path.join(path.dirname(pkgJson), "bin", "prettier.cjs"); } -function formatGuard(changedFiles) { +/** + * Is the pushed content of `file` already Prettier-formatted? + * + * Fed through stdin with `--stdin-filepath` so Prettier still picks the parser + * from the real path and still honours `.prettierignore` for it — an ignored + * path comes back byte-identical and therefore reads as formatted. Returns null + * when the verdict is unknown (unreadable blob, no parser), so an unknown never + * blocks a push. + */ +function pushedBlobIsFormatted(prettierBin, sha, file) { + let committed; + try { + committed = execFileSync("git", ["show", `${sha}:${file}`], { + encoding: "utf8", + maxBuffer: 32 * 1024 * 1024, + stdio: ["ignore", "pipe", "ignore"], + }); + } catch { + // Deleted in this push, or otherwise not present at this sha. + return null; + } + try { + const formatted = execFileSync(process.execPath, [prettierBin, "--ignore-unknown", "--stdin-filepath", file], { + input: committed, + encoding: "utf8", + maxBuffer: 32 * 1024 * 1024, + stdio: ["pipe", "pipe", "ignore"], + }); + return formatted === committed; + } catch { + return null; + } +} + +function formatGuard(changedBlobs) { if (process.env.SKIP_FORMAT_GUARD === "1") { return { name: "format", ok: true, skipped: "SKIP_FORMAT_GUARD=1" }; } - const existing = changedFiles.filter((f) => existsSync(f)); - if (existing.length === 0) return { name: "format", ok: true }; + if (changedBlobs.length === 0) return { name: "format", ok: true }; let prettierBin; try { prettierBin = resolvePrettierBin(); } catch { return { name: "format", ok: true, note: "prettier not resolvable — format check skipped" }; } - try { - // prettier respects .prettierignore for listed paths; --ignore-unknown skips - // files it has no parser for (e.g. images) without failing. - execFileSync(process.execPath, [prettierBin, "--check", "--ignore-unknown", ...existing], { - stdio: ["ignore", "pipe", "pipe"], - }); - return { name: "format", ok: true }; - } catch (error) { - const detail = [error.stdout, error.stderr] - .map((b) => (b ? b.toString() : "")) - .join("") - .trim(); - return { - name: "format", - ok: false, - message: - `Prettier found unformatted files in this push (CI format:check would fail):\n` + - (detail ? `${detail}\n` : "") + - ` Fix with: npm run format\n` + - ` To push anyway: SKIP_FORMAT_GUARD=1 git push`, - }; + const unformatted = []; + for (const { sha, file } of changedBlobs) { + if (pushedBlobIsFormatted(prettierBin, sha, file) === false) unformatted.push(file); } + if (unformatted.length === 0) return { name: "format", ok: true }; + return { + name: "format", + ok: false, + message: + `Prettier found unformatted files in this push (CI format:check would fail):\n` + + unformatted.map((file) => ` ${file}\n`).join("") + + ` These are the committed contents, not your working copy — run \`npm run format\`\n` + + ` and commit the result (amend or a follow-up commit).\n` + + ` To push anyway: SKIP_FORMAT_GUARD=1 git push`, + }; } // --------------------------------------------------------------------------- @@ -273,7 +320,8 @@ function main() { if (ranges.length === 0) process.exit(0); // deletion-only push or nothing to do const branch = currentBranch(); const changedFiles = collectChangedFiles(ranges); - const results = [autoMergeGuard(branch), formatGuard(changedFiles), driftGuard(changedFiles)]; + // formatGuard reads the pushed blobs; driftGuard only needs the paths. + const results = [autoMergeGuard(branch), formatGuard(collectChangedBlobs(ranges)), driftGuard(changedFiles)]; process.exit(report(results)); } diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index 0edffec318..6053cc9ddf 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -2,6 +2,7 @@ import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSy import { tmpdir } from "node:os"; import path from "node:path"; +import { parse } from "@babel/parser"; import { describe, expect, it } from "vitest"; import { appModeDefinitions } from "@/lib/app-modes"; @@ -157,11 +158,111 @@ function reachabilityRoots(routeAbs: string): string[] { return [routeAbs, ...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; +/** + * Specifiers worth following out of one file, and only those. + * + * A plain "every specifier in the file" walk reports adoption for a route that + * keeps its results import but stops rendering it — reduce + * `(search-app)/services/page.tsx` to `
` with its imports intact and the + * gate stayed green, which is the regression it exists to catch. So a static + * `import { X } from "…"` is followed only when `X` is actually mounted: + * rendered as JSX, re-exported as the default (a wrapper page mounts with no JSX + * of its own), or re-exported by name. + * + * Three forms are followed unconditionally, because each *is* a mount mechanism + * rather than a binding that might go unused: + * - `import("…")` — how `clinical-dashboard-lazy.tsx` code-splits the mode + * workspaces, so the band behind Differentials/Favourites/prescribing is only + * reachable this way + * - `export … from "…"` — a barrel or pass-through re-export + * - side-effect `import "…"` + */ +function followableSpecifiers(source: string, filename: string): string[] { + let ast: ReturnType; + try { + ast = parse(source, { sourceType: "unambiguous", plugins: ["jsx", "typescript"] }); + } catch { + // An unparseable file must not silently drop out of the walk. + throw new Error(`search-results-band-adoption: could not parse ${filename}`); + } + + const body = (ast.program?.body ?? []) as unknown[]; + const mounted = new Set(); + const staticImports: Array<{ source: string; locals: string[] }> = []; + const always: string[] = []; + + const visit = (value: unknown) => { + if (!value || typeof value !== "object") return; + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + const node = value as Record; + const type = node.type; + + // and — the element name is a mount. + if (type === "JSXOpeningElement") { + let name = node.name as Record | undefined; + while (name && name.type === "JSXMemberExpression") name = name.object as Record; + if (name && name.type === "JSXIdentifier" && typeof name.name === "string") mounted.add(name.name); + } + // `export default X` mounts X without any JSX in this file. + if (type === "ExportDefaultDeclaration") { + const decl = node.declaration as Record | undefined; + if (decl && decl.type === "Identifier" && typeof decl.name === "string") mounted.add(decl.name); + } + // `export { X }` — the mount happens in whatever imports it. + if (type === "ExportNamedDeclaration" && !node.source) { + for (const spec of (node.specifiers ?? []) as Array>) { + const local = spec.local as Record | undefined; + if (local && typeof local.name === "string") mounted.add(local.name); + } + } + // `import("…")`, including inside dynamic(() => import("…")). + if ( + type === "ImportExpression" || + (type === "CallExpression" && (node.callee as { type?: string })?.type === "Import") + ) { + const arg = (node.source ?? (node.arguments as unknown[])?.[0] ?? null) as Record | null; + if (arg && arg.type === "StringLiteral" && typeof arg.value === "string") always.push(arg.value); + } + for (const key of Object.keys(node)) { + if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; + visit(node[key]); + } + }; + + for (const raw of body) { + const statement = raw as Record; + if (statement.type === "ImportDeclaration") { + const spec = (statement.source as { value?: string })?.value; + if (typeof spec !== "string") continue; + const locals = ((statement.specifiers ?? []) as Array>) + .map((s) => (s.local as { name?: string })?.name) + .filter((n): n is string => typeof n === "string"); + // A bare `import "x"` has no bindings and is a side effect — always follow. + if (locals.length === 0) always.push(spec); + else staticImports.push({ source: spec, locals }); + continue; + } + if ( + (statement.type === "ExportNamedDeclaration" || statement.type === "ExportAllDeclaration") && + statement.source + ) { + const spec = (statement.source as { value?: string })?.value; + if (typeof spec === "string") always.push(spec); + continue; + } + } + // Mount detection has to see the whole program, imports included, because a + // re-export statement is both. + visit(body); + + const followed = staticImports + .filter(({ locals }) => locals.some((local) => mounted.has(local))) + .map((i) => i.source); + return [...new Set([...always, ...followed])]; +} function resolveSpecifier(specifier: string, fromFile: string): string | null { let base: string; @@ -198,9 +299,7 @@ function routeReachesBand(routeAbs: string): boolean { } 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; + for (const specifier of followableSpecifiers(source, file)) { const target = resolveSpecifier(specifier, file); if (!target || seen.has(target) || isMockupPath(target)) continue; seen.add(target); @@ -394,6 +493,26 @@ describe("band adoption detection", () => { "utf8", ); expect(routeReachesBand(path.join(dir, "wired-route.tsx"))).toBe(true); + + // The regression this gate exists to catch: the results component is still + // imported, but nothing renders it. A walk that follows every specifier + // reports adoption here, which is how gutting a real page to `
` with + // its imports intact stayed green. + writeFileSync( + path.join(dir, "imported-only-route.tsx"), + 'import { Banded } from "./banded-child";\nexport default function Page() {\n return
;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "imported-only-route.tsx"))).toBe(false); + + // A wrapper page mounts with no JSX of its own, so a default re-export of + // an imported binding has to count. + writeFileSync( + path.join(dir, "reexport-route.tsx"), + 'import { Banded } from "./banded-child";\nexport default Banded;\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "reexport-route.tsx"))).toBe(true); } finally { rmSync(dir, { recursive: true, force: true }); } From fc166c8fb290e84e2944738df06003b897f8fae4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 04:11:33 +0000 Subject: [PATCH 05/11] fix(guard): block when Prettier cannot check the push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex found me over-applying fail-open. A malformed prettier config in the push makes Prettier exit non-zero; my catch returned null and formatGuard only blocked on an explicit false, so a config error let the push through while CI's prettier --check . failed for the same reason. That is the guard waving through exactly the break it exists to catch. Three verdicts now, each verified in its own scratch repo so a stray config could not confound the others: unformatted commit -> exit 1, "found unformatted files" formatted commit -> exit 0, silent malformed config -> exit 1, "could not check this push" Fail-open is now reserved for a blob that is absent at the pushed sha — deleted in this push, so CI has nothing to check either. --ignore-unknown still covers the benign no-parser case, so any other non-zero exit is a real problem and is surfaced with Prettier's own stderr. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- scripts/guard-push.mjs | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index 4bc7e2c5fa..342e76d8ac 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -213,19 +213,29 @@ function pushedBlobIsFormatted(prettierBin, sha, file) { stdio: ["ignore", "pipe", "ignore"], }); } catch { - // Deleted in this push, or otherwise not present at this sha. - return null; + // Deleted in this push, or otherwise not present at this sha: nothing to + // check, and nothing CI will check either. Skip. + return { verdict: "skip" }; } try { const formatted = execFileSync(process.execPath, [prettierBin, "--ignore-unknown", "--stdin-filepath", file], { input: committed, encoding: "utf8", maxBuffer: 32 * 1024 * 1024, - stdio: ["pipe", "pipe", "ignore"], + stdio: ["pipe", "pipe", "pipe"], }); - return formatted === committed; - } catch { - return null; + return { verdict: formatted === committed ? "formatted" : "unformatted" }; + } catch (error) { + // Prettier itself failed — a malformed .prettierrc in this push exits 2, and + // `prettier --check .` in CI will fail for the same reason. Treating that as + // "unknown, allow" is how the guard would wave through the break it exists to + // catch, so surface it instead. `--ignore-unknown` already covers the benign + // no-parser case, so a non-zero exit here is a real problem. + const detail = [error?.stderr, error?.stdout] + .map((b) => (b ? b.toString() : "")) + .join("") + .trim(); + return { verdict: "error", detail: detail || `prettier exited ${error?.status ?? "non-zero"} for ${file}` }; } } @@ -241,8 +251,22 @@ function formatGuard(changedBlobs) { return { name: "format", ok: true, note: "prettier not resolvable — format check skipped" }; } const unformatted = []; + const errors = []; for (const { sha, file } of changedBlobs) { - if (pushedBlobIsFormatted(prettierBin, sha, file) === false) unformatted.push(file); + const { verdict, detail } = pushedBlobIsFormatted(prettierBin, sha, file); + if (verdict === "unformatted") unformatted.push(file); + else if (verdict === "error") errors.push(detail); + } + if (errors.length > 0) { + return { + name: "format", + ok: false, + message: + `Prettier could not check this push, so CI's \`prettier --check .\` will fail too:\n` + + errors.map((detail) => ` ${detail}\n`).join("") + + ` A malformed prettier config in the push is the usual cause.\n` + + ` To push anyway: SKIP_FORMAT_GUARD=1 git push`, + }; } if (unformatted.length === 0) return { name: "format", ok: true }; return { From 055df85698efe2d91ee3a5ec8338da1f22749fdc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 04:19:31 +0000 Subject: [PATCH 06/11] fix(test): stop counting a named re-export as a rendered mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A page that re-exported a banded component while its own default rendered
reported adoption: the walker treated `export { Banded }` as a mount and followed the import. Re-exporting renders nothing, so drop that branch — wherever the re-export is finally mounted, the walk sees the JSX there. Pinned by a fixture that fails against the previous behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- tests/search-results-band-adoption.test.ts | 30 +++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index 6053cc9ddf..80b2aef211 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -166,8 +166,11 @@ function reachabilityRoots(routeAbs: string): string[] { * `(search-app)/services/page.tsx` to `
` with its imports intact and the * gate stayed green, which is the regression it exists to catch. So a static * `import { X } from "…"` is followed only when `X` is actually mounted: - * rendered as JSX, re-exported as the default (a wrapper page mounts with no JSX - * of its own), or re-exported by name. + * rendered as JSX, or re-exported as the default (a wrapper page mounts with no + * JSX of its own). A named re-export — `export { X }` — is *not* a mount: it + * renders nothing, so counting it let a page re-export a banded component while + * its own default rendered `
`. Wherever that re-export is finally + * mounted, this walk sees the JSX there instead. * * Three forms are followed unconditionally, because each *is* a mount mechanism * rather than a binding that might go unused: @@ -211,13 +214,10 @@ function followableSpecifiers(source: string, filename: string): string[] { const decl = node.declaration as Record | undefined; if (decl && decl.type === "Identifier" && typeof decl.name === "string") mounted.add(decl.name); } - // `export { X }` — the mount happens in whatever imports it. - if (type === "ExportNamedDeclaration" && !node.source) { - for (const spec of (node.specifiers ?? []) as Array>) { - const local = spec.local as Record | undefined; - if (local && typeof local.name === "string") mounted.add(local.name); - } - } + // Deliberately NOT a mount: `export { X }` for an imported `X`. Re-exporting + // a binding renders nothing, so a page that re-exports a banded component + // while its own default renders `
` would otherwise report adoption. + // A consumer that mounts the re-export is caught where it mounts it. // `import("…")`, including inside dynamic(() => import("…")). if ( type === "ImportExpression" || @@ -513,6 +513,18 @@ describe("band adoption detection", () => { "utf8", ); expect(routeReachesBand(path.join(dir, "reexport-route.tsx"))).toBe(true); + + // A *named* re-export is not a mount. Re-exporting `Banded` renders + // nothing, so a page whose own default renders `
` must still be + // reported as an orphan — the false green Codex found on #1400. + writeFileSync( + path.join(dir, "named-reexport-route.tsx"), + 'import { Banded } from "./banded-child";\n' + + "export { Banded };\n" + + "export default function Page() {\n return
;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "named-reexport-route.tsx"))).toBe(false); } finally { rmSync(dir, { recursive: true, force: true }); } From 20235ccba1413c54f3e34208adf5ffbd6144f511 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 04:40:53 +0000 Subject: [PATCH 07/11] fix(guard): verify formatting in a checkout of the pushed commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both about working-tree state leaking into the verdict: - A dynamic `prettier.config.mjs` could not be evaluated by the previous hand-staged config approach, so the guard fell back to the working-tree config — recreating the mismatch it exists to prevent. - A push that changes prettier policy (.prettierrc, .prettierignore, .editorconfig, package.json) changes the verdict for files the push never touched, which a changed-paths-only check cannot see. Both are answered by checking the pushed commit where CI checks it: a `git worktree` of that sha (<1s here) with node_modules linked in, so a dynamic config loads its plugins. A policy change escalates to a whole-tree `prettier --check .`; otherwise only the changed paths are checked. Failure to create the worktree now fails closed — being unable to check is not evidence the push is clean. Verified on six isolated scratch repos, exit codes 1/0/1/1/1/0: committed unformatted with a clean working tree blocks; all-clean passes; a committed-only broken static config blocks; a committed-only broken dynamic config blocks (was passing); a config-only change that breaks an untouched file blocks (was passing); a benign config change still passes. Also records #116: an unmergeable PR runs no CI at all and says nothing, which is what cost this branch three pushes of missing checks today. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- docs/outstanding-issues.md | 3 +- scripts/guard-push.mjs | 239 +++++++++++++++---------------------- 2 files changed, 95 insertions(+), 147 deletions(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 0f8aeb9952..c898e2be8d 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 @@ -153,6 +153,7 @@ removed after current-main verification; it is not missing recommended work. | #113 | P2 | issue | `ModeNav` clips its labels at every phone width on `main` | **Outcome:** the mode navigation bar honours its own "labels are never abbreviated" contract, or does not render. **Detail:** measured in Chromium against the running app on 2026-07-29 (PR #1390 review, landed as `8f861bb0`), `span.truncate` `scrollWidth` vs `clientWidth` on `/therapy-compass/search`: 320px viewport clips `Compare` to **15 of 56px**; 360px → 29px; 390px → 39px; 412px → 46px; **430px is the worst case at 3 of 4 labels clipped** (`Compare` 17px, `Recommend` 57 of 77, `Pathways` 57 of 60) because crossing the 26rem band adds a fourth slot to the same space; clean only from ~35rem. Root cause is `grid-auto-columns: 1fr` (`globals.css:2373`) in both lower bands — equal tracks make the WIDEST slot set what every slot needs — combined with `truncate`, so the shortfall is silent. The CSS block's stated budgets are wrong: it claims four labels need 394px, measured intrinsic widths are Search 92.4 / Compare 144.3 / Recommend 125.6 / Pathways 108.5 = **471px**, short by roughly the `0/4` badge plus its gap. **Next:** pick one — (a) content-size every band (adopt the ≥34rem `display: flex` shape throughout) and move thresholds to ~21rem for three slots and ~31rem for four; (b) drop the count badge below the top band, worth ~38px; (c) raise the thresholds so phones keep the collapsed control. (a) is the smallest change and deletes a layout mode. Add the band-boundary browser assertion review asked for, covering the widest label in the set. **Stop:** a hardcoded `rem` threshold cannot guarantee fit for an arbitrary item list, and `ModeNav` is shared — do not treat a Therapy-tuned number as a general solution. | PR #1390 thread `PRRT_kwDOSh5Fis6UyB5X` (open at merge); measured session 2026-07-29 | 2026-07-29 | | #114 | P2 | issue | The live Web-Vitals instrument cannot measure its own noise | **Outcome:** the `#017` baseline rests on evidence whose reproducibility can be checked. **Detail:** `live-web-vitals.yml` (landed `8dbfc5d1`) runs Lighthouse **once** per route/strategy. `#017` asks for reproducible evidence and says to stop when the evidence is too noisy — one sample can neither establish reproducibility nor recognise noise, so the instrument cannot detect the condition its own governing item tells the operator to stop on. The rule is a hard threshold (LCP < 2500ms), so a route near the line resolves to a pass or a breach on run-to-run variance alone, invisibly, and a favourable sample would mark **seven** gated findings WONTFIX. Related and also unclosed: Chrome ships with the `ubuntu-24.04` runner image and is NOT pinned by `LIGHTHOUSE_VERSION`, so a metric shift between a baseline and its follow-up can originate in the browser; the build is now recorded per report in `summary.json` (`chromeVersions`) so a cross-version comparison is visibly invalid, but nothing prevents one. **Next:** N runs per route/strategy with a sample-indexed report name, `expectedRuns`/`incompleteEvidence` reworked to expect N per cell, median as the graded aggregate (the Lighthouse and `lighthouse-ci` convention), and — most important — a spread that straddles a threshold treated as INCOMPLETE EVIDENCE rather than resolved either way. Costs N× dispatch time. Pinning Chrome needs a container or a setup action. **Stop:** do not record an `#017` verdict from a single-sample run, and do not compare baselines whose `chromeVersions` differ. | PR #1385 thread `PRRT_kwDOSh5Fis6U0Zq0` (open at merge); `scripts/summarise-web-vitals.mjs` | 2026-07-29 | | #115 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` now parses each file with `@babel/parser` and follows a static `import { X } from "…"` only when `X` is actually mounted — rendered as JSX, re-exported as the default (a wrapper page mounts with no JSX of its own), or re-exported by name. `import("…")`, `export … from "…"` and side-effect imports are followed unconditionally, because each is itself a mount mechanism rather than a binding that can go unused; the lazy form is required for the band behind the code-split dashboard workspaces. Verified by reducing `(search-app)/services/page.tsx` to `
` with its imports intact: previously green, now reported as an orphan; restored, it passes. Pinned by temp-dir fixtures covering imported-but-unrendered (false), default re-export (true) and lazy `import()` (true). The residual `void Binding` / type-only-reference shape is unchanged and still caught by `no-unused-vars` under `--max-warnings 0`. | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` now parses each file with `@babel/parser` and follows a static `import { X } from "…"` only when `X` is actually mounted — rendered as JSX, or supplied as this module's default export. Followed unconditionally: `import("…")` (required for the band behind the code-split dashboard workspaces), `export { default } from "…"`, and side-effect imports. **Deliberately not followed:** `export { X }`, `export { X } from "…"` and `export * from "…"` — a re-export renders nothing, so counting one let a page re-export a banded component while its own default rendered `
`; wherever the re-export is finally mounted, the walk sees the JSX there. Verified by reducing `(search-app)/services/page.tsx` to `
` with its imports intact: previously green, now reported as an orphan; restored, it passes. Pinned by temp-dir fixtures covering imported-but-unrendered, all three re-export spellings (false) and default re-export plus lazy `import()` (true), each confirmed to fail against the prior behaviour. Two of the three false greens were found in review after a fix landed, which is why the fixtures assert direction rather than only passing. The residual `void Binding` / type-only-reference shape is unchanged and still caught by `no-unused-vars` under `--max-warnings 0`. Remaining known gap, tracked here rather than left silent: a dynamic `import()` is followed regardless of which lazy binding is mounted, so `clinical-dashboard-lazy.tsx` mounting any one lazy export reaches them all — closing that needs the walk to track bindings per file, not just files. | 2026-07-30 | +| #116 | P2 | issue | An unmergeable PR runs no CI at all, with no signal that it stopped | **Outcome:** a PR that has silently stopped being tested says so. **Detail:** on 2026-07-30 PR #1400 ran **no** `CI`, `Gitleaks` or `Semgrep` workflow across three consecutive pushes, and nothing anywhere said why. Cause: the PR had a real content conflict with `main`, so GitHub could not build `refs/pull/1400/merge`, and every `pull_request`-triggered workflow is skipped in that state. `pull_request_target` ones (`PR Policy`) still ran, and CircleCI posted failure ~3s after each push - faster than a checkout, so no step executed. The symptom reads as "CI is broken" or "my tests fail", and about 40 minutes went into replicating CI steps locally (all green) before the cause was found. **Diagnostic:** compare check-run counts with another open PR (3 vs 16-19), list workflow runs and note only `pull_request_target` fired, then confirm with `git merge-tree --write-tree origin/main HEAD`. **Next:** make staleness visible instead of silent - e.g. a `pull_request_target` job that fails when `mergeable_state` is `dirty`, so a conflicted PR shows one red check naming the conflict rather than an empty check list. `npm run sync:pr-branches` covers the behind-but-clean case only. **Stop:** never conclude "CI is failing" from a missing check - the absence of a check is not a failing check. | PR #1400; session 2026-07-30 | 2026-07-30 | ## Resolved / archive diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index ca4f9b50d0..0d749a64b3 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -33,7 +33,7 @@ */ import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync } from "node:fs"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -199,140 +199,107 @@ function resolvePrettierBin() { } /** - * Files in the pushed commit that change Prettier's verdict. + * Does this path decide Prettier's verdict for files other than itself? * - * The config has to come from the pushed tree, not from disk. Commit a broken - * `.prettierrc`, fix it in the working tree without committing, and a guard that - * reads the working-tree config approves a push that CI — which only ever sees - * the pushed SHA — rejects. + * Matched on the basename so a nested config counts too. When one of these + * changes, the verdict for *unchanged* files can change with it — a `tabWidth` + * edit can make existing source fail CI's repository-wide `prettier --check .` + * while a changed-paths-only check passes — so a policy change escalates to a + * whole-tree check. */ -const PRETTIER_CONFIG_FILES = [ - ".prettierrc", - ".prettierrc.json", - ".prettierrc.json5", - ".prettierrc.yml", - ".prettierrc.yaml", - ".prettierrc.toml", - ".prettierignore", - ".editorconfig", - "package.json", -]; - -/** - * Config formats that cannot be evaluated outside the repo — they are modules - * and may import plugins from `node_modules`. Copying one into a scratch tree - * would make Prettier fail to load it, so their presence downgrades the guard to - * the working-tree config with a visible note instead of a wrong verdict. - */ -const DYNAMIC_PRETTIER_CONFIG_FILES = [ - ".prettierrc.js", - ".prettierrc.cjs", - ".prettierrc.mjs", - ".prettierrc.ts", - "prettier.config.js", - "prettier.config.cjs", - "prettier.config.mjs", - "prettier.config.ts", -]; - -function pushedBlob(sha, file) { - try { - return execFileSync("git", ["show", `${sha}:${file}`], { - maxBuffer: 64 * 1024 * 1024, - stdio: ["ignore", "pipe", "ignore"], - }); - } catch { - // Absent at this sha (deleted in the push, or never existed). - return null; - } +function isPrettierPolicyFile(file) { + return /^(?:\.prettierrc(?:\..+)?|prettier\.config\.(?:js|cjs|mjs|ts)|\.prettierignore|\.editorconfig|package\.json)$/.test( + path.basename(file), + ); } /** - * Materialise the pushed SHA's config plus the pushed contents of `files` into a - * scratch directory, at their repo-relative paths. + * Check the pushed commit the way CI does: in a real checkout of it. * - * Running Prettier with that directory as cwd reproduces what CI does to a fresh - * checkout: config discovery, `.prettierignore` matching and parser inference all - * resolve against the pushed tree rather than the working copy. + * A worktree is what makes the verdict trustworthy, and it is cheap (<1s here). + * Every earlier attempt leaked working-tree state into the answer: + * - checking `` on disk let a formatted working copy vouch for an + * unformatted committed blob (a push sends commits, not the working tree) + * - piping committed blobs through stdin still resolved `.prettierrc` from disk, + * so a committed-broken/tree-corrected config passed here and failed CI + * - hand-staging config files could not evaluate a dynamic `prettier.config.mjs` + * at all, because it may import plugins from `node_modules` + * A checkout with `node_modules` linked in has none of those gaps. */ -function stagePushedTree(sha, files) { +function checkPushedCommit(prettierBin, sha, files) { + tryGit(["worktree", "prune"]); // clear any worktree a crashed run left behind const dir = mkdtempSync(path.join(tmpdir(), "guard-push-format-")); - const write = (relative, contents) => { - const target = path.join(dir, relative); - mkdirSync(path.dirname(target), { recursive: true }); - writeFileSync(target, contents); - }; - for (const config of PRETTIER_CONFIG_FILES) { - const contents = pushedBlob(sha, config); - if (contents) write(config, contents); + rmSync(dir, { recursive: true, force: true }); // `git worktree add` wants a fresh path + try { + execFileSync("git", ["worktree", "add", "--detach", "--quiet", dir, sha], { + stdio: ["ignore", "ignore", "pipe"], + }); + } catch (error) { + // Fail CLOSED. Being unable to check is not evidence that the push is clean, + // and SKIP_FORMAT_GUARD=1 is the escape hatch when this is genuinely stuck. + const detail = (error?.stderr ? error.stderr.toString() : "").trim(); + return { + verdict: "error", + detail: `could not check out ${sha} to verify formatting${detail ? `: ${detail}` : ""}`, + }; } - const staged = []; - for (const file of files) { - const contents = pushedBlob(sha, file); - // Deleted in this push: nothing to check, and nothing CI will check either. - if (!contents) continue; - write(file, contents); - staged.push(file); + try { + // A dynamic config may import plugins; without this it cannot load at all. + const modules = path.resolve("node_modules"); + if (existsSync(modules)) { + try { + symlinkSync(modules, path.join(dir, "node_modules"), "dir"); + } catch { + // Already present, or symlinks unavailable — prettier still loads static configs. + } + } + const policyChanged = files.some(isPrettierPolicyFile); + // Deleted paths are gone from the checkout, and prettier errors on a missing + // argument, so ask only for what is actually there. + const present = files.filter((file) => existsSync(path.join(dir, file))); + if (!policyChanged && present.length === 0) return { verdict: "formatted" }; + // A policy change alters the verdict for files this push never touched, so + // check the whole tree — exactly what CI's `prettier --check .` does. + const batches = policyChanged ? [["."]] : chunk(present, 200); + for (const batch of batches) { + const result = runPrettierCheck(prettierBin, dir, batch); + if (result.verdict !== "formatted") return result; + } + return { verdict: "formatted" }; + } finally { + tryGit(["worktree", "remove", "--force", dir]); + rmSync(dir, { recursive: true, force: true }); } - return { dir, staged }; } -/** Prettier: 0 clean, 1 unformatted files found, anything else a real failure. */ -function checkStagedTree(prettierBin, dir, staged) { - // Chunked so a large push cannot exceed the platform argument limit. - for (let index = 0; index < staged.length; index += 200) { - const batch = staged.slice(index, index + 200); - try { - execFileSync(process.execPath, [prettierBin, "--check", "--ignore-unknown", ...batch], { - cwd: dir, - stdio: ["ignore", "pipe", "pipe"], - }); - } catch (error) { - const detail = [error?.stdout, error?.stderr] - .map((buffer) => (buffer ? buffer.toString() : "")) - .join("") - .trim(); - // An invalid config option also exits 1 under --check, reported as - // `[error] Invalid tabWidth value…`, so the status alone cannot tell a - // formatting difference from a broken config. - if (error?.status === 1 && !detail.includes("[error]")) return { verdict: "unformatted", detail }; - // A malformed config in the push exits 2 here and fails CI's - // `prettier --check .` for the same reason. Treating it as "unknown, allow" - // is how the guard would wave through the break it exists to catch. - return { verdict: "error", detail: detail || `prettier exited ${error?.status ?? "non-zero"}` }; - } - } - return { verdict: "formatted" }; +function chunk(items, size) { + const out = []; + for (let index = 0; index < items.length; index += size) out.push(items.slice(index, index + size)); + return out; } -/** - * Fallback for a dynamic config: check the pushed blob through stdin against the - * *working-tree* config. `--stdin-filepath` keeps parser inference and - * `.prettierignore` matching keyed to the real path. - */ -function checkViaWorkingTreeConfig(prettierBin, changedBlobs) { - const unformatted = []; - for (const { sha, file } of changedBlobs) { - const committed = pushedBlob(sha, file); - if (!committed) continue; - try { - const formatted = execFileSync(process.execPath, [prettierBin, "--ignore-unknown", "--stdin-filepath", file], { - input: committed, - maxBuffer: 64 * 1024 * 1024, - stdio: ["pipe", "pipe", "pipe"], - }); - if (!formatted.equals(committed)) unformatted.push(file); - } catch (error) { - const detail = [error?.stderr, error?.stdout] - .map((buffer) => (buffer ? buffer.toString() : "")) - .join("") - .trim(); - return { verdict: "error", detail: detail || `prettier exited ${error?.status ?? "non-zero"} for ${file}` }; - } +/** Prettier: 0 clean, 1 unformatted, anything else a real failure. */ +function runPrettierCheck(prettierBin, cwd, args) { + try { + execFileSync(process.execPath, [prettierBin, "--check", "--ignore-unknown", ...args], { + cwd, + stdio: ["ignore", "pipe", "pipe"], + }); + return { verdict: "formatted" }; + } catch (error) { + const detail = [error?.stdout, error?.stderr] + .map((buffer) => (buffer ? buffer.toString() : "")) + .join("") + .trim(); + // An invalid config option also exits 1 under --check, reported as + // `[error] Invalid tabWidth value…`, so the status alone cannot tell a + // formatting difference from a broken config. + if (error?.status === 1 && !detail.includes("[error]")) return { verdict: "unformatted", detail }; + // A malformed or unloadable config in the push fails CI's `prettier --check .` + // for the same reason. Treating it as "unknown, allow" is how the guard would + // wave through the break it exists to catch. + return { verdict: "error", detail: detail || `prettier exited ${error?.status ?? "non-zero"}` }; } - return unformatted.length === 0 - ? { verdict: "formatted" } - : { verdict: "unformatted", detail: unformatted.map((file) => `[warn] ${file}`).join("\n") }; } function formatGuard(changedBlobs) { @@ -355,35 +322,16 @@ function formatGuard(changedBlobs) { const unformatted = []; const errors = []; - let note; for (const [sha, files] of bySha) { - const dynamic = DYNAMIC_PRETTIER_CONFIG_FILES.filter((config) => pushedBlob(sha, config)); - if (dynamic.length > 0) { - note = `pushed ${dynamic.join(", ")} cannot be loaded outside the repo — used the working-tree prettier config`; - const { verdict, detail } = checkViaWorkingTreeConfig( - prettierBin, - changedBlobs.filter((b) => b.sha === sha), - ); - if (verdict === "unformatted") unformatted.push(detail); - else if (verdict === "error") errors.push(detail); - continue; - } - const { dir, staged } = stagePushedTree(sha, files); - try { - if (staged.length === 0) continue; - const { verdict, detail } = checkStagedTree(prettierBin, dir, staged); - if (verdict === "unformatted") unformatted.push(detail); - else if (verdict === "error") errors.push(detail); - } finally { - rmSync(dir, { recursive: true, force: true }); - } + const { verdict, detail } = checkPushedCommit(prettierBin, sha, files); + if (verdict === "unformatted") unformatted.push(detail); + else if (verdict === "error") errors.push(detail); } if (errors.length > 0) { return { name: "format", ok: false, - note, message: `Prettier could not check this push, so CI's \`prettier --check .\` will fail too:\n` + errors.map((detail) => `${detail}\n`).join("") + @@ -391,16 +339,15 @@ function formatGuard(changedBlobs) { ` To push anyway: SKIP_FORMAT_GUARD=1 git push`, }; } - if (unformatted.length === 0) return { name: "format", ok: true, note }; + if (unformatted.length === 0) return { name: "format", ok: true }; return { name: "format", ok: false, - note, message: `Prettier found unformatted files in this push (CI format:check would fail):\n` + unformatted.map((detail) => `${detail}\n`).join("") + - ` These are the committed contents checked against the pushed prettier config,\n` + - ` not your working copy — run \`npm run format\` and commit the result.\n` + + ` This is a checkout of the pushed commit, not your working copy — run\n` + + ` \`npm run format\` and commit the result.\n` + ` To push anyway: SKIP_FORMAT_GUARD=1 git push`, }; } From 0ce9f5e632914b080e1fe9f8dd7bf69ca7691dac Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 04:56:18 +0000 Subject: [PATCH 08/11] fix(test,guard): resolve band reachability by reach, not presence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six false greens were reported on the adoption gate in one day — an unrendered import, `export { X }`, `export { X } from`, `export *`, a bare side-effect import, JSX inside an unmounted helper, and a lazy import reaching every sibling export of the same module. They are one defect: the walk asked whether a file mentions the band, when the question is whether anything the route mounts reaches it. Two of the six were introduced by an earlier patch to this same walker, so this replaces the heuristic rather than adding a seventh case. Each module is now parsed into a small graph (exported name -> local, local -> identifiers its body references, locals that render the band) and the walk carries, at every hop, the set of exports the importer actually mounts. The special cases fall out of module semantics instead of being enumerated: `dynamic(() => import("…").then((m) => m.Named))` follows only that binding; a bare `import "…"` renders nothing so is not followed; `export { X } from` is followed only when the importer wants X; `export *` never supplies a default, so a page needing only a default gets no hop from it. Verified: all five production search routes still reach the band; gutting services/page.tsx and tools/page.tsx to
each reports an orphan; the two fixtures guarding the new mechanisms were confirmed to fail under targeted mutation (presence-based band check, and following bare imports) after an initial pair that did not bite was replaced. Also from review: - guard-push escalates to a whole-tree check for a package.json only when it carries a prettier field, so a routine dependency bump does not. Verified: bump passes, prettier-field narrowing blocks, .prettierrc narrowing blocks. - AGENTS.md no longer claims the guard checks working-tree paths; that was the defect this PR fixed, and leaving it would teach distrust of a fixed guard. - Repairs the #115 ledger row, whose Source cell had been overwritten by the detail text, and rewrites it for the new design. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- AGENTS.md | 4 +- docs/outstanding-issues.md | 118 ++--- scripts/guard-push.mjs | 21 +- tests/search-results-band-adoption.test.ts | 480 ++++++++++++++++----- 4 files changed, 451 insertions(+), 172 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d3f195289f..ff1e72e038 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,10 +167,10 @@ When a branch or PR review completes, record it with `npm run ledger:append -- - - Let the repository run coordinator control cross-worktree verification. It permits at most two focused Vitest/read-only typecheck leases from different worktrees; full Vitest, coverage, lint, build, Playwright, and live-provider tests remain exclusive. Do not install while a repository test, build, lint, typecheck, or server command is active. Avoid aggressive short-interval polling, and do not repeat an unchanged full gate after it passes. - For UI, frontend, browser, routing, styling, reduced-motion, or forced-colors changes, run `npm run ensure` before browser work and use `npm run verify:ui` as the Chromium UI gate. For phone-chrome changes, run `npm run verify:phone-chrome` first: it checks installed-lock parity, selects the affected browser/PWA owners and exact journeys, and adds `verify:ui` last only when shared chrome foundations make the broad gate necessary. Inspect uncertain scope with `-- --dry-run`. - **Run `npm run format` and commit the result before every push.** `format:check` is in neither `npm run test`, `npm run typecheck`, nor `npm run lint`, so the ordinary loop reports green while `Static PR checks` and `ci/circleci: verify` both fail on `prettier --check .`. Three CI failures on 2026-07-30 came from exactly this. Two traps beyond simply running it: - - **Formatting without committing does nothing for the push.** A push sends commits, not your working tree, so formatting after committing leaves the unformatted blob on the branch. Worse, `guard-push.mjs` runs `prettier --check` against working-tree **paths**, so it reports clean while the pushed commit is still unformatted — the guard passing is not evidence. Amend or add a follow-up commit. + - **Formatting without committing does nothing for the push.** A push sends commits, not your working tree, so formatting after committing leaves the unformatted blob on the branch. Amend or add a follow-up commit. - **A per-file check is not the repository-wide check.** `prettier --check ` on the source file you edited passes while a doc or ledger edit in the same push fails; that was the missed file twice out of three. - `.githooks/pre-push` carries the guard, but `core.hooksPath` is set by this checkout's `npm install`, so an agent pushing from its own environment bypasses it entirely and only CI catches the break. + `.githooks/pre-push` carries the guard, and since 2026-07-30 it checks the pushed commit where CI checks it: `guard-push.mjs` puts the pushed SHA in a temporary `git worktree` with `node_modules` linked in and runs Prettier there, so neither the working tree's contents nor its prettier config can vouch for the commit, and a dynamic `prettier.config.*` still loads. A push that changes prettier policy (`.prettierrc*`, `.prettierignore`, `.editorconfig`, or a `package.json` carrying a `prettier` field) escalates to a whole-tree `prettier --check .`, because a policy change alters the verdict for files the push never touched. But `core.hooksPath` is set by this checkout's `npm install`, so an agent pushing from its own environment bypasses the hook entirely and only CI catches the break — which is why the rule above is still a rule. - For release or handoff confidence, use `npm run verify:release`; this includes the full Playwright project set. - For clinical ingestion, answer generation, source governance, privacy, production-readiness, or environment changes, run the smallest relevant domain check plus `npm run check:production-readiness`. diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 9d28a9dfd2..d4b4aeb997 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -95,65 +95,65 @@ removed after current-main verification; it is not missing recommended work. > > **Exact-head release confirmation (2026-07-28):** after final review narrowed expanded chunk selection to the measured clozapine blood-count action shape, `output/rag-retrieval-post-exact-head.json` repeated all 36 cases with document/content recall 1.0, MRR 0.8921, content MRR 0.9406, nDCG 0.9308, irrelevant-at-10 0.0917, zero failures and zero per-case document/content reciprocal-rank regressions versus `rag-retrieval-post-final.json`. Median latency rose 13,563 -> 19,729 ms while p90 improved 56,765 -> 55,660 ms; neither run had a latency-failed case, so no ranking or latency gate changed. Cache-bypassed exact-head answer probes for both admission/discharge cases and the clozapine threshold case were substantive, grounded, expected-source-backed and free of citation/numeric/route failures; all used zero provider requests and $0 generation cost. This was the protected behavior-change merge gate, not a rerun for #023. -| ID | Pri | Type | Summary | Detail / next action | Source | Added | -| ---- | --- | ----- | -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | -| #059 | P1 | task | Verify containment of every credential reported exposed in chat | **Outcome:** every reported exposed credential is rejected or retired. **Next:** in approved security windows, verify and revoke or rotate the GitHub token, OpenAI key, Supabase service-role JWT, database password, and E2E credential; create replacements only when required and update only intended secret stores. **Success:** provider evidence confirms the old credentials cannot authenticate, replacements are distinct and minimally scoped, presence/readiness checks pass, and secret scans remain clean. **Stop:** no provider or secret-store action without approval; never print or paste values into Git, logs, issues, or chat. | session 2026-07-24 security reconciliation; AI Agent Target Manifest | 2026-07-24 | -| #065 | P2 | task | Complete the paused compact document source-text accordion | **Outcome:** the document viewer uses compact nested disclosures while retaining complete text, citation/search navigation, print behavior, and composer clearance. **Next:** only when the user explicitly resumes, reconcile `codex/chat-document-text-accordion-7cb4` with current `main` and complete the focused 320/390/1280 px tests. **Success:** default disclosures are closed; deep links and search open only the active passage; printing expands/restores state; no overflow. **Verify:** focused document-viewer Playwright, `verify:cheap`, `verify:ui`, and static production-readiness. **Stop:** remain paused until explicit user return; no provider calls. | paused document-viewer task; `codex/chat-document-text-accordion-7cb4` | 2026-07-24 | -| #001 | P2 | task | Semantic reranking still gated off | `RAG_SEMANTIC_RERANK_ENABLED=false` from PR #901. Do not enable until the provider-backed 36/36 retrieval-quality gate **and** an ambiguity-focused canary are explicitly approved and recorded. | `docs/process-hardening.md` (Semantic reranking rollout debt); PR #901 | 2026-07-21 | -| #053 | P1 | task | Execute cross-border privacy/legal package | Execute OpenAI and Railway DPAs; decide ZDR and Australian data residency; obtain prompt-cache behavior in writing; review subprocessors; obtain APP 8 and APP 5/1 counsel sign-off. Do not represent the release as privacy-approved or alter final public privacy wording before sign-off. | `docs/openai-cross-border-basis.md`; `docs/privacy-impact-assessment.md` | 2026-07-24 | -| #055 | P2 | task | Run one exact-SHA full release and PR gate | Before the next full-confidence release/handoff, record the candidate/PR SHA and run the local/provider release gates, Firefox/WebKit, required hosted CI, and actionable GitHub review-thread closure once. Stop at the first actionable failure and rerun only the repaired smallest gate. | `docs/launch-operator-runbook.md`; `docs/codex-review-protocol.md` | 2026-07-24 | -| #056 | P2 | task | Reconcile the existing staging migration history | `Clinical KB Staging` already exists as a healthy, empty Supabase/Railway tier with distinct secrets and no production clinical data, but it is 23 repository migrations behind. In the next approved staging schema window, apply the exact missing migration chain, then re-run indexing, health, identity and data-boundary proof. Do not recreate the environment or copy production clinical documents. | current-main staging verification; `docs/staging-setup.md`; `docs/operator-backlog.md` | 2026-07-27 | -| #057 | P2 | task | Complete staging soak and rollback rehearsal | After #056, run the documented soak and rollback against an exact candidate; retain latency/error/rollback evidence. Stop on unsafe data, identity mismatch, or an unowned rollback decision. | `docs/launch-operator-runbook.md`; `docs/capacity-review.md` | 2026-07-24 | -| #005 | P3 | rec | `finalScore` saturates at clamp ceiling | Base + ~40 stacked boosts routinely exceed 1.0, so strong matches tie at 1.0 and order by an arbitrary `document_id` tiebreak. If ranking is ever revisited, break ties by the **pre-clamp** score rather than raising the `[0,1]` ceiling (downstream gates assume `[0,1]`). Ordering already sorts by the unbounded pre-clamp `rankScore` (`clinical-search.ts:1735,1927,1950-1955`), so the clamp confines only the reported confidence value, not result order. Not a defect on the current golden set; any change here is a protected RAG surface (canary required). | `docs/rag-hybrid-findings-and-todo.md` P1 item 4; `src/lib/clinical-search.ts:1735` | 2026-07-21 | -| #011 | P3 | task | Auth DB-connection allocation is operator-only | Supabase Auth (GoTrue) is capped at ~10 absolute DB connections (Supabase perf advisor). Switch to **percentage-based** allocation in the Supabase **dashboard** before the first compute scale-up — **not settable via SQL/MCP** (operator-owned). Verify via a staging soak + an approval-gated read-only advisor re-check. | `docs/auth-connection-cap-runbook.md`; `docs/process-hardening.md` (Known follow-up debts) | 2026-07-21 | -| #013 | P3 | rec | Route-chunk + mockup catalogue JSON weight | `build:analyze`: `/specifiers` ships `specifiers-search-index.json` (~180 KB parsed), `/forms` ships `forms-catalog.json` (~132 KB), `/formulation` ships `formulation-content.json` (~52 KB, client-side local search — needs index/full split or a search endpoint, architectural). All route-scoped (not initial bundle). Also `*-mockups.tsx` (~100 KB across chunks) build though `/mockups` 404s in prod — exclude from the prod artifact. | session 2026-07-21 (build:analyze) | 2026-07-21 | -| #016 | P3 | rec | "Big but not easy" structural + motion perf | Deferred larger levers: (a) nonce-CSP forces every product route to `╞Æ Dynamic` (zero static generation) — evaluate Partial Prerendering / static shells for the static clinical catalogues (DSM/differentials/therapy/specifiers/formulation); (b) sidebar expand/collapse animates `grid-template-columns` (biggest smoothness cost, motion-gated — needs a transform-overlay rethink); (c) Therapy Compass fetches 692 KB / 2.5 MB JSON client-side (defer until interaction + confirm brotli); (d) settings/setup/admin dialogs static-imported into the home chunk (`next/dynamic` them); (e) `src/app/(search-app)/layout.tsx:4` imports 71.6 KB of Therapy-Compass-only CSS in the ROUTE-GROUP layout, making it render-blocking on `/`, `/documents`, `/forms`, `/dsm` and every mode home; (f) `shared-search-app-shell.tsx:8` statically imports the `therapy-compass` barrel, pulling `workspace.tsx` + `bindings.tsx` + `nav.tsx` into every `(search-app)` route; (g) three client waterfalls (`use-app-preferences.ts:156-182`, `ClinicalDashboard.tsx:977-1069`, `signed-image.tsx:60-84` + `use-signed-image-url.ts:39`) and the paint offenders in `globals.css` beyond the sidebar grid — three stacked `backdrop-filter` passes on an always-mounted translating element (`:709-748`), `box-shadow` inside a `transition` list (`:677-684`), and `@keyframes shimmer` animating `background-position` on the shared `Skeleton` (`:2289-2296`). **CORRECTED 2026-07-29 on (c):** the Therapy Compass filenames are unversioned and Next serves `/public` with an ETag, so only the FIRST visit pays 690.6 KB / 2,470 KB — repeat visits pay ~4 revalidation round trips. The fix is content-hashed filenames + `immutable` (touching `scripts/build-therapies-index.mjs` and `check:therapy-data-index`), NOT a bare `Cache-Control` line. See `docs/audit/latency-audit-2026-07-28.md` L3-1/L3-2/L3-3/L3-6/L3-7. | session 2026-07-21 (build route table + design audit) | 2026-07-21 | -| #017 | P3 | task | Field Web-Vitals baseline via live Lighthouse | In-sandbox runtime vitals were blocked (prod server hard-requires Supabase secrets; dev-mode CLS measured excellent at 0.00–0.04, content-first pages 0.000). Run Lighthouse against `psychiatry.tools` for real LCP/INP/CLS to prioritize #012–#016 by measured impact rather than reasoning. **Instrument landed (PR #1385):** `live-web-vitals.yml` is dispatch-only, takes no secrets and runs nothing until someone dispatches it — which is itself a live production action needing explicit approval. `scripts/summarise-web-vitals.mjs` holds the decision rule, committed before any numbers were read: mobile LCP < 2500 ms AND CLS < 0.1 on every route (plus INP < 200 ms from CrUX) closes this row and makes only the explicitly measured **payload** findings gated by #017 WONTFIX — e.g. the route-chunk/catalogue weight in #013. It does **not** close #016 wholesale: #016's motion, CSS, client-waterfall, caching, and dynamic-import items stay open unless they have separate evidence, because Lighthouse LCP/CLS do not validate those. Any breach makes only that route's findings actionable. It fails closed — a missing run, a null metric, a route-slug collision, and a measurement that landed on a different URL than requested (redirect, dropped or reordered query) are all breaches. **Read the first dispatch as a measurement to be sanity-checked, not as an oracle:** the grading logic took eight rounds of review corrections to reach this shape, each fix locally right and globally incomplete, so cross-check the emitted table against the raw Lighthouse JSON artifact before recording a verdict — most of all a PASS, which would close #017 and the explicitly measured payload findings. Lighthouse cannot measure INP in lab conditions, so that clause of the rule is confirmed from CrUX field data and any pass is provisional on it. | session 2026-07-21 (measurement pass) | 2026-07-21 | -| #018 | P2 | task | Split the lithium, ADHD and metabolic residuals by mechanism | Current evidence keeps the mechanisms separate. **Lithium — closed within this item:** the row/atom-aware subject guard, foreign-parameter rejection and query-specific range promotion returned `0.5–1.0 mmol/L` with correct targeting/citation; the full retrieval canary remained 36/36 with recall 1.0 and zero per-case RR regressions, and the full answer canary passed every blocking gate. **ADHD — open corpus debt:** `CG.MHSP.ADHD.pdf` is absent from the hosted corpus and the retrieved chart exposes `accessible_table_count=0`; repair corpus/fixture or ingestion evidence rather than weakening extractive budgets. **Metabolic — open structured-evidence debt:** the standalone plural classifier worsened the live answer and was reverted; obtain auditable schedule text/table evidence before another candidate. | targeted live lithium/ADHD/metabolic evidence 2026-07-27; `docs/evidence/rag-reliability-evidence-2026-07-27.md`; refuted approaches | 2026-07-21 | -| #021 | P3 | rec | E-3d H2 residual: strong/comparison generation discards | approx. 6 generation attempts per full 44-case run still fail the final quality gate and fall to extractive on strong-route comparison/complex shapes (the designed-conservative outcome). PARKED: weakest cost/benefit on the queue — a wave (approx. $2-4 pair + reviewer cycle) to shave seconds off a few hard cases. Revisit only if latency/waste complaints or a cheaper lever appears. | E-3c design record; runs #59-#61 diagnostics | 2026-07-21 | -| #022 | P2 | task | Source-governance metadata refresh (operator) | The selected policy is now encoded locally as auditable `third_party_reference_attested` evidence with policy version, reviewer qualification, evidence references and append-only review history. It deliberately preserves `clinical_validation_status=unverified`; malformed, stale or non-BMJ evidence remains review debt. Migration `20260727010000_bmj_third_party_source_attestation.sql` is prepared but was **not applied**. The ten most visible local-document candidates are captured in `docs/evidence/rag-top-local-review-manifest-2026-07-26.json` with `attestation_applied=false`; qualified human review, deliberate hosted apply/attestation, and warning-rate remeasurement remain operator work. | governance worklist; local policy/migration tests; top-ten evidence manifest | 2026-07-21 | -| #023 | P2 | task | Complete scheduled browser and labeling disposition | The 2026-07-26 retrieval and answer artifacts are read and compared under resolved #051. Scheduled CI run `30216361999` failed its existing production dependency audit before Firefox/WebKit, while production Chromium passed. After that audit is green, capture one scheduled/manual browser-matrix datapoint; separately record the human decision for the stable irrelevant-at-10 set. #084 now makes each top-10 grade and matched signal reproducible, but it does not substitute for the human disposition. Do not rerun or spend on RAG for this item. | runs `30216191889`/`30216361999`; per-rank diagnostics #084; session 2026-07-27 | 2026-07-21 | -| #024 | P2 | issue | WebKit e2e `_rsc` prefetch access-control errors | PR #1205 narrowed catch-all interception and duplicate navigation, but Next 16.2.11 still raises `_rsc` access-control `pageerror`s after document-source fallbacks: `/documents/source?id=&page=2&chunk=safety%20plan` → `/documents/?page=2&chunk=safety+plan`; `/documents/source/evidence?id=not-a-uuid&page=2` → `/documents/search`. The invalid-id failure survived removing every Playwright route; Chromium passed both. **Next:** on a provider-free macOS host, run both URLs in stable Safari and Safari Technology Preview without interception, capture console text plus `_rsc` status/access-control headers, and compare Playwright WebKit with routing on/off. Treat as an app defect only if native Safari reproduces; otherwise return to the harness. Never suppress `pageerror` or change CORS without native evidence. | PRs #1179/#1205; current-main local WebKit evidence; session 2026-07-28 | 2026-07-28 | -| #025 | P2 | task | Activate the three webhooks (operator secrets) | Merged (#968/#1100) + deployed but inert — verified live: `POST /api/webhooks/railway` returns `503 webhook_not_configured`; the Supabase document-change trigger exists but lacks both activation inputs. To turn on: (1) Railway ΓåÆ set `RAILWAY_WEBHOOK_SECRET` + add the `?token=…` webhook URL; (2) set `SLACK_WEBHOOK_URL`/`DISCORD_WEBHOOK_URL` in BOTH the Railway **app/server env** and **GitHub repo secrets**; (3) set one matching document-change secret in the Railway app env as `SUPABASE_INGESTION_WEBHOOK_SECRET` and in Supabase Vault as `ingestion_webhook_secret`, then set the per-environment database GUC `app.ingestion_webhook_base_url` to the deployed app origin. Each path fails closed until fully configured, so this is pure ops. See `docs/webhooks.md` for verification and rotation. | sessions 2026-07-22/24; PRs #968/#1100; docs/webhooks.md | 2026-07-22 | -| #027 | P3 | rec | External uptime monitor independent of GitHub/Railway | `live-domain-monitor.yml` runs on GitHub's cron, so it won't run in exactly the outage it should catch (Actions or the deploy itself down). Add an off-platform synthetic monitor (UptimeRobot / Better Stack / Checkly) hitting `/api/health` with a webhook alert. Provider setup, not code. | session 2026-07-22 webhook review | 2026-07-22 | -| #028 | P3 | rec | Runtime error tracking (Sentry or similar) | No error tracking in the repo — production exceptions on `psychiatry.tools`, including how often `RAG_PROVIDER_MODE=auto` silently degrades to source-only, are invisible. Weigh adding `@sentry/nextjs` (dependency + DSN secret + instrumentation) vs cost; alert ΓåÆ chat/issue. Provider-backed; needs explicit sign-off before adding the dependency. | session 2026-07-22 webhook review | 2026-07-22 | -| #033 | P3 | rec | Source governance metadata absent from the LLM prompt | `buildRagSourceBlock` omits `document_status`, `clinical_validation_status`, and `extraction_quality`, so the model cannot self-caveat during generation and governance is enforced only post-hoc. Generation-surface change: needs `eval:rag` plus `eval:quality --rag-only` (grounded-supported must not drop, citation-failure 0) and explicit approval. Carries the same "unknown Γëá bad" hazard as #032 — on a partially-enriched corpus the model would likely over-caveat correct sources, so design the prompt wording before spending an eval. | `src/lib/rag/rag-source-block.ts:126-198`; PR #1051 audit item 8 | 2026-07-22 | -| #035 | P3 | rec | Threshold-conflict detection covers only 3 params | `detectThresholdDisagreements` checks only ANC, WBC, and platelets paired with withholding verbs, so cross-source conflicts on medication doses, lithium/thyroid levels, or vital signs go undetected. Deliberately narrow (see the comment at `:469-474`). Broadening changes when an answer is classified `conflicting` and adds warnings — real false-positive risk. Needs new fixtures plus a behaviour review before any change. | `src/lib/evidence.ts:469-574`; PR #1051 audit item 7 | 2026-07-22 | -| #036 | P3 | rec | No explicit `is_public` visibility flag on documents | Public-corpus visibility is implicit: `owner_id IS NULL` on an `indexed` document (`resolveSearchScope`). The `metadata.public_corpus` marker is written by the promotion migrations but never used as a retrieval filter. Promotion is unconditional on `clinical_validation_status`, so unverified documents are publicly searchable — compensated by keeping `unverified_source` in the frontend-visible warning set. A hard schema flag touches RLS and the clinical-risk-gated retrieval RPCs; weigh against the existing compensating control before acting. | `supabase/schema.sql:61-108`; `src/lib/search-scope.ts:181-236`; PR #1051 audit item 3 | 2026-07-22 | -| #037 | P3 | rec | D5 trust-cap-all-claims flag parked OFF | `NEXT_PUBLIC_RAG_TRUST_CAP_ALL_CLAIMS` extends authority gating from high-risk claims to **all** supported claims (`deriveTrust`). Ships OFF by design; flipping it caps trust to `medium` for routine claims across the board — a product/clinical-UX decision, not a defect. Both states are test-pinned. Next action: product decision, then flip and re-baseline the UI expectations. | `src/lib/answer-render-policy.ts:159-177`; PR #1051 audit item 11 | 2026-07-22 | -| #038 | P3 | rec | Consolidate shared comparison behavior | Several clinical modes expose comparison workflows with similar selection, empty-state and mobile-dock needs. Define one shared behavioral contract before another comparison surface is added; keep mode-specific clinical content separate. This is a design-system recommendation, not a current defect. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | -| #039 | P3 | rec | Consolidate catalogue toolbar patterns | Catalogue/search pages have independently evolved filter, sort, result-count and mobile toolbar behavior. Inventory the existing implementations and converge only the repeated interaction contract; do not flatten mode-specific search semantics. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | -| #040 | P3 | rec | Add targeted visual-regression baselines | Keep a small approved baseline set for high-value desktop/mobile surfaces and accessibility modes instead of screenshotting every route. Start with account/settings, document viewer, mode homes and bottom-composer interactions; define an intentional-update workflow before enabling blocking comparisons. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | -| #079 | P3 | task | Disposition retained worktrees in bounded cleanup batches | **Outcome:** the retained reconciliation tail is gradually classified without another disruptive all-worktree sweep. **Next:** process no more than ten worktrees per explicitly scheduled pass using current owner/process metadata, open-PR state, exact review-ledger coverage, ancestry, and cherry-pick-aware content proof. **Success:** remove only clean, inactive, bundled worktrees whose content is merged or explicitly rejected; record every disposition and retain recovery evidence. **Stop:** preserve dirty, active, secret-bearing, post-freeze, paused, or ambiguous work and never use reset, force deletion, broad clean, or process killing. | final reconciliation inventory retained 104 independent worktrees; session 2026-07-24 | 2026-07-24 | -| #085 | P3 | rec | Upload-limit client/server sync is unguarded | `NEXT_PUBLIC_MAX_UPLOAD_MB` (client, build-time inlined) and `MAX_UPLOAD_MB` (server, runtime) default 150/150 but nothing keeps them in sync — no zod link, gate, or test. Lower server-only → the 413-after-full-transfer UX FV-04 (#1064/#1069) was built to prevent; lower client-only → false pre-check rejection of files the server would accept (breaks #1064's client-is-a-strict-superset invariant). Both documented in `.env.example`/`docs/deployment-architecture.md` but unenforced; the client value is also frozen at `next build`, so changing the Railway service var without an image rebuild silently no-ops. Cheapest guard: a `check:*` script (or CI assertion) that fails when the two configured values disagree. | session 2026-07-28 (FV-04 adversarial workflow, PR #1069); ID #085 after #084 claimed on main | 2026-07-27 | -| #086 | P3 | task | Repository maturity backlog — remaining structural work | **Outcome:** the deferred repository-maturity backlog ships as verified draft PRs, one per structural change. **Canonical runbook:** [`docs/maturity-backlog-workorders.md`](maturity-backlog-workorders.md). **Remaining:** X3 `rag.ts` decomposition; X7 finish the `src/lib` domain reorg; X6 clinical/retrieval/answer coverage floors; X5 ACL-migration consolidation (provider-gated); L1 archive the retired `backfill:*` one-shots + the dead `ci-change-scope` token; L4 ledger rotation; M1 repo-host hardening (maintainer, audit §8). **Next:** X3 on user go-ahead. **Stop:** RAG/retrieval items need the flag + go-ahead; X5 is live-DB provider-gated. | `docs/maturity-backlog-workorders.md`; audit §8/§10; session 2026-07-28 | 2026-07-28 | -| #088 | P3 | task | Watch for union-driver duplication as open PRs merge the repaired ledger | **Outcome:** the 2026-07-28 ledger repair does not resurface as duplicated rows. **Next:** on the first few open PRs that merge `origin/main` after the repair, run `npm run check:branch-review-ledger` on the merged head. Ordinary 3-way merges take main's repaired lines cleanly; only a same-hunk conflict would let `merge=union` keep both the corrupted and repaired copies. **Success:** three consecutive post-repair merges pass the guard. **Stop:** if duplication appears, remove exact duplicates only (the ledger contract allows that) and never rewrite surrounding records. | branch-review-ledger hygiene pass rewrote 226 historical lines; session 2026-07-28 | 2026-07-28 | -| #090 | P2 | task | Upgrade the eslint ecosystem to clear remaining dev-scoped high advisories | **Outcome:** full `npm audit` reports zero high advisories from the eslint toolchain. **Next:** in a dedicated dependency pass, upgrade eslint and its plugin/config set together (npm offers `eslint@10.8.0`, `isSemVerMajor`); residual highs (`@eslint/config-array`, `@eslint/eslintrc`, `eslint`, `eslint-config-next`, `eslint-plugin-import`, `eslint-plugin-jsx-a11y`, plus the advisory's numeric `<=5.0.7` hit on the unused `brace-expansion@1.1.16` / `2.1.2` maintenance lines that still ship an unpatched `main`) cascade from this toolchain. **Success:** `npm run lint` clean, `verify:cheap` green, full-audit highs cleared, no rule-config regressions. **Stop:** an eslint major previously broke `eslint-plugin-react` here — do not bundle into an unrelated PR, and do not force with `npm audit fix --force`. Production `npm audit --omit=dev` is already clean after the exceljs `archiver@8` / `unzipper@0.12.5` overrides on PR #1314; this item is eslint-dev cleanup only. | session 2026-07-28 brace-expansion triage (PR #1314) | 2026-07-28 | -| #091 | P2 | issue | Results band cannot express a partial-source failure | **Outcome:** a favourites/results count is never asserted while some of its sources failed to load. **Detail:** `SearchResultsBandStatus` is a single flat status, so a page fed by several registries has no way to say "3 loaded, 1 failed". `saved-registry-favourites-status.ts:29` works around this with `itemCount > 0 && folded !== "ready" ? "ready" : folded`, and `favourites-command-library-page.tsx` applies the same mask a second time — so a partial failure renders a confident count with no fault indication, which is the exact defect class PR #1316 removed everywhere else. Neither favourites page consumes the true `registryStatus` the fold already returns. **Next:** decide between (a) a `partial` arm on the status union carrying a "some sources unavailable" note beside an honest count, or (b) surfacing `registryStatus` as a separate inline notice and dropping the mask. **Stop:** do not widen the mask to more surfaces before the shape is decided. | PR #1316 review thread PRRT_kwDOSh5Fis6UUf-k; session 2026-07-28 | 2026-07-28 | -| #092 | P3 | task | Refetch pulse deferred on auth-backed registries (privacy invariant) | **Outcome:** a background refresh keeps the prior count visible instead of a skeleton, without weakening identity clearing. **Detail:** the `refetching` status is built in the band and adopted only on `formulation-home-page.tsx`, where the lag is `useDeferredValue` over static data. It is deliberately NOT adopted on `use-registry-records.ts:85`, `use-medication-catalog.ts:76` or `use-differential-catalog.ts:133`, which all clear data on entering loading. `use-differential-catalog.ts:122` states why: "Auth must clear prior identity's matches immediately", and `:164` that "a later retype of any prior query cannot resurrect authorized matches." **Next:** if adopted, guard preservation on identity AND query equality, and pin with a test that an identity change still clears immediately. **Stop:** never hold records across an auth transition. | PR #1316 plan phase 6; session 2026-07-28 | 2026-07-28 | -| #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 | -| #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 | -| #100 | P2 | rec | Buffered answer generation has no incremental verified delivery | **Outcome:** a clinician sees verified answer content before the whole generation completes. Highest-leverage latency finding in the 2026-07-28 audit: generation is buffered (`openai.ts:465`) and delivered in ONE `final` SSE frame, so time-to-first-content equals total latency — a strong answer inside its 25 s SLO still shows a blank panel for 25 s. The 15 s `sse-heartbeat` exists because that silence routinely exceeds 15 s; it instruments the defect rather than fixing it. **Naive token streaming is REFUTED, not merely unbuilt:** `answer-stream-contract.ts:18-21` removed `token`/`revising` deliberately because a rolling deployment would "re-expose unvalidated clinical prose", and raw tokens bypass the numeric-faithfulness gate the 2026-07-01 audit filed as H1. **Only admissible shape:** progressive disclosure of already-verified units (evidence/sources at retrieval-complete, then per-section after that section clears verification) over the existing whitelisted `progress` event. Needs a clinical-governance decision plus a canary pair. Also add the refutation to `docs/rag-behaviour/refuted-approaches.md`. Cross-references #021. **Stop:** do not re-land `token` streaming. | `docs/audit/latency-audit-2026-07-28.md` L0-1; `src/lib/answer-stream-contract.ts:18-21`; `src/lib/sse-heartbeat.ts` | 2026-07-29 | -| #101 | P3 | rec | Canary-gated retrieval parallelisation candidates | **Outcome:** independent retrieval stages stop running serially, proven by a live canary pair. Candidates: metadata/memory/visual hydration triples repeated on four branches (`rag.ts:2460,2493,2521` and three more) while `rag.ts:2751-2804` already parallelises three RPCs in one `Promise.all`, so the omission is inconsistency rather than intent; the nested `await`-in-loop scope enumeration (`search-scope.ts:202,328`); typeahead results never cached (`rag.ts:2698-2711`); universal-search coalescing (`/api/search` has it, `/api/search/universal` does not). Each changes candidate assembly, truncation, or what the next keystroke returns, so each needs 36/36 retrieval plus recall 1.0 and zero per-case rr regressions. Distinct from #001 (semantic rerank). Resolved #075 and #083 are the precedents for why these are gated rather than free. **Stop:** needs the #098 harness and explicit canary approval first. | `docs/audit/latency-audit-2026-07-28.md` L2-1/L2-2/L2-8/L1-5 | 2026-07-29 | -| #102 | P3 | task | Apply the additive `documents` index debt (operator) | **Outcome:** bare-column `ILIKE` and the paged status scan on `documents` are index-served on hosted. `documents_title_trgm_idx` indexes a CONCATENATED expression, so the bare-column predicates in `api/documents/route.ts:193` and `rag-candidate-sources.ts:477` (RAG path) cannot use it and fall back to scanning; `search-scope.ts:271-277` sorts per page against the single-column `documents_status_idx`. **Runbook prepared 2026-07-29 — NOT applied, item stays open:** three `CREATE INDEX CONCURRENTLY` statements authored and reviewed in `docs/operator-apply-performance-latency-remediation.md` — additive, though **the "recall is byte-identical" claim was RETRACTED on 2026-07-29 review**: `fetchDocumentTitleAliasRows` (`rag-candidate-sources.ts:482`) applies `.limit(12)` with no `ORDER BY`, so a new index can change which title-alias documents feed candidate assembly. The documents-list and `(status,id)` uses stay ordering-safe; the RAG-path index is canary-gated, and making that `.limit(12)` deterministic first does **not** lift the gate — an unordered `LIMIT` has no stable selection to preserve, so imposing an order can pick a different twelve and is itself an ordering behaviour change on a retrieval surface, which AGENTS.md requires a canary pair for. Sequencing the ordering fix first is worthwhile (unordered `LIMIT` on a retrieval input is latent nondeterminism regardless) but yields two canary-gated changes, not one (PR #1377 review). **Deliberately NO migration file:** an additive-index migration without a synchronized `schema.sql` mirror and regenerated drift manifest is exactly what closed PR #1312, and the mirror cannot come first because `required_indexes` in `search_schema_health()` (`schema.sql:3178`) runs against live. **Next (operator):** **author the migration first** — `supabase/migrations/` is the source of truth and `schema.sql` only a mirror, so hand-run operator SQL never reaches staging, disaster-recovery replay, or a local `supabase db reset`, and a `required_indexes` registration would fail there (PR #1377 review); follow the `20260717170000_registry_projection_cleanup.sql` idempotent pattern. **That migration must also carry the health-function change** — `required_indexes` lives inside `search_schema_health()`, which is redefined by `create or replace function` in eleven migrations (copy `20260705180000_reconcile_search_health_indexes.sql:62`); editing `schema.sql:3177` alone moves only the mirror and leaves the indexes unmonitored on hosted (PR #1377 review). Then apply concurrently, confirm `indisvalid`, mirror both the index statements and the identical function body into `schema.sql`, run `npm run drift:manifest` (Docker), and deploy the migration LAST — in that order, in one change. Expect `check:drift` to report them as unexpected between steps 1 and 2. **Rollback is three deployed phases, not the reverse of one:** retract `required_indexes` via its own `create or replace function` migration and deploy → drop concurrently live → only then deploy the `schema.sql` removal plus an idempotent forward `drop index if exists` migration, because Supabase wraps migrations in a transaction and a plain `DROP INDEX` there takes the lock the concurrent procedure exists to avoid (PR #1377 review). | `docs/audit/latency-audit-2026-07-28.md` L2-3/L2-5; `docs/operator-apply-performance-latency-remediation.md` | 2026-07-29 | -| #103 | P2 | issue | Wide table-facts trigram index missing from `schema.sql` | **Outcome:** the migration chain and `schema.sql` agree on `document_table_facts` trigram indexes. `supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` creates a wide 5-column trigram index that is **absent from `supabase/schema.sql`**, so local replay and the live database can diverge. Distinct from #102: different owner and verification path. **Next:** confirm whether the wide index exists live, then take one of exactly two routes — **retained:** mirror `document_table_facts_text_trgm_idx` into `supabase/schema.sql` beside the narrow one and regenerate `drift-manifest.json`; **redundant:** drop it through a new forward migration, never by deleting `20260714190000`. **`drift-allowlist.json` is NOT a third option** (PR #1377 review): its own header scopes it to _"Known live-vs-`schema.sql` divergence"_, so it can silence a live drift finding but cannot reconcile the migration chain with the mirror — a fresh `supabase db reset` still runs `20260714190000` and creates the index while `schema.sql` still omits it, leaving this row's stated outcome unmet. **No offline gate catches this today:** the migration↔`schema.sql` parity test (`tests/drift-detection.test.ts:59-68`) only asserts one migration's `schema_drift_snapshot` function definition, not an index inventory — which is why this sits open rather than red in CI, and why a replay-to-schema inventory comparison is the check that would have caught it. Note the narrow `document_table_facts_title_row_param_trgm_idx` (`schema.sql:6425`) is the one the effective RPC expression (`:6726`) actually matches, so the wide index may be genuinely redundant — do not drop it without live scan evidence, per the monitored-not-auto-fixed index policy. | `docs/audit/latency-audit-2026-07-28.md` limitations; `npm run check:drift` | 2026-07-29 | -| #104 | P3 | rec | CORRECTION — the worker's triple image read is deliberate, not debt | **Outcome:** a future audit does not re-file this a third time. The 2026-07-28 latency audit listed L4-2 (`worker/main.ts` `readFile`s each extracted image up to 3x per document — hash, caption on cache miss, upload) as "CONFIRMED with no fix evidence", carried forward from the 2026-07-01 audit's `L11`. **That was wrong.** The 2026-07-01 disposition table already recorded it as a deliberate peak-memory trade-off, and the rationale is documented in place at `worker/main.ts:866-869`: holding every extracted image Buffer for a document with hundreds of multi-MB page images would multiply the worker's peak memory, and disk I/O is the cheaper resource for a background pipeline. The three reads (`:872`, `:1034`, `:1129`) are real but accepted. **Next:** none — revisit only if ingestion throughput becomes a measured complaint AND a bounded-buffer design is proposed. **Stop:** do not "fix" this by caching buffers; that trades a decided memory ceiling for disk I/O nobody has measured as a problem. | `docs/audit/repo-audit-2026-07-01.md` L11 + disposition table; `docs/audit/latency-audit-2026-07-28.md` L4-2 retraction | 2026-07-29 | -| #105 | P3 | task | Verify the `#017`-exempt client latency wins in a browser | **Outcome:** the two zero-payload client fixes are confirmed in a real browser. `#017` gates _payload_ decisions (#012/#013/#016 are all byte-count items); a `loading` fallback ships zero bytes and a resource hint ships ~60, so neither can be justified or refuted by a Lighthouse number — that is why these were not held behind #017. **Implementation shipped 2026-07-29; browser verification still PENDING:** 10 of 11 `ssr:false` dashboard surfaces had NO `loading` fallback and rendered nothing between HTML arrival and chunk execution — all now use the shared `LoadingPanel` (`role="status"` + accessible label); Supabase `preconnect`/`dns-prefetch` added, since `AuthProvider` awaits a cross-origin `getUser()` on mount that every auth-gated fetch queues behind and there were no resource hints anywhere in `src/`. Shipped with `verify:cheap` + `verify:pr-local` only. **Next:** run `npm run verify:ui` once the heavy-run lock is free, and confirm the preconnect appears in `` on a live page. **Stop:** the two sidebar dialogs are intentionally excluded — they mount on open, so a fallback would render into a closed dialog. | `docs/audit/latency-audit-2026-07-28.md` L3-4/L3-5; `src/components/clinical-dashboard/clinical-dashboard-lazy.tsx` | 2026-07-29 | -| #106 | P2 | rec | Ingestion worker and indexing agent are verified by grepping their own source | **Outcome:** the ingestion worker and indexing agent are verified by executing code, not by asserting on their own source text. **Detail:** measured 2026-07-29 via `npm run test:coverage` — `worker/main.ts` (2,015 lines) and `supabase/functions/indexing-v3-agent/index.ts` (1,966 lines) each report **0% executed lines**; no test imports either module. Both are covered only by `readFileSync` + `toContain` assertions in `worker-safe-logging.test.ts`, `worker-visual-capture.test.ts` and `document-metadata-merge.test.ts`, which pass whenever a string is present and break on harmless refactors; `document-metadata-merge.test.ts` additionally reimplements the SQL deep-merge in TypeScript and tests the reimplementation rather than the worker. Area totals: `worker/` 18.6% lines, `supabase/functions/` 4.5%. **Next:** continue the extraction pattern that already works here — `indexing-v3-agent/behavior.ts` (167 lines, 96%) and `ingestion-worker/auth.ts` (30 lines, 90%) — pulling the highest-risk decision points out of `worker/main.ts` (job claim/retry, generation commit, failure classification) into importable modules with executing tests, retiring the matching source-text assertion as each lands. Roughly cost-neutral: each extracted test replaces a grep assertion. **Stop:** do not try to make the 2,000-line entrypoint importable in one pass; extract incrementally and keep each step green. | session 2026-07-29 test-coverage analysis | 2026-07-29 | -| #107 | P2 | rec | Component state matrices are the largest untested surface | **Outcome:** loading / empty / error / disabled states on interactive components are covered by executing tests, not only by E2E happy paths. **Detail:** measured 2026-07-29 — production components (excluding mockups) sit at **38.2% lines / 22.8% branch** across 12,602 lines, with **83 of 208 files at zero executed lines**; there are 51 `.dom.test.tsx` files against 195 components. Playwright does visit these routes, so they are smoke-covered, but branch coverage is where the state matrix lives and smoke journeys rarely reach it. Worst by uncovered lines: `global-search-shell.tsx` (7%), `mode-action-popup.tsx` (21%), `answer-content.tsx` (27%), `document-search-results.tsx` (32%), `universal-search-command-surface.tsx` (39%), `master-search-header.tsx` (43%). A concrete first target with clinical meaning: `calculator-ui.tsx` now covers all exported scoring logic, but `seedCheckboxDefaults`, `toggleCheckboxAnswer` and `selectOptionAnswer` stay uncovered because they are module-private and only reachable through React event handlers — `seedCheckboxDefaults` is what makes an all-negative CAGE / SAD PERSONS screen read as a valid 0 rather than incomplete, so a regression there is a false-negative risk. **Next:** treat as a per-PR convention rather than a backfill push — `docs/testing.md` already prescribes the state matrix, so the gap is enforcement. Start with `global-search-shell.tsx`, which `docs/search-chrome-behaviour.md` treats as a contract surface. Keep additions in the jsdom tier (measured ~0.54s per file) instead of new Playwright journeys (~231 production journeys already run serially at `workers: 1` against a 45-minute CI budget). **Stop:** do not chase the coverage percentage by backfilling low-risk components; the re-ratcheted broad floor in `vitest.config.mts` holds the line. | session 2026-07-29 test-coverage analysis | 2026-07-29 | -| #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. **Progress 2026-07-30 — the prerequisite is DONE; only the deletion is left, and it needs a caller who can delete refs.** Re-verified against the live remote first, which mattered: `claude/clinical-kb-pwa-review-asi3wb` and `claude/dazzling-blackwell-f348d0` are **already gone** (surfaced by `git remote prune`, deleted by someone with the permission this session lacks), so the list is three, not five. The remaining three are still at exactly the recorded HEADs, and the proof was re-run and strengthened: for each, `tree(tip) == tree(merge-base)` byte-for-byte, so the branch nets zero content change from where it forked and nothing on it is absent from `main`. That is stronger than either check the guide names — `--cherry-pick` still reports 13/6/4 patch-unique commits on them, which is the squash-merge false positive, and none is an ancestor of `main`, so `--merged` would also miss them. A completed `branch-cleanup` row keyed to its own HEAD is now appended for each of the three (`npm run check:branch-review-ledger` passes at 1272 records), so the ORDER MATTERS constraint above is satisfied and the refs can now be deleted safely at any time. Deletion re-attempted and still blocked: `git push origin --delete` exits 1 with **HTTP 403** from the session git proxy (`recentRelayFailures` empty, so it is credential scope, not a relay fault), and a tool search confirms the GitHub MCP set exposes `create_branch` but no delete-branch capability. **Next:** delete these three refs from the GitHub UI or any session whose credentials permit ref deletion — `codex/document-reader-condensed-view`, `cursor/page-anchored-search-composer-30ee`, `cursor/pr-1379-babysit-ledger-9365`. Nothing else is required first. **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. **FIXED 2026-07-29:** `scripts/sweep-branch-ledger.mjs` now refuses outright on a shallow clone via the exported `shallowCloneRefusal`, printing no inventory and exiting 1 in both text and `--json` mode, before the fetch and before any branch maths. `docs/branch-cleanup-guide.md` §Safety Rules gains the `is-shallow-repository` precondition ahead of its numbered steps, because the raw `git` commands it documents have no such guard. Proven in a real `--depth 1` clone: unguarded the sweep exited **0** and named the live checked-out branch a deletion candidate with "no unique patch content"; guarded it exits 1 with the `--unshallow` remedy. Five cases in `tests/repo-hygiene.test.ts` cover both directions, including that the string `"false"` (truthy) must NOT be read as shallow — the way this guard could fail dangerously in reverse. **Hardened in review:** an indeterminate `is-shallow-repository` result (empty output from a swallowed `git` failure) is now refused as its own failure rather than treated as complete, and the same refusal was extended to `scripts/reconciliation-preflight.mjs`, which reports its own merge-base-derived ahead/behind. That guard then had to move OUT of the preflight CLI and INTO the exported `collectReconciliationState`, because `buildReconciliationEvidencePack` calls the collector directly and stamps `status: "complete"`: in a `--depth 1` clone the guarded CLI exited 1 while the evidence-pack command exited 0 and persisted shallow ahead/behind as completed evidence. The collector now throws `UnverifiedHistoryError` (`code: "history-not-verified"`), so every current and future caller fails closed by default instead of by remembering to ask; the CLI catches it only to keep the `--json` envelope. Regression cases live with each entry point (`tests/reconciliation-preflight.test.ts`, `tests/reconciliation-evidence-pack.test.ts`) and build a real `--depth 1` clone, asserting `is-shallow-repository` is `true` first so a git behaviour change cannot make them pass vacuously. **Second failure mode, found in review after the first fix landed: complete history is not complete branch coverage.** `git clone --depth 1` implies `--single-branch`, pinning `remote.origin.fetch` to the one cloned branch; `git fetch --unshallow` converts the history so `--is-shallow-repository` reads `false` and the shallow guard passes, but it does not widen the refspec, and an ordinary `git fetch origin` respects the narrow one. Measured in a `main`+`feature` fixture: after unshallowing, `git ls-remote --heads origin` listed both while `refs/remotes/origin` held only `origin/main`, and the sweep exited **0** reporting `"branches": []` — and an empty inventory is not a safe failure, since it reads as "nothing to clean up" and a missing `origin/main` makes every `rev-list` fail into `0/0`, i.e. every branch a deletion candidate. Fixed both ways: the sweep's fetch now passes an explicit `+refs/heads/*:refs/remotes/origin/*` (repairing coverage without rewriting the operator's config), and `branchCoverageRefusal` refuses when neither the configured refspec nor a completed wildcard fetch establishes coverage — `--no-fetch`, offline, or a failed fetch. Its remedy is deliberately `git remote set-branches origin '*'`, not `--unshallow`, which fixes history and does nothing here. **Two further routes to the same empty-inventory answer, both found in review, both from checking only half of the refspec.** (1) The DESTINATION matters as much as the source, because the sweep enumerates `refs/remotes/origin` and nothing else: with `+refs/heads/*:refs/remotes/upstream/*`, `refs/remotes/upstream` held `upstream/main` and `upstream/feature` while `refs/remotes/origin` stayed empty and the sweep exited **0** with `"branches": []`. (2) Git substitutes the matched suffix into ``, so a `refs/*` source nests one level deeper: `+refs/*:refs/remotes/origin/*` writes `refs/remotes/origin/heads/main`, `origin/main` then does not resolve at all, every comparison fails into `0/0`, and the sweep exited **0** naming both `heads/feature` and **`heads/main`** as deletion candidates — a green run recommending the deletion of `main`. Coverage from config therefore requires exactly `refs/heads/*` to `refs/remotes/origin/*`; a completed wildcard fetch still establishes coverage by itself, since the sweep passes that destination explicitly. **Stop:** never delete a branch, or report a branch as unmerged, from a shallow clone, a single-branch refspec, or a refspec whose destination is not `refs/remotes/origin/*`. | 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 | P2 | issue | `issues:next-id` has no concurrency protection | **Outcome:** two agents working the same hour cannot allocate the same ledger id. **Detail:** the marker at the top of this file is a plain HTML comment read-modify-written by whoever edits next, with no lock and no post-merge check. On 2026-07-29 it collided **twice in one hour**: PR #1391 claimed `#096`/`#097`, lost them, claimed `#098`/`#099`, lost those too, and its final pair `#108`/`#109` collided a third time with the branch-cleanup work that reached `main` first — resolved during the #1391 merge by renumbering to `#110`/`#111` and bumping the marker to 112. Each collision is silent: `docs/outstanding-issues.md` has NO union merge driver (unlike `docs/branch-review-ledger.md`), so it presents as an ordinary content conflict that a hurried resolution can settle by dropping one side's rows entirely. Nothing checks for duplicate ids afterwards. **Next:** add a duplicate-id and marker-consistency check to the verification gates — the cheapest useful form is a test asserting every `#NNN` id appears exactly once across both tables and that the marker exceeds the max, which turns a silent loss into a red gate. Consider`merge=union`in`.gitattributes` as well, though row-level union does not by itself prevent two rows sharing an id. **Stop:** do not resolve one of these conflicts by taking one side wholesale without diffing the id sets first; that is how rows get dropped. | session 2026-07-29 PR sweep; PR #1391 conflict resolution; `.gitattributes` | 2026-07-29 | -| #113 | P2 | issue | `ModeNav` clips its labels at every phone width on `main` | **Outcome:** the mode navigation bar honours its own "labels are never abbreviated" contract, or does not render. **Detail:** measured in Chromium against the running app on 2026-07-29 (PR #1390 review, landed as `8f861bb0`), `span.truncate` `scrollWidth` vs `clientWidth` on `/therapy-compass/search`: 320px viewport clips `Compare` to **15 of 56px**; 360px → 29px; 390px → 39px; 412px → 46px; **430px is the worst case at 3 of 4 labels clipped** (`Compare` 17px, `Recommend` 57 of 77, `Pathways` 57 of 60) because crossing the 26rem band adds a fourth slot to the same space; clean only from ~35rem. Root cause is `grid-auto-columns: 1fr` (`globals.css:2373`) in both lower bands — equal tracks make the WIDEST slot set what every slot needs — combined with `truncate`, so the shortfall is silent. The CSS block's stated budgets are wrong: it claims four labels need 394px, measured intrinsic widths are Search 92.4 / Compare 144.3 / Recommend 125.6 / Pathways 108.5 = **471px**, short by roughly the `0/4` badge plus its gap. **Next:** pick one — (a) content-size every band (adopt the ≥34rem `display: flex` shape throughout) and move thresholds to ~21rem for three slots and ~31rem for four; (b) drop the count badge below the top band, worth ~38px; (c) raise the thresholds so phones keep the collapsed control. (a) is the smallest change and deletes a layout mode. Add the band-boundary browser assertion review asked for, covering the widest label in the set. **Stop:** a hardcoded `rem` threshold cannot guarantee fit for an arbitrary item list, and `ModeNav` is shared — do not treat a Therapy-tuned number as a general solution. | PR #1390 thread `PRRT_kwDOSh5Fis6UyB5X` (open at merge); measured session 2026-07-29 | 2026-07-29 | -| #114 | P2 | issue | The live Web-Vitals instrument cannot measure its own noise | **Outcome:** the `#017` baseline rests on evidence whose reproducibility can be checked. **Detail:** `live-web-vitals.yml` (landed `8dbfc5d1`) runs Lighthouse **once** per route/strategy. `#017` asks for reproducible evidence and says to stop when the evidence is too noisy — one sample can neither establish reproducibility nor recognise noise, so the instrument cannot detect the condition its own governing item tells the operator to stop on. The rule is a hard threshold (LCP < 2500ms), so a route near the line resolves to a pass or a breach on run-to-run variance alone, invisibly, and a favourable sample would mark **seven** gated findings WONTFIX. Related and also unclosed: Chrome ships with the `ubuntu-24.04` runner image and is NOT pinned by `LIGHTHOUSE_VERSION`, so a metric shift between a baseline and its follow-up can originate in the browser; the build is now recorded per report in `summary.json` (`chromeVersions`) so a cross-version comparison is visibly invalid, but nothing prevents one. **Next:** N runs per route/strategy with a sample-indexed report name, `expectedRuns`/`incompleteEvidence` reworked to expect N per cell, median as the graded aggregate (the Lighthouse and `lighthouse-ci` convention), and — most important — a spread that straddles a threshold treated as INCOMPLETE EVIDENCE rather than resolved either way. Costs N× dispatch time. Pinning Chrome needs a container or a setup action. **Stop:** do not record an `#017` verdict from a single-sample run, and do not compare baselines whose `chromeVersions` differ. | PR #1385 thread `PRRT_kwDOSh5Fis6U0Zq0` (open at merge); `scripts/summarise-web-vitals.mjs` | 2026-07-29 | -| #115 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` now parses each file with `@babel/parser` and follows a static `import { X } from "…"` only when `X` is actually mounted — rendered as JSX, re-exported as the default (a wrapper page mounts with no JSX of its own), or re-exported by name. `import("…")`, `export … from "…"` and side-effect imports are followed unconditionally, because each is itself a mount mechanism rather than a binding that can go unused; the lazy form is required for the band behind the code-split dashboard workspaces. Verified by reducing `(search-app)/services/page.tsx` to `
` with its imports intact: previously green, now reported as an orphan; restored, it passes. Pinned by temp-dir fixtures covering imported-but-unrendered (false), default re-export (true) and lazy `import()` (true). The residual `void Binding` / type-only-reference shape is unchanged and still caught by `no-unused-vars` under `--max-warnings 0`. | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` now parses each file with `@babel/parser` and follows a static `import { X } from "…"` only when `X` is actually mounted — rendered as JSX, or supplied as this module's default export. Followed unconditionally: `import("…")` (required for the band behind the code-split dashboard workspaces), `export { default } from "…"`, and side-effect imports. **Deliberately not followed:** `export { X }`, `export { X } from "…"` and `export * from "…"` — a re-export renders nothing, so counting one let a page re-export a banded component while its own default rendered `
`; wherever the re-export is finally mounted, the walk sees the JSX there. Verified by reducing `(search-app)/services/page.tsx` to `
` with its imports intact: previously green, now reported as an orphan; restored, it passes. Pinned by temp-dir fixtures covering imported-but-unrendered, all three re-export spellings (false) and default re-export plus lazy `import()` (true), each confirmed to fail against the prior behaviour. Two of the three false greens were found in review after a fix landed, which is why the fixtures assert direction rather than only passing. The residual `void Binding` / type-only-reference shape is unchanged and still caught by `no-unused-vars` under `--max-warnings 0`. Remaining known gap, tracked here rather than left silent: a dynamic `import()` is followed regardless of which lazy binding is mounted, so `clinical-dashboard-lazy.tsx` mounting any one lazy export reaches them all — closing that needs the walk to track bindings per file, not just files. | 2026-07-30 | -| #116 | P2 | issue | An unmergeable PR runs no CI at all, with no signal that it stopped | **Outcome:** a PR that has silently stopped being tested says so. **Detail:** on 2026-07-30 PR #1400 ran **no** `CI`, `Gitleaks` or `Semgrep` workflow across three consecutive pushes, and nothing anywhere said why. Cause: the PR had a real content conflict with `main`, so GitHub could not build `refs/pull/1400/merge`, and every `pull_request`-triggered workflow is skipped in that state. `pull_request_target` ones (`PR Policy`) still ran, and CircleCI posted failure ~3s after each push - faster than a checkout, so no step executed. The symptom reads as "CI is broken" or "my tests fail", and about 40 minutes went into replicating CI steps locally (all green) before the cause was found. **Diagnostic:** compare check-run counts with another open PR (3 vs 16-19), list workflow runs and note only `pull_request_target` fired, then confirm with `git merge-tree --write-tree origin/main HEAD`. **Next:** make staleness visible instead of silent - e.g. a `pull_request_target` job that fails when `mergeable_state` is `dirty`, so a conflicted PR shows one red check naming the conflict rather than an empty check list. `npm run sync:pr-branches` covers the behind-but-clean case only. **Stop:** never conclude "CI is failing" from a missing check - the absence of a check is not a failing check. | PR #1400; session 2026-07-30 | 2026-07-30 | +| ID | Pri | Type | Summary | Detail / next action | Source | Added | +| ---- | --- | ----- | -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------ | ---------- | +| #059 | P1 | task | Verify containment of every credential reported exposed in chat | **Outcome:** every reported exposed credential is rejected or retired. **Next:** in approved security windows, verify and revoke or rotate the GitHub token, OpenAI key, Supabase service-role JWT, database password, and E2E credential; create replacements only when required and update only intended secret stores. **Success:** provider evidence confirms the old credentials cannot authenticate, replacements are distinct and minimally scoped, presence/readiness checks pass, and secret scans remain clean. **Stop:** no provider or secret-store action without approval; never print or paste values into Git, logs, issues, or chat. | session 2026-07-24 security reconciliation; AI Agent Target Manifest | 2026-07-24 | +| #065 | P2 | task | Complete the paused compact document source-text accordion | **Outcome:** the document viewer uses compact nested disclosures while retaining complete text, citation/search navigation, print behavior, and composer clearance. **Next:** only when the user explicitly resumes, reconcile `codex/chat-document-text-accordion-7cb4` with current `main` and complete the focused 320/390/1280 px tests. **Success:** default disclosures are closed; deep links and search open only the active passage; printing expands/restores state; no overflow. **Verify:** focused document-viewer Playwright, `verify:cheap`, `verify:ui`, and static production-readiness. **Stop:** remain paused until explicit user return; no provider calls. | paused document-viewer task; `codex/chat-document-text-accordion-7cb4` | 2026-07-24 | +| #001 | P2 | task | Semantic reranking still gated off | `RAG_SEMANTIC_RERANK_ENABLED=false` from PR #901. Do not enable until the provider-backed 36/36 retrieval-quality gate **and** an ambiguity-focused canary are explicitly approved and recorded. | `docs/process-hardening.md` (Semantic reranking rollout debt); PR #901 | 2026-07-21 | +| #053 | P1 | task | Execute cross-border privacy/legal package | Execute OpenAI and Railway DPAs; decide ZDR and Australian data residency; obtain prompt-cache behavior in writing; review subprocessors; obtain APP 8 and APP 5/1 counsel sign-off. Do not represent the release as privacy-approved or alter final public privacy wording before sign-off. | `docs/openai-cross-border-basis.md`; `docs/privacy-impact-assessment.md` | 2026-07-24 | +| #055 | P2 | task | Run one exact-SHA full release and PR gate | Before the next full-confidence release/handoff, record the candidate/PR SHA and run the local/provider release gates, Firefox/WebKit, required hosted CI, and actionable GitHub review-thread closure once. Stop at the first actionable failure and rerun only the repaired smallest gate. | `docs/launch-operator-runbook.md`; `docs/codex-review-protocol.md` | 2026-07-24 | +| #056 | P2 | task | Reconcile the existing staging migration history | `Clinical KB Staging` already exists as a healthy, empty Supabase/Railway tier with distinct secrets and no production clinical data, but it is 23 repository migrations behind. In the next approved staging schema window, apply the exact missing migration chain, then re-run indexing, health, identity and data-boundary proof. Do not recreate the environment or copy production clinical documents. | current-main staging verification; `docs/staging-setup.md`; `docs/operator-backlog.md` | 2026-07-27 | +| #057 | P2 | task | Complete staging soak and rollback rehearsal | After #056, run the documented soak and rollback against an exact candidate; retain latency/error/rollback evidence. Stop on unsafe data, identity mismatch, or an unowned rollback decision. | `docs/launch-operator-runbook.md`; `docs/capacity-review.md` | 2026-07-24 | +| #005 | P3 | rec | `finalScore` saturates at clamp ceiling | Base + ~40 stacked boosts routinely exceed 1.0, so strong matches tie at 1.0 and order by an arbitrary `document_id` tiebreak. If ranking is ever revisited, break ties by the **pre-clamp** score rather than raising the `[0,1]` ceiling (downstream gates assume `[0,1]`). Ordering already sorts by the unbounded pre-clamp `rankScore` (`clinical-search.ts:1735,1927,1950-1955`), so the clamp confines only the reported confidence value, not result order. Not a defect on the current golden set; any change here is a protected RAG surface (canary required). | `docs/rag-hybrid-findings-and-todo.md` P1 item 4; `src/lib/clinical-search.ts:1735` | 2026-07-21 | +| #011 | P3 | task | Auth DB-connection allocation is operator-only | Supabase Auth (GoTrue) is capped at ~10 absolute DB connections (Supabase perf advisor). Switch to **percentage-based** allocation in the Supabase **dashboard** before the first compute scale-up — **not settable via SQL/MCP** (operator-owned). Verify via a staging soak + an approval-gated read-only advisor re-check. | `docs/auth-connection-cap-runbook.md`; `docs/process-hardening.md` (Known follow-up debts) | 2026-07-21 | +| #013 | P3 | rec | Route-chunk + mockup catalogue JSON weight | `build:analyze`: `/specifiers` ships `specifiers-search-index.json` (~180 KB parsed), `/forms` ships `forms-catalog.json` (~132 KB), `/formulation` ships `formulation-content.json` (~52 KB, client-side local search — needs index/full split or a search endpoint, architectural). All route-scoped (not initial bundle). Also `*-mockups.tsx` (~100 KB across chunks) build though `/mockups` 404s in prod — exclude from the prod artifact. | session 2026-07-21 (build:analyze) | 2026-07-21 | +| #016 | P3 | rec | "Big but not easy" structural + motion perf | Deferred larger levers: (a) nonce-CSP forces every product route to `╞Æ Dynamic` (zero static generation) — evaluate Partial Prerendering / static shells for the static clinical catalogues (DSM/differentials/therapy/specifiers/formulation); (b) sidebar expand/collapse animates `grid-template-columns` (biggest smoothness cost, motion-gated — needs a transform-overlay rethink); (c) Therapy Compass fetches 692 KB / 2.5 MB JSON client-side (defer until interaction + confirm brotli); (d) settings/setup/admin dialogs static-imported into the home chunk (`next/dynamic` them); (e) `src/app/(search-app)/layout.tsx:4` imports 71.6 KB of Therapy-Compass-only CSS in the ROUTE-GROUP layout, making it render-blocking on `/`, `/documents`, `/forms`, `/dsm` and every mode home; (f) `shared-search-app-shell.tsx:8` statically imports the `therapy-compass` barrel, pulling `workspace.tsx` + `bindings.tsx` + `nav.tsx` into every `(search-app)` route; (g) three client waterfalls (`use-app-preferences.ts:156-182`, `ClinicalDashboard.tsx:977-1069`, `signed-image.tsx:60-84` + `use-signed-image-url.ts:39`) and the paint offenders in `globals.css` beyond the sidebar grid — three stacked `backdrop-filter` passes on an always-mounted translating element (`:709-748`), `box-shadow` inside a `transition` list (`:677-684`), and `@keyframes shimmer` animating `background-position` on the shared `Skeleton` (`:2289-2296`). **CORRECTED 2026-07-29 on (c):** the Therapy Compass filenames are unversioned and Next serves `/public` with an ETag, so only the FIRST visit pays 690.6 KB / 2,470 KB — repeat visits pay ~4 revalidation round trips. The fix is content-hashed filenames + `immutable` (touching `scripts/build-therapies-index.mjs` and `check:therapy-data-index`), NOT a bare `Cache-Control` line. See `docs/audit/latency-audit-2026-07-28.md` L3-1/L3-2/L3-3/L3-6/L3-7. | session 2026-07-21 (build route table + design audit) | 2026-07-21 | +| #017 | P3 | task | Field Web-Vitals baseline via live Lighthouse | In-sandbox runtime vitals were blocked (prod server hard-requires Supabase secrets; dev-mode CLS measured excellent at 0.00–0.04, content-first pages 0.000). Run Lighthouse against `psychiatry.tools` for real LCP/INP/CLS to prioritize #012–#016 by measured impact rather than reasoning. **Instrument landed (PR #1385):** `live-web-vitals.yml` is dispatch-only, takes no secrets and runs nothing until someone dispatches it — which is itself a live production action needing explicit approval. `scripts/summarise-web-vitals.mjs` holds the decision rule, committed before any numbers were read: mobile LCP < 2500 ms AND CLS < 0.1 on every route (plus INP < 200 ms from CrUX) closes this row and makes only the explicitly measured **payload** findings gated by #017 WONTFIX — e.g. the route-chunk/catalogue weight in #013. It does **not** close #016 wholesale: #016's motion, CSS, client-waterfall, caching, and dynamic-import items stay open unless they have separate evidence, because Lighthouse LCP/CLS do not validate those. Any breach makes only that route's findings actionable. It fails closed — a missing run, a null metric, a route-slug collision, and a measurement that landed on a different URL than requested (redirect, dropped or reordered query) are all breaches. **Read the first dispatch as a measurement to be sanity-checked, not as an oracle:** the grading logic took eight rounds of review corrections to reach this shape, each fix locally right and globally incomplete, so cross-check the emitted table against the raw Lighthouse JSON artifact before recording a verdict — most of all a PASS, which would close #017 and the explicitly measured payload findings. Lighthouse cannot measure INP in lab conditions, so that clause of the rule is confirmed from CrUX field data and any pass is provisional on it. | session 2026-07-21 (measurement pass) | 2026-07-21 | +| #018 | P2 | task | Split the lithium, ADHD and metabolic residuals by mechanism | Current evidence keeps the mechanisms separate. **Lithium — closed within this item:** the row/atom-aware subject guard, foreign-parameter rejection and query-specific range promotion returned `0.5–1.0 mmol/L` with correct targeting/citation; the full retrieval canary remained 36/36 with recall 1.0 and zero per-case RR regressions, and the full answer canary passed every blocking gate. **ADHD — open corpus debt:** `CG.MHSP.ADHD.pdf` is absent from the hosted corpus and the retrieved chart exposes `accessible_table_count=0`; repair corpus/fixture or ingestion evidence rather than weakening extractive budgets. **Metabolic — open structured-evidence debt:** the standalone plural classifier worsened the live answer and was reverted; obtain auditable schedule text/table evidence before another candidate. | targeted live lithium/ADHD/metabolic evidence 2026-07-27; `docs/evidence/rag-reliability-evidence-2026-07-27.md`; refuted approaches | 2026-07-21 | +| #021 | P3 | rec | E-3d H2 residual: strong/comparison generation discards | approx. 6 generation attempts per full 44-case run still fail the final quality gate and fall to extractive on strong-route comparison/complex shapes (the designed-conservative outcome). PARKED: weakest cost/benefit on the queue — a wave (approx. $2-4 pair + reviewer cycle) to shave seconds off a few hard cases. Revisit only if latency/waste complaints or a cheaper lever appears. | E-3c design record; runs #59-#61 diagnostics | 2026-07-21 | +| #022 | P2 | task | Source-governance metadata refresh (operator) | The selected policy is now encoded locally as auditable `third_party_reference_attested` evidence with policy version, reviewer qualification, evidence references and append-only review history. It deliberately preserves `clinical_validation_status=unverified`; malformed, stale or non-BMJ evidence remains review debt. Migration `20260727010000_bmj_third_party_source_attestation.sql` is prepared but was **not applied**. The ten most visible local-document candidates are captured in `docs/evidence/rag-top-local-review-manifest-2026-07-26.json` with `attestation_applied=false`; qualified human review, deliberate hosted apply/attestation, and warning-rate remeasurement remain operator work. | governance worklist; local policy/migration tests; top-ten evidence manifest | 2026-07-21 | +| #023 | P2 | task | Complete scheduled browser and labeling disposition | The 2026-07-26 retrieval and answer artifacts are read and compared under resolved #051. Scheduled CI run `30216361999` failed its existing production dependency audit before Firefox/WebKit, while production Chromium passed. After that audit is green, capture one scheduled/manual browser-matrix datapoint; separately record the human decision for the stable irrelevant-at-10 set. #084 now makes each top-10 grade and matched signal reproducible, but it does not substitute for the human disposition. Do not rerun or spend on RAG for this item. | runs `30216191889`/`30216361999`; per-rank diagnostics #084; session 2026-07-27 | 2026-07-21 | +| #024 | P2 | issue | WebKit e2e `_rsc` prefetch access-control errors | PR #1205 narrowed catch-all interception and duplicate navigation, but Next 16.2.11 still raises `_rsc` access-control `pageerror`s after document-source fallbacks: `/documents/source?id=&page=2&chunk=safety%20plan` → `/documents/?page=2&chunk=safety+plan`; `/documents/source/evidence?id=not-a-uuid&page=2` → `/documents/search`. The invalid-id failure survived removing every Playwright route; Chromium passed both. **Next:** on a provider-free macOS host, run both URLs in stable Safari and Safari Technology Preview without interception, capture console text plus `_rsc` status/access-control headers, and compare Playwright WebKit with routing on/off. Treat as an app defect only if native Safari reproduces; otherwise return to the harness. Never suppress `pageerror` or change CORS without native evidence. | PRs #1179/#1205; current-main local WebKit evidence; session 2026-07-28 | 2026-07-28 | +| #025 | P2 | task | Activate the three webhooks (operator secrets) | Merged (#968/#1100) + deployed but inert — verified live: `POST /api/webhooks/railway` returns `503 webhook_not_configured`; the Supabase document-change trigger exists but lacks both activation inputs. To turn on: (1) Railway ΓåÆ set `RAILWAY_WEBHOOK_SECRET` + add the `?token=…` webhook URL; (2) set `SLACK_WEBHOOK_URL`/`DISCORD_WEBHOOK_URL` in BOTH the Railway **app/server env** and **GitHub repo secrets**; (3) set one matching document-change secret in the Railway app env as `SUPABASE_INGESTION_WEBHOOK_SECRET` and in Supabase Vault as `ingestion_webhook_secret`, then set the per-environment database GUC `app.ingestion_webhook_base_url` to the deployed app origin. Each path fails closed until fully configured, so this is pure ops. See `docs/webhooks.md` for verification and rotation. | sessions 2026-07-22/24; PRs #968/#1100; docs/webhooks.md | 2026-07-22 | +| #027 | P3 | rec | External uptime monitor independent of GitHub/Railway | `live-domain-monitor.yml` runs on GitHub's cron, so it won't run in exactly the outage it should catch (Actions or the deploy itself down). Add an off-platform synthetic monitor (UptimeRobot / Better Stack / Checkly) hitting `/api/health` with a webhook alert. Provider setup, not code. | session 2026-07-22 webhook review | 2026-07-22 | +| #028 | P3 | rec | Runtime error tracking (Sentry or similar) | No error tracking in the repo — production exceptions on `psychiatry.tools`, including how often `RAG_PROVIDER_MODE=auto` silently degrades to source-only, are invisible. Weigh adding `@sentry/nextjs` (dependency + DSN secret + instrumentation) vs cost; alert ΓåÆ chat/issue. Provider-backed; needs explicit sign-off before adding the dependency. | session 2026-07-22 webhook review | 2026-07-22 | +| #033 | P3 | rec | Source governance metadata absent from the LLM prompt | `buildRagSourceBlock` omits `document_status`, `clinical_validation_status`, and `extraction_quality`, so the model cannot self-caveat during generation and governance is enforced only post-hoc. Generation-surface change: needs `eval:rag` plus `eval:quality --rag-only` (grounded-supported must not drop, citation-failure 0) and explicit approval. Carries the same "unknown Γëá bad" hazard as #032 — on a partially-enriched corpus the model would likely over-caveat correct sources, so design the prompt wording before spending an eval. | `src/lib/rag/rag-source-block.ts:126-198`; PR #1051 audit item 8 | 2026-07-22 | +| #035 | P3 | rec | Threshold-conflict detection covers only 3 params | `detectThresholdDisagreements` checks only ANC, WBC, and platelets paired with withholding verbs, so cross-source conflicts on medication doses, lithium/thyroid levels, or vital signs go undetected. Deliberately narrow (see the comment at `:469-474`). Broadening changes when an answer is classified `conflicting` and adds warnings — real false-positive risk. Needs new fixtures plus a behaviour review before any change. | `src/lib/evidence.ts:469-574`; PR #1051 audit item 7 | 2026-07-22 | +| #036 | P3 | rec | No explicit `is_public` visibility flag on documents | Public-corpus visibility is implicit: `owner_id IS NULL` on an `indexed` document (`resolveSearchScope`). The `metadata.public_corpus` marker is written by the promotion migrations but never used as a retrieval filter. Promotion is unconditional on `clinical_validation_status`, so unverified documents are publicly searchable — compensated by keeping `unverified_source` in the frontend-visible warning set. A hard schema flag touches RLS and the clinical-risk-gated retrieval RPCs; weigh against the existing compensating control before acting. | `supabase/schema.sql:61-108`; `src/lib/search-scope.ts:181-236`; PR #1051 audit item 3 | 2026-07-22 | +| #037 | P3 | rec | D5 trust-cap-all-claims flag parked OFF | `NEXT_PUBLIC_RAG_TRUST_CAP_ALL_CLAIMS` extends authority gating from high-risk claims to **all** supported claims (`deriveTrust`). Ships OFF by design; flipping it caps trust to `medium` for routine claims across the board — a product/clinical-UX decision, not a defect. Both states are test-pinned. Next action: product decision, then flip and re-baseline the UI expectations. | `src/lib/answer-render-policy.ts:159-177`; PR #1051 audit item 11 | 2026-07-22 | +| #038 | P3 | rec | Consolidate shared comparison behavior | Several clinical modes expose comparison workflows with similar selection, empty-state and mobile-dock needs. Define one shared behavioral contract before another comparison surface is added; keep mode-specific clinical content separate. This is a design-system recommendation, not a current defect. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | +| #039 | P3 | rec | Consolidate catalogue toolbar patterns | Catalogue/search pages have independently evolved filter, sort, result-count and mobile toolbar behavior. Inventory the existing implementations and converge only the repeated interaction contract; do not flatten mode-specific search semantics. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | +| #040 | P3 | rec | Add targeted visual-regression baselines | Keep a small approved baseline set for high-value desktop/mobile surfaces and accessibility modes instead of screenshotting every route. Start with account/settings, document viewer, mode homes and bottom-composer interactions; define an intentional-update workflow before enabling blocking comparisons. | design audit reconciliation; session 2026-07-22 | 2026-07-22 | +| #079 | P3 | task | Disposition retained worktrees in bounded cleanup batches | **Outcome:** the retained reconciliation tail is gradually classified without another disruptive all-worktree sweep. **Next:** process no more than ten worktrees per explicitly scheduled pass using current owner/process metadata, open-PR state, exact review-ledger coverage, ancestry, and cherry-pick-aware content proof. **Success:** remove only clean, inactive, bundled worktrees whose content is merged or explicitly rejected; record every disposition and retain recovery evidence. **Stop:** preserve dirty, active, secret-bearing, post-freeze, paused, or ambiguous work and never use reset, force deletion, broad clean, or process killing. | final reconciliation inventory retained 104 independent worktrees; session 2026-07-24 | 2026-07-24 | +| #085 | P3 | rec | Upload-limit client/server sync is unguarded | `NEXT_PUBLIC_MAX_UPLOAD_MB` (client, build-time inlined) and `MAX_UPLOAD_MB` (server, runtime) default 150/150 but nothing keeps them in sync — no zod link, gate, or test. Lower server-only → the 413-after-full-transfer UX FV-04 (#1064/#1069) was built to prevent; lower client-only → false pre-check rejection of files the server would accept (breaks #1064's client-is-a-strict-superset invariant). Both documented in `.env.example`/`docs/deployment-architecture.md` but unenforced; the client value is also frozen at `next build`, so changing the Railway service var without an image rebuild silently no-ops. Cheapest guard: a `check:*` script (or CI assertion) that fails when the two configured values disagree. | session 2026-07-28 (FV-04 adversarial workflow, PR #1069); ID #085 after #084 claimed on main | 2026-07-27 | +| #086 | P3 | task | Repository maturity backlog — remaining structural work | **Outcome:** the deferred repository-maturity backlog ships as verified draft PRs, one per structural change. **Canonical runbook:** [`docs/maturity-backlog-workorders.md`](maturity-backlog-workorders.md). **Remaining:** X3 `rag.ts` decomposition; X7 finish the `src/lib` domain reorg; X6 clinical/retrieval/answer coverage floors; X5 ACL-migration consolidation (provider-gated); L1 archive the retired `backfill:*` one-shots + the dead `ci-change-scope` token; L4 ledger rotation; M1 repo-host hardening (maintainer, audit §8). **Next:** X3 on user go-ahead. **Stop:** RAG/retrieval items need the flag + go-ahead; X5 is live-DB provider-gated. | `docs/maturity-backlog-workorders.md`; audit §8/§10; session 2026-07-28 | 2026-07-28 | +| #088 | P3 | task | Watch for union-driver duplication as open PRs merge the repaired ledger | **Outcome:** the 2026-07-28 ledger repair does not resurface as duplicated rows. **Next:** on the first few open PRs that merge `origin/main` after the repair, run `npm run check:branch-review-ledger` on the merged head. Ordinary 3-way merges take main's repaired lines cleanly; only a same-hunk conflict would let `merge=union` keep both the corrupted and repaired copies. **Success:** three consecutive post-repair merges pass the guard. **Stop:** if duplication appears, remove exact duplicates only (the ledger contract allows that) and never rewrite surrounding records. | branch-review-ledger hygiene pass rewrote 226 historical lines; session 2026-07-28 | 2026-07-28 | +| #090 | P2 | task | Upgrade the eslint ecosystem to clear remaining dev-scoped high advisories | **Outcome:** full `npm audit` reports zero high advisories from the eslint toolchain. **Next:** in a dedicated dependency pass, upgrade eslint and its plugin/config set together (npm offers `eslint@10.8.0`, `isSemVerMajor`); residual highs (`@eslint/config-array`, `@eslint/eslintrc`, `eslint`, `eslint-config-next`, `eslint-plugin-import`, `eslint-plugin-jsx-a11y`, plus the advisory's numeric `<=5.0.7` hit on the unused `brace-expansion@1.1.16` / `2.1.2` maintenance lines that still ship an unpatched `main`) cascade from this toolchain. **Success:** `npm run lint` clean, `verify:cheap` green, full-audit highs cleared, no rule-config regressions. **Stop:** an eslint major previously broke `eslint-plugin-react` here — do not bundle into an unrelated PR, and do not force with `npm audit fix --force`. Production `npm audit --omit=dev` is already clean after the exceljs `archiver@8` / `unzipper@0.12.5` overrides on PR #1314; this item is eslint-dev cleanup only. | session 2026-07-28 brace-expansion triage (PR #1314) | 2026-07-28 | +| #091 | P2 | issue | Results band cannot express a partial-source failure | **Outcome:** a favourites/results count is never asserted while some of its sources failed to load. **Detail:** `SearchResultsBandStatus` is a single flat status, so a page fed by several registries has no way to say "3 loaded, 1 failed". `saved-registry-favourites-status.ts:29` works around this with `itemCount > 0 && folded !== "ready" ? "ready" : folded`, and `favourites-command-library-page.tsx` applies the same mask a second time — so a partial failure renders a confident count with no fault indication, which is the exact defect class PR #1316 removed everywhere else. Neither favourites page consumes the true `registryStatus` the fold already returns. **Next:** decide between (a) a `partial` arm on the status union carrying a "some sources unavailable" note beside an honest count, or (b) surfacing `registryStatus` as a separate inline notice and dropping the mask. **Stop:** do not widen the mask to more surfaces before the shape is decided. | PR #1316 review thread PRRT_kwDOSh5Fis6UUf-k; session 2026-07-28 | 2026-07-28 | +| #092 | P3 | task | Refetch pulse deferred on auth-backed registries (privacy invariant) | **Outcome:** a background refresh keeps the prior count visible instead of a skeleton, without weakening identity clearing. **Detail:** the `refetching` status is built in the band and adopted only on `formulation-home-page.tsx`, where the lag is `useDeferredValue` over static data. It is deliberately NOT adopted on `use-registry-records.ts:85`, `use-medication-catalog.ts:76` or `use-differential-catalog.ts:133`, which all clear data on entering loading. `use-differential-catalog.ts:122` states why: "Auth must clear prior identity's matches immediately", and `:164` that "a later retype of any prior query cannot resurrect authorized matches." **Next:** if adopted, guard preservation on identity AND query equality, and pin with a test that an identity change still clears immediately. **Stop:** never hold records across an auth transition. | PR #1316 plan phase 6; session 2026-07-28 | 2026-07-28 | +| #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 | +| #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 | +| #100 | P2 | rec | Buffered answer generation has no incremental verified delivery | **Outcome:** a clinician sees verified answer content before the whole generation completes. Highest-leverage latency finding in the 2026-07-28 audit: generation is buffered (`openai.ts:465`) and delivered in ONE `final` SSE frame, so time-to-first-content equals total latency — a strong answer inside its 25 s SLO still shows a blank panel for 25 s. The 15 s `sse-heartbeat` exists because that silence routinely exceeds 15 s; it instruments the defect rather than fixing it. **Naive token streaming is REFUTED, not merely unbuilt:** `answer-stream-contract.ts:18-21` removed `token`/`revising` deliberately because a rolling deployment would "re-expose unvalidated clinical prose", and raw tokens bypass the numeric-faithfulness gate the 2026-07-01 audit filed as H1. **Only admissible shape:** progressive disclosure of already-verified units (evidence/sources at retrieval-complete, then per-section after that section clears verification) over the existing whitelisted `progress` event. Needs a clinical-governance decision plus a canary pair. Also add the refutation to `docs/rag-behaviour/refuted-approaches.md`. Cross-references #021. **Stop:** do not re-land `token` streaming. | `docs/audit/latency-audit-2026-07-28.md` L0-1; `src/lib/answer-stream-contract.ts:18-21`; `src/lib/sse-heartbeat.ts` | 2026-07-29 | +| #101 | P3 | rec | Canary-gated retrieval parallelisation candidates | **Outcome:** independent retrieval stages stop running serially, proven by a live canary pair. Candidates: metadata/memory/visual hydration triples repeated on four branches (`rag.ts:2460,2493,2521` and three more) while `rag.ts:2751-2804` already parallelises three RPCs in one `Promise.all`, so the omission is inconsistency rather than intent; the nested `await`-in-loop scope enumeration (`search-scope.ts:202,328`); typeahead results never cached (`rag.ts:2698-2711`); universal-search coalescing (`/api/search` has it, `/api/search/universal` does not). Each changes candidate assembly, truncation, or what the next keystroke returns, so each needs 36/36 retrieval plus recall 1.0 and zero per-case rr regressions. Distinct from #001 (semantic rerank). Resolved #075 and #083 are the precedents for why these are gated rather than free. **Stop:** needs the #098 harness and explicit canary approval first. | `docs/audit/latency-audit-2026-07-28.md` L2-1/L2-2/L2-8/L1-5 | 2026-07-29 | +| #102 | P3 | task | Apply the additive `documents` index debt (operator) | **Outcome:** bare-column `ILIKE` and the paged status scan on `documents` are index-served on hosted. `documents_title_trgm_idx` indexes a CONCATENATED expression, so the bare-column predicates in `api/documents/route.ts:193` and `rag-candidate-sources.ts:477` (RAG path) cannot use it and fall back to scanning; `search-scope.ts:271-277` sorts per page against the single-column `documents_status_idx`. **Runbook prepared 2026-07-29 — NOT applied, item stays open:** three `CREATE INDEX CONCURRENTLY` statements authored and reviewed in `docs/operator-apply-performance-latency-remediation.md` — additive, though **the "recall is byte-identical" claim was RETRACTED on 2026-07-29 review**: `fetchDocumentTitleAliasRows` (`rag-candidate-sources.ts:482`) applies `.limit(12)` with no `ORDER BY`, so a new index can change which title-alias documents feed candidate assembly. The documents-list and `(status,id)` uses stay ordering-safe; the RAG-path index is canary-gated, and making that `.limit(12)` deterministic first does **not** lift the gate — an unordered `LIMIT` has no stable selection to preserve, so imposing an order can pick a different twelve and is itself an ordering behaviour change on a retrieval surface, which AGENTS.md requires a canary pair for. Sequencing the ordering fix first is worthwhile (unordered `LIMIT` on a retrieval input is latent nondeterminism regardless) but yields two canary-gated changes, not one (PR #1377 review). **Deliberately NO migration file:** an additive-index migration without a synchronized `schema.sql` mirror and regenerated drift manifest is exactly what closed PR #1312, and the mirror cannot come first because `required_indexes` in `search_schema_health()` (`schema.sql:3178`) runs against live. **Next (operator):** **author the migration first** — `supabase/migrations/` is the source of truth and `schema.sql` only a mirror, so hand-run operator SQL never reaches staging, disaster-recovery replay, or a local `supabase db reset`, and a `required_indexes` registration would fail there (PR #1377 review); follow the `20260717170000_registry_projection_cleanup.sql` idempotent pattern. **That migration must also carry the health-function change** — `required_indexes` lives inside `search_schema_health()`, which is redefined by `create or replace function` in eleven migrations (copy `20260705180000_reconcile_search_health_indexes.sql:62`); editing `schema.sql:3177` alone moves only the mirror and leaves the indexes unmonitored on hosted (PR #1377 review). Then apply concurrently, confirm `indisvalid`, mirror both the index statements and the identical function body into `schema.sql`, run `npm run drift:manifest` (Docker), and deploy the migration LAST — in that order, in one change. Expect `check:drift` to report them as unexpected between steps 1 and 2. **Rollback is three deployed phases, not the reverse of one:** retract `required_indexes` via its own `create or replace function` migration and deploy → drop concurrently live → only then deploy the `schema.sql` removal plus an idempotent forward `drop index if exists` migration, because Supabase wraps migrations in a transaction and a plain `DROP INDEX` there takes the lock the concurrent procedure exists to avoid (PR #1377 review). | `docs/audit/latency-audit-2026-07-28.md` L2-3/L2-5; `docs/operator-apply-performance-latency-remediation.md` | 2026-07-29 | +| #103 | P2 | issue | Wide table-facts trigram index missing from `schema.sql` | **Outcome:** the migration chain and `schema.sql` agree on `document_table_facts` trigram indexes. `supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` creates a wide 5-column trigram index that is **absent from `supabase/schema.sql`**, so local replay and the live database can diverge. Distinct from #102: different owner and verification path. **Next:** confirm whether the wide index exists live, then take one of exactly two routes — **retained:** mirror `document_table_facts_text_trgm_idx` into `supabase/schema.sql` beside the narrow one and regenerate `drift-manifest.json`; **redundant:** drop it through a new forward migration, never by deleting `20260714190000`. **`drift-allowlist.json` is NOT a third option** (PR #1377 review): its own header scopes it to _"Known live-vs-`schema.sql` divergence"_, so it can silence a live drift finding but cannot reconcile the migration chain with the mirror — a fresh `supabase db reset` still runs `20260714190000` and creates the index while `schema.sql` still omits it, leaving this row's stated outcome unmet. **No offline gate catches this today:** the migration↔`schema.sql` parity test (`tests/drift-detection.test.ts:59-68`) only asserts one migration's `schema_drift_snapshot` function definition, not an index inventory — which is why this sits open rather than red in CI, and why a replay-to-schema inventory comparison is the check that would have caught it. Note the narrow `document_table_facts_title_row_param_trgm_idx` (`schema.sql:6425`) is the one the effective RPC expression (`:6726`) actually matches, so the wide index may be genuinely redundant — do not drop it without live scan evidence, per the monitored-not-auto-fixed index policy. | `docs/audit/latency-audit-2026-07-28.md` limitations; `npm run check:drift` | 2026-07-29 | +| #104 | P3 | rec | CORRECTION — the worker's triple image read is deliberate, not debt | **Outcome:** a future audit does not re-file this a third time. The 2026-07-28 latency audit listed L4-2 (`worker/main.ts` `readFile`s each extracted image up to 3x per document — hash, caption on cache miss, upload) as "CONFIRMED with no fix evidence", carried forward from the 2026-07-01 audit's `L11`. **That was wrong.** The 2026-07-01 disposition table already recorded it as a deliberate peak-memory trade-off, and the rationale is documented in place at `worker/main.ts:866-869`: holding every extracted image Buffer for a document with hundreds of multi-MB page images would multiply the worker's peak memory, and disk I/O is the cheaper resource for a background pipeline. The three reads (`:872`, `:1034`, `:1129`) are real but accepted. **Next:** none — revisit only if ingestion throughput becomes a measured complaint AND a bounded-buffer design is proposed. **Stop:** do not "fix" this by caching buffers; that trades a decided memory ceiling for disk I/O nobody has measured as a problem. | `docs/audit/repo-audit-2026-07-01.md` L11 + disposition table; `docs/audit/latency-audit-2026-07-28.md` L4-2 retraction | 2026-07-29 | +| #105 | P3 | task | Verify the `#017`-exempt client latency wins in a browser | **Outcome:** the two zero-payload client fixes are confirmed in a real browser. `#017` gates _payload_ decisions (#012/#013/#016 are all byte-count items); a `loading` fallback ships zero bytes and a resource hint ships ~60, so neither can be justified or refuted by a Lighthouse number — that is why these were not held behind #017. **Implementation shipped 2026-07-29; browser verification still PENDING:** 10 of 11 `ssr:false` dashboard surfaces had NO `loading` fallback and rendered nothing between HTML arrival and chunk execution — all now use the shared `LoadingPanel` (`role="status"` + accessible label); Supabase `preconnect`/`dns-prefetch` added, since `AuthProvider` awaits a cross-origin `getUser()` on mount that every auth-gated fetch queues behind and there were no resource hints anywhere in `src/`. Shipped with `verify:cheap` + `verify:pr-local` only. **Next:** run `npm run verify:ui` once the heavy-run lock is free, and confirm the preconnect appears in `` on a live page. **Stop:** the two sidebar dialogs are intentionally excluded — they mount on open, so a fallback would render into a closed dialog. | `docs/audit/latency-audit-2026-07-28.md` L3-4/L3-5; `src/components/clinical-dashboard/clinical-dashboard-lazy.tsx` | 2026-07-29 | +| #106 | P2 | rec | Ingestion worker and indexing agent are verified by grepping their own source | **Outcome:** the ingestion worker and indexing agent are verified by executing code, not by asserting on their own source text. **Detail:** measured 2026-07-29 via `npm run test:coverage` — `worker/main.ts` (2,015 lines) and `supabase/functions/indexing-v3-agent/index.ts` (1,966 lines) each report **0% executed lines**; no test imports either module. Both are covered only by `readFileSync` + `toContain` assertions in `worker-safe-logging.test.ts`, `worker-visual-capture.test.ts` and `document-metadata-merge.test.ts`, which pass whenever a string is present and break on harmless refactors; `document-metadata-merge.test.ts` additionally reimplements the SQL deep-merge in TypeScript and tests the reimplementation rather than the worker. Area totals: `worker/` 18.6% lines, `supabase/functions/` 4.5%. **Next:** continue the extraction pattern that already works here — `indexing-v3-agent/behavior.ts` (167 lines, 96%) and `ingestion-worker/auth.ts` (30 lines, 90%) — pulling the highest-risk decision points out of `worker/main.ts` (job claim/retry, generation commit, failure classification) into importable modules with executing tests, retiring the matching source-text assertion as each lands. Roughly cost-neutral: each extracted test replaces a grep assertion. **Stop:** do not try to make the 2,000-line entrypoint importable in one pass; extract incrementally and keep each step green. | session 2026-07-29 test-coverage analysis | 2026-07-29 | +| #107 | P2 | rec | Component state matrices are the largest untested surface | **Outcome:** loading / empty / error / disabled states on interactive components are covered by executing tests, not only by E2E happy paths. **Detail:** measured 2026-07-29 — production components (excluding mockups) sit at **38.2% lines / 22.8% branch** across 12,602 lines, with **83 of 208 files at zero executed lines**; there are 51 `.dom.test.tsx` files against 195 components. Playwright does visit these routes, so they are smoke-covered, but branch coverage is where the state matrix lives and smoke journeys rarely reach it. Worst by uncovered lines: `global-search-shell.tsx` (7%), `mode-action-popup.tsx` (21%), `answer-content.tsx` (27%), `document-search-results.tsx` (32%), `universal-search-command-surface.tsx` (39%), `master-search-header.tsx` (43%). A concrete first target with clinical meaning: `calculator-ui.tsx` now covers all exported scoring logic, but `seedCheckboxDefaults`, `toggleCheckboxAnswer` and `selectOptionAnswer` stay uncovered because they are module-private and only reachable through React event handlers — `seedCheckboxDefaults` is what makes an all-negative CAGE / SAD PERSONS screen read as a valid 0 rather than incomplete, so a regression there is a false-negative risk. **Next:** treat as a per-PR convention rather than a backfill push — `docs/testing.md` already prescribes the state matrix, so the gap is enforcement. Start with `global-search-shell.tsx`, which `docs/search-chrome-behaviour.md` treats as a contract surface. Keep additions in the jsdom tier (measured ~0.54s per file) instead of new Playwright journeys (~231 production journeys already run serially at `workers: 1` against a 45-minute CI budget). **Stop:** do not chase the coverage percentage by backfilling low-risk components; the re-ratcheted broad floor in `vitest.config.mts` holds the line. | session 2026-07-29 test-coverage analysis | 2026-07-29 | +| #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. **Progress 2026-07-30 — the prerequisite is DONE; only the deletion is left, and it needs a caller who can delete refs.** Re-verified against the live remote first, which mattered: `claude/clinical-kb-pwa-review-asi3wb` and `claude/dazzling-blackwell-f348d0` are **already gone** (surfaced by `git remote prune`, deleted by someone with the permission this session lacks), so the list is three, not five. The remaining three are still at exactly the recorded HEADs, and the proof was re-run and strengthened: for each, `tree(tip) == tree(merge-base)` byte-for-byte, so the branch nets zero content change from where it forked and nothing on it is absent from `main`. That is stronger than either check the guide names — `--cherry-pick` still reports 13/6/4 patch-unique commits on them, which is the squash-merge false positive, and none is an ancestor of `main`, so `--merged` would also miss them. A completed `branch-cleanup` row keyed to its own HEAD is now appended for each of the three (`npm run check:branch-review-ledger` passes at 1272 records), so the ORDER MATTERS constraint above is satisfied and the refs can now be deleted safely at any time. Deletion re-attempted and still blocked: `git push origin --delete` exits 1 with **HTTP 403** from the session git proxy (`recentRelayFailures` empty, so it is credential scope, not a relay fault), and a tool search confirms the GitHub MCP set exposes `create_branch` but no delete-branch capability. **Next:** delete these three refs from the GitHub UI or any session whose credentials permit ref deletion — `codex/document-reader-condensed-view`, `cursor/page-anchored-search-composer-30ee`, `cursor/pr-1379-babysit-ledger-9365`. Nothing else is required first. **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. **FIXED 2026-07-29:** `scripts/sweep-branch-ledger.mjs` now refuses outright on a shallow clone via the exported `shallowCloneRefusal`, printing no inventory and exiting 1 in both text and `--json` mode, before the fetch and before any branch maths. `docs/branch-cleanup-guide.md` §Safety Rules gains the `is-shallow-repository` precondition ahead of its numbered steps, because the raw `git` commands it documents have no such guard. Proven in a real `--depth 1` clone: unguarded the sweep exited **0** and named the live checked-out branch a deletion candidate with "no unique patch content"; guarded it exits 1 with the `--unshallow` remedy. Five cases in `tests/repo-hygiene.test.ts` cover both directions, including that the string `"false"` (truthy) must NOT be read as shallow — the way this guard could fail dangerously in reverse. **Hardened in review:** an indeterminate `is-shallow-repository` result (empty output from a swallowed `git` failure) is now refused as its own failure rather than treated as complete, and the same refusal was extended to `scripts/reconciliation-preflight.mjs`, which reports its own merge-base-derived ahead/behind. That guard then had to move OUT of the preflight CLI and INTO the exported `collectReconciliationState`, because `buildReconciliationEvidencePack` calls the collector directly and stamps `status: "complete"`: in a `--depth 1` clone the guarded CLI exited 1 while the evidence-pack command exited 0 and persisted shallow ahead/behind as completed evidence. The collector now throws `UnverifiedHistoryError` (`code: "history-not-verified"`), so every current and future caller fails closed by default instead of by remembering to ask; the CLI catches it only to keep the `--json` envelope. Regression cases live with each entry point (`tests/reconciliation-preflight.test.ts`, `tests/reconciliation-evidence-pack.test.ts`) and build a real `--depth 1` clone, asserting `is-shallow-repository` is `true` first so a git behaviour change cannot make them pass vacuously. **Second failure mode, found in review after the first fix landed: complete history is not complete branch coverage.** `git clone --depth 1` implies `--single-branch`, pinning `remote.origin.fetch` to the one cloned branch; `git fetch --unshallow` converts the history so `--is-shallow-repository` reads `false` and the shallow guard passes, but it does not widen the refspec, and an ordinary `git fetch origin` respects the narrow one. Measured in a `main`+`feature` fixture: after unshallowing, `git ls-remote --heads origin` listed both while `refs/remotes/origin` held only `origin/main`, and the sweep exited **0** reporting `"branches": []` — and an empty inventory is not a safe failure, since it reads as "nothing to clean up" and a missing `origin/main` makes every `rev-list` fail into `0/0`, i.e. every branch a deletion candidate. Fixed both ways: the sweep's fetch now passes an explicit `+refs/heads/*:refs/remotes/origin/*` (repairing coverage without rewriting the operator's config), and `branchCoverageRefusal` refuses when neither the configured refspec nor a completed wildcard fetch establishes coverage — `--no-fetch`, offline, or a failed fetch. Its remedy is deliberately `git remote set-branches origin '*'`, not `--unshallow`, which fixes history and does nothing here. **Two further routes to the same empty-inventory answer, both found in review, both from checking only half of the refspec.** (1) The DESTINATION matters as much as the source, because the sweep enumerates `refs/remotes/origin` and nothing else: with `+refs/heads/*:refs/remotes/upstream/*`, `refs/remotes/upstream` held `upstream/main` and `upstream/feature` while `refs/remotes/origin` stayed empty and the sweep exited **0** with `"branches": []`. (2) Git substitutes the matched suffix into ``, so a `refs/*` source nests one level deeper: `+refs/*:refs/remotes/origin/*` writes `refs/remotes/origin/heads/main`, `origin/main` then does not resolve at all, every comparison fails into `0/0`, and the sweep exited **0** naming both `heads/feature` and **`heads/main`** as deletion candidates — a green run recommending the deletion of `main`. Coverage from config therefore requires exactly `refs/heads/*` to `refs/remotes/origin/*`; a completed wildcard fetch still establishes coverage by itself, since the sweep passes that destination explicitly. **Stop:** never delete a branch, or report a branch as unmerged, from a shallow clone, a single-branch refspec, or a refspec whose destination is not `refs/remotes/origin/*`. | 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 | P2 | issue | `issues:next-id` has no concurrency protection | **Outcome:** two agents working the same hour cannot allocate the same ledger id. **Detail:** the marker at the top of this file is a plain HTML comment read-modify-written by whoever edits next, with no lock and no post-merge check. On 2026-07-29 it collided **twice in one hour**: PR #1391 claimed `#096`/`#097`, lost them, claimed `#098`/`#099`, lost those too, and its final pair `#108`/`#109` collided a third time with the branch-cleanup work that reached `main` first — resolved during the #1391 merge by renumbering to `#110`/`#111` and bumping the marker to 112. Each collision is silent: `docs/outstanding-issues.md` has NO union merge driver (unlike `docs/branch-review-ledger.md`), so it presents as an ordinary content conflict that a hurried resolution can settle by dropping one side's rows entirely. Nothing checks for duplicate ids afterwards. **Next:** add a duplicate-id and marker-consistency check to the verification gates — the cheapest useful form is a test asserting every `#NNN` id appears exactly once across both tables and that the marker exceeds the max, which turns a silent loss into a red gate. Consider`merge=union`in`.gitattributes` as well, though row-level union does not by itself prevent two rows sharing an id. **Stop:** do not resolve one of these conflicts by taking one side wholesale without diffing the id sets first; that is how rows get dropped. | session 2026-07-29 PR sweep; PR #1391 conflict resolution; `.gitattributes` | 2026-07-29 | +| #113 | P2 | issue | `ModeNav` clips its labels at every phone width on `main` | **Outcome:** the mode navigation bar honours its own "labels are never abbreviated" contract, or does not render. **Detail:** measured in Chromium against the running app on 2026-07-29 (PR #1390 review, landed as `8f861bb0`), `span.truncate` `scrollWidth` vs `clientWidth` on `/therapy-compass/search`: 320px viewport clips `Compare` to **15 of 56px**; 360px → 29px; 390px → 39px; 412px → 46px; **430px is the worst case at 3 of 4 labels clipped** (`Compare` 17px, `Recommend` 57 of 77, `Pathways` 57 of 60) because crossing the 26rem band adds a fourth slot to the same space; clean only from ~35rem. Root cause is `grid-auto-columns: 1fr` (`globals.css:2373`) in both lower bands — equal tracks make the WIDEST slot set what every slot needs — combined with `truncate`, so the shortfall is silent. The CSS block's stated budgets are wrong: it claims four labels need 394px, measured intrinsic widths are Search 92.4 / Compare 144.3 / Recommend 125.6 / Pathways 108.5 = **471px**, short by roughly the `0/4` badge plus its gap. **Next:** pick one — (a) content-size every band (adopt the ≥34rem `display: flex` shape throughout) and move thresholds to ~21rem for three slots and ~31rem for four; (b) drop the count badge below the top band, worth ~38px; (c) raise the thresholds so phones keep the collapsed control. (a) is the smallest change and deletes a layout mode. Add the band-boundary browser assertion review asked for, covering the widest label in the set. **Stop:** a hardcoded `rem` threshold cannot guarantee fit for an arbitrary item list, and `ModeNav` is shared — do not treat a Therapy-tuned number as a general solution. | PR #1390 thread `PRRT_kwDOSh5Fis6UyB5X` (open at merge); measured session 2026-07-29 | 2026-07-29 | +| #114 | P2 | issue | The live Web-Vitals instrument cannot measure its own noise | **Outcome:** the `#017` baseline rests on evidence whose reproducibility can be checked. **Detail:** `live-web-vitals.yml` (landed `8dbfc5d1`) runs Lighthouse **once** per route/strategy. `#017` asks for reproducible evidence and says to stop when the evidence is too noisy — one sample can neither establish reproducibility nor recognise noise, so the instrument cannot detect the condition its own governing item tells the operator to stop on. The rule is a hard threshold (LCP < 2500ms), so a route near the line resolves to a pass or a breach on run-to-run variance alone, invisibly, and a favourable sample would mark **seven** gated findings WONTFIX. Related and also unclosed: Chrome ships with the `ubuntu-24.04` runner image and is NOT pinned by `LIGHTHOUSE_VERSION`, so a metric shift between a baseline and its follow-up can originate in the browser; the build is now recorded per report in `summary.json` (`chromeVersions`) so a cross-version comparison is visibly invalid, but nothing prevents one. **Next:** N runs per route/strategy with a sample-indexed report name, `expectedRuns`/`incompleteEvidence` reworked to expect N per cell, median as the graded aggregate (the Lighthouse and `lighthouse-ci` convention), and — most important — a spread that straddles a threshold treated as INCOMPLETE EVIDENCE rather than resolved either way. Costs N× dispatch time. Pinning Chrome needs a container or a setup action. **Stop:** do not record an `#017` verdict from a single-sample run, and do not compare baselines whose `chromeVersions` differ. | PR #1385 thread `PRRT_kwDOSh5Fis6U0Zq0` (open at merge); `scripts/summarise-web-vitals.mjs` | 2026-07-29 | +| #115 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` no longer asks "does this file mention the band?" but "does anything the route actually mounts reach it?". It parses each module with `@babel/parser` into a small graph — exported name to local declaration, local to the identifiers its body references, and which locals render the band — then walks from the route's default export, carrying at each hop the set of exports the importer mounts. So a static `import { X }` is followed only when `X` is reachable from a mounted declaration; `dynamic(() => import("…").then((m) => m.Named))` follows only that binding, which is how the code-split dashboard workspaces are written; a bare `import "…"` is not followed at all; `export { X } from "…"` is followed only when the importer wants `X`; and `export * from "…"` never supplies a default, so a page whose importer wants only the default gets no hop from it. **Why the redesign rather than more patches:** six false greens were reported in one day (unrendered import, `export { X }`, `export { X } from`, `export *`, bare side-effect import, JSX in an unmounted helper, and a lazy import reaching every sibling export), all one defect — presence is not reach. Two of the six were introduced by an earlier patch to the same walker. **Verified:** all five production search routes still reach the band; gutting `(search-app)/services/page.tsx` and `tools/page.tsx` to `
` each reports an orphan; fourteen temp-dir fixtures cover both directions, and the two guarding the new mechanisms were confirmed to fail against the prior behaviour by targeted mutation (presence-based band check, and following bare imports). Residual: reachability is per module, so a mounted declaration referencing an identifier anywhere in its body counts, and control flow inside it is not modelled. | PR #1400; session 2026-07-30 | 2026-07-30 | +| #116 | P2 | issue | An unmergeable PR runs no CI at all, with no signal that it stopped | **Outcome:** a PR that has silently stopped being tested says so. **Detail:** on 2026-07-30 PR #1400 ran **no** `CI`, `Gitleaks` or `Semgrep` workflow across three consecutive pushes, and nothing anywhere said why. Cause: the PR had a real content conflict with `main`, so GitHub could not build `refs/pull/1400/merge`, and every `pull_request`-triggered workflow is skipped in that state. `pull_request_target` ones (`PR Policy`) still ran, and CircleCI posted failure ~3s after each push - faster than a checkout, so no step executed. The symptom reads as "CI is broken" or "my tests fail", and about 40 minutes went into replicating CI steps locally (all green) before the cause was found. **Diagnostic:** compare check-run counts with another open PR (3 vs 16-19), list workflow runs and note only `pull_request_target` fired, then confirm with `git merge-tree --write-tree origin/main HEAD`. **Next:** make staleness visible instead of silent - e.g. a `pull_request_target` job that fails when `mergeable_state` is `dirty`, so a conflicted PR shows one red check naming the conflict rather than an empty check list. `npm run sync:pr-branches` covers the behind-but-clean case only. **Stop:** never conclude "CI is failing" from a missing check - the absence of a check is not a failing check. | PR #1400; session 2026-07-30 | 2026-07-30 | ## Resolved / archive diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index 0d749a64b3..4e17ed4701 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -207,10 +207,21 @@ function resolvePrettierBin() { * while a changed-paths-only check passes — so a policy change escalates to a * whole-tree check. */ -function isPrettierPolicyFile(file) { - return /^(?:\.prettierrc(?:\..+)?|prettier\.config\.(?:js|cjs|mjs|ts)|\.prettierignore|\.editorconfig|package\.json)$/.test( - path.basename(file), - ); +function isPrettierPolicyFile(file, checkoutDir) { + const base = path.basename(file); + if (/^(?:\.prettierrc(?:\..+)?|prettier\.config\.(?:js|cjs|mjs|ts)|\.prettierignore|\.editorconfig)$/.test(base)) { + return true; + } + // A package.json is policy only when it actually carries a `prettier` field. + // Matching every package.json would send each routine dependency bump through a + // whole-tree check it cannot possibly need. + if (base !== "package.json") return false; + try { + return JSON.parse(readFileSync(path.join(checkoutDir, file), "utf8")).prettier !== undefined; + } catch { + // Unreadable or unparseable: assume it is policy rather than assume it is not. + return true; + } } /** @@ -253,7 +264,7 @@ function checkPushedCommit(prettierBin, sha, files) { // Already present, or symlinks unavailable — prettier still loads static configs. } } - const policyChanged = files.some(isPrettierPolicyFile); + const policyChanged = files.some((file) => isPrettierPolicyFile(file, dir)); // Deleted paths are gone from the checkout, and prettier errors on a missing // argument, so ask only for what is actually there. const present = files.filter((file) => existsSync(path.join(dir, file))); diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index feff669da8..e6ec17b546 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -159,34 +159,70 @@ function reachabilityRoots(routeAbs: string): string[] { } /** - * Specifiers worth following out of one file, and only those. + * A module reduced to what decides whether it can render the band. * - * A plain "every specifier in the file" walk reports adoption for a route that - * keeps its results import but stops rendering it — reduce - * `(search-app)/services/page.tsx` to `
` with its imports intact and the - * gate stayed green, which is the regression it exists to catch. So a static - * `import { X } from "…"` is followed only when `X` is actually mounted: - * rendered as JSX, or re-exported as the default (a wrapper page mounts with no - * JSX of its own). A named re-export — `export { X }` — is *not* a mount: it - * renders nothing, so counting it let a page re-export a banded component while - * its own default rendered `
`. Wherever that re-export is finally - * mounted, this walk sees the JSX there instead. - * - * Followed unconditionally, because each *is* a mount mechanism rather than a - * binding that might go unused: - * - `import("…")` — how `clinical-dashboard-lazy.tsx` code-splits the mode - * workspaces, so the band behind Differentials/Favourites/prescribing is only - * reachable this way - * - `export { default } from "…"` / `export { X as default } from "…"` — a - * pass-through page whose default component comes from elsewhere - * - side-effect `import "…"` - * - * `export { X } from "…"` and `export * from "…"` are *not* followed. They are - * the one-statement spelling of the named re-export above and carry the same - * false green: a page can re-export a banded component while its own default - * renders `
`. + * Presence is not reach. Five separate false greens on this gate were all the + * same defect — the walk asked "does this file mention it?" when the question is + * "does anything the route actually mounts reach it?". Every spelling that got + * patched individually (an unrendered import, `export { X }`, `export { X } from`, + * `export *`, a bare side-effect import, JSX inside an unmounted helper) is a + * reachability question, so this models reachability once instead. */ -function followableSpecifiers(source: string, filename: string): string[] { +type ModuleGraph = { + /** exported name (including "default") -> the local declarations behind it */ + exportedLocals: Map; + /** local declaration -> identifiers its body references, JSX element names included */ + localRefs: Map>; + /** locals whose own body renders the band */ + bandLocals: Set; + /** `import { imported as local } from source` */ + imports: Array<{ source: string; local: string; imported: string }>; + /** `export { imported as exported } from source`; `exported: "*"` for `export *` */ + reexports: Array<{ source: string; exported: string; imported: string }>; + /** `import(source)[.then(m => m.imported)]`, attributed to the local that owns it */ + lazyImports: Array<{ owner: string; source: string; imported: string }>; +}; + +/** Which exports of a module the importer actually mounts. `"*"` means any. */ +type MountRoots = Set | "*"; + +/** `export default () => …` has no name to hang references off. */ +const ANONYMOUS_DEFAULT = "__default__"; + +function declaredNames(node: Record | undefined): string[] { + if (!node) return []; + if (node.type === "FunctionDeclaration" || node.type === "ClassDeclaration") { + const name = (node.id as { name?: string })?.name; + return name ? [name] : []; + } + if (node.type === "VariableDeclaration") { + return ((node.declarations ?? []) as Array>) + .map((declarator) => (declarator.id as { name?: string })?.name) + .filter((name): name is string => typeof name === "string"); + } + return []; +} + +/** `import("x").then(m => m.Foo)` -> "Foo"; anything else -> null. */ +function thenExportName(args: unknown): string | null { + const first = ((args ?? []) as Array>)[0]; + const body = first?.body as Record | undefined; + if (body?.type !== "MemberExpression") return null; + const property = body.property as { name?: string } | undefined; + return typeof property?.name === "string" ? property.name : null; +} + +function dynamicImportSource(node: Record | undefined): string | null { + if (!node) return null; + const isImport = + node.type === "ImportExpression" || + (node.type === "CallExpression" && (node.callee as { type?: string })?.type === "Import"); + if (!isImport) return null; + const arg = (node.source ?? (node.arguments as unknown[])?.[0] ?? null) as Record | null; + return arg?.type === "StringLiteral" && typeof arg.value === "string" ? arg.value : null; +} + +function buildModuleGraph(source: string, filename: string): ModuleGraph { let ast: ReturnType; try { ast = parse(source, { sourceType: "unambiguous", plugins: ["jsx", "typescript"] }); @@ -195,82 +231,204 @@ function followableSpecifiers(source: string, filename: string): string[] { throw new Error(`search-results-band-adoption: could not parse ${filename}`); } - const body = (ast.program?.body ?? []) as unknown[]; - const mounted = new Set(); - const staticImports: Array<{ source: string; locals: string[] }> = []; - const always: string[] = []; + const graph: ModuleGraph = { + exportedLocals: new Map(), + localRefs: new Map(), + bandLocals: new Set(), + imports: [], + reexports: [], + lazyImports: [], + }; + const addExport = (exported: string, local: string) => { + graph.exportedLocals.set(exported, [...(graph.exportedLocals.get(exported) ?? []), local]); + }; - const visit = (value: unknown) => { - if (!value || typeof value !== "object") return; - if (Array.isArray(value)) { - for (const item of value) visit(item); + /** Record everything one local declaration's body reaches. */ + const scan = (root: unknown, owner: string) => { + const refs = graph.localRefs.get(owner) ?? new Set(); + graph.localRefs.set(owner, refs); + const visit = (value: unknown) => { + if (!value || typeof value !== "object") return; + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + const node = value as Record; + const type = node.type; + if (type === "JSXOpeningElement") { + let name = node.name as Record | undefined; + while (name && name.type === "JSXMemberExpression") name = name.object as Record; + if (name && name.type === "JSXIdentifier" && typeof name.name === "string") { + refs.add(name.name); + if (name.name === BAND_IDENTIFIER) graph.bandLocals.add(owner); + } + } + if (type === "Identifier" && typeof node.name === "string") refs.add(node.name); + // `dynamic(() => import("x").then(m => m.Foo))` names the binding it mounts, + // which is how every lazy workspace in clinical-dashboard-lazy.tsx is written. + // Matched on the outer `.then(…)` so the inner bare-import branch below can + // skip it and not also enqueue the module's default. + if (type === "CallExpression") { + const callee = node.callee as Record | undefined; + if (callee?.type === "MemberExpression" && (callee.property as { name?: string })?.name === "then") { + const inner = dynamicImportSource(callee.object as Record | undefined); + if (inner) { + graph.lazyImports.push({ owner, source: inner, imported: thenExportName(node.arguments) ?? "*" }); + } + } + } + const bare = dynamicImportSource(node); + if (bare && !graph.lazyImports.some((entry) => entry.owner === owner && entry.source === bare)) { + // A plain `import("x")` in `dynamic()` mounts the module's default export. + graph.lazyImports.push({ owner, source: bare, imported: "default" }); + } + for (const key of Object.keys(node)) { + if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; + visit(node[key]); + } + }; + visit(root); + }; + + /** Attribute a declaration's references to each name it declares. */ + const scanDeclaration = (statement: Record) => { + if (statement.type === "VariableDeclaration") { + for (const declarator of (statement.declarations ?? []) as Array>) { + const name = (declarator.id as { name?: string })?.name; + if (name) scan(declarator, name); + } return; } - const node = value as Record; - const type = node.type; - - // and — the element name is a mount. - if (type === "JSXOpeningElement") { - let name = node.name as Record | undefined; - while (name && name.type === "JSXMemberExpression") name = name.object as Record; - if (name && name.type === "JSXIdentifier" && typeof name.name === "string") mounted.add(name.name); - } - // `export default X` mounts X without any JSX in this file. - if (type === "ExportDefaultDeclaration") { - const decl = node.declaration as Record | undefined; - if (decl && decl.type === "Identifier" && typeof decl.name === "string") mounted.add(decl.name); + for (const name of declaredNames(statement)) scan(statement, name); + }; + + for (const raw of (ast.program?.body ?? []) as unknown[] as Array>) { + const type = raw.type; + + if (type === "ImportDeclaration") { + const specifier = (raw.source as { value?: string })?.value; + if (typeof specifier !== "string") continue; + // A bare `import "x"` executes the module but renders nothing, so it is not + // a mount and is deliberately not followed. + for (const spec of (raw.specifiers ?? []) as Array>) { + const local = (spec.local as { name?: string })?.name; + if (!local) continue; + const imported = + spec.type === "ImportDefaultSpecifier" + ? "default" + : spec.type === "ImportNamespaceSpecifier" + ? "*" + : ((spec.imported as { name?: string })?.name ?? local); + graph.imports.push({ source: specifier, local, imported }); + } + continue; } - // Deliberately NOT a mount: `export { X }` for an imported `X`. Re-exporting - // a binding renders nothing, so a page that re-exports a banded component - // while its own default renders `
` would otherwise report adoption. - // A consumer that mounts the re-export is caught where it mounts it. - // `import("…")`, including inside dynamic(() => import("…")). - if ( - type === "ImportExpression" || - (type === "CallExpression" && (node.callee as { type?: string })?.type === "Import") - ) { - const arg = (node.source ?? (node.arguments as unknown[])?.[0] ?? null) as Record | null; - if (arg && arg.type === "StringLiteral" && typeof arg.value === "string") always.push(arg.value); + + if (type === "ExportAllDeclaration") { + const specifier = (raw.source as { value?: string })?.value; + if (typeof specifier === "string") graph.reexports.push({ source: specifier, exported: "*", imported: "*" }); + continue; } - for (const key of Object.keys(node)) { - if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; - visit(node[key]); + + if (type === "ExportNamedDeclaration") { + const specifier = (raw.source as { value?: string })?.value; + if (typeof specifier === "string") { + for (const spec of (raw.specifiers ?? []) as Array>) { + const exported = (spec.exported as { name?: string })?.name; + const imported = (spec.local as { name?: string })?.name ?? exported; + if (exported && imported) graph.reexports.push({ source: specifier, exported, imported }); + } + continue; + } + const declaration = raw.declaration as Record | undefined; + if (declaration) { + for (const name of declaredNames(declaration)) addExport(name, name); + scanDeclaration(declaration); + continue; + } + for (const spec of (raw.specifiers ?? []) as Array>) { + const exported = (spec.exported as { name?: string })?.name; + const local = (spec.local as { name?: string })?.name; + if (exported && local) addExport(exported, local); + } + continue; } - }; - for (const raw of body) { - const statement = raw as Record; - if (statement.type === "ImportDeclaration") { - const spec = (statement.source as { value?: string })?.value; - if (typeof spec !== "string") continue; - const locals = ((statement.specifiers ?? []) as Array>) - .map((s) => (s.local as { name?: string })?.name) - .filter((n): n is string => typeof n === "string"); - // A bare `import "x"` has no bindings and is a side effect — always follow. - if (locals.length === 0) always.push(spec); - else staticImports.push({ source: spec, locals }); + if (type === "ExportDefaultDeclaration") { + const declaration = raw.declaration as Record | undefined; + if (declaration?.type === "Identifier" && typeof declaration.name === "string") { + addExport("default", declaration.name); + continue; + } + const owner = declaredNames(declaration)[0] ?? ANONYMOUS_DEFAULT; + addExport("default", owner); + if (declaration) scan(declaration, owner); continue; } - // `export { default } from "…"` / `export { X as default } from "…"` supplies - // this module's default component, so it mounts. A named or star re-export - // does not, and following it is the same false green as `export { X }` above. - if (statement.type === "ExportNamedDeclaration" && statement.source) { - const spec = (statement.source as { value?: string })?.value; - const suppliesDefault = ((statement.specifiers ?? []) as Array>).some( - (s) => (s.exported as { name?: string })?.name === "default", - ); - if (typeof spec === "string" && suppliesDefault) always.push(spec); + + scanDeclaration(raw); + } + + return graph; +} + +/** Locals reachable from the exports the importer mounts. */ +function reachableLocals(graph: ModuleGraph, roots: MountRoots): Set { + const queue: string[] = + roots === "*" + ? [...graph.exportedLocals.values()].flat() + : [...roots].flatMap((name) => graph.exportedLocals.get(name) ?? []); + const reached = new Set(); + while (queue.length > 0) { + const local = queue.pop() as string; + if (reached.has(local)) continue; + reached.add(local); + for (const ref of graph.localRefs.get(local) ?? []) if (!reached.has(ref)) queue.push(ref); + } + return reached; +} + +/** + * Modules the mounted code actually reaches, and which of their exports it wants. + * + * Re-exports are followed by their own semantics rather than by a special case: + * `export { X as default } from "…"` supplies a default and is followed when the + * importer wants the default; `export { X } from "…"` only when it wants `X`; and + * `export * from "…"` never supplies a default, so a page whose importer wants + * only the default gets no hop from it. + */ +function nextHops(graph: ModuleGraph, reached: Set, roots: MountRoots) { + const hops = new Map(); + const want = (source: string, imported: string | string[]) => { + const existing = hops.get(source); + if (existing === "*") return; + if (imported === "*") { + hops.set(source, "*"); + return; + } + const names = Array.isArray(imported) ? imported : [imported]; + hops.set(source, new Set([...(existing ?? []), ...names])); + }; + + for (const { source, local, imported } of graph.imports) if (reached.has(local)) want(source, imported); + for (const { owner, source, imported } of graph.lazyImports) if (reached.has(owner)) want(source, imported); + for (const { source, exported, imported } of graph.reexports) { + if (exported === "*") { + // `export *` re-exports named exports only, never the default. + if (roots === "*") want(source, "*"); + else { + const named = [...roots].filter((name) => name !== "default"); + if (named.length > 0) want(source, named); + } continue; } + if (roots === "*" || roots.has(exported)) want(source, imported); } - // Mount detection has to see the whole program, imports included, because a - // re-export statement is both. - visit(body); - - const followed = staticImports - .filter(({ locals }) => locals.some((local) => mounted.has(local))) - .map((i) => i.source); - return [...new Set([...always, ...followed])]; + return [...hops.entries()].map(([source, wanted]) => ({ source, roots: wanted })); +} + +function rootsKey(roots: MountRoots): string { + return roots === "*" ? "*" : [...roots].sort().join(","); } function resolveSpecifier(specifier: string, fromFile: string): string | null { @@ -289,30 +447,48 @@ function resolveSpecifier(specifier: string, fromFile: string): string | null { * 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. + * reachability rather than failing loudly. The lazy indirection through + * `clinical-dashboard-lazy` adds a hop, so the cap has headroom over the longest + * real chain rather than sitting on it. */ -const MAX_IMPORT_DEPTH = 8; +const MAX_IMPORT_DEPTH = 10; +/** + * Breadth-first from what a route actually mounts to a band it actually renders. + * + * Each hop carries the exports the importer wants, so the same module can be + * visited twice for different bindings — that is the point. `clinical-dashboard-lazy` + * lazily exports every mode workspace, and reaching it for `FavouritesHub` must + * not also reach `document-search-results`. + */ function routeReachesBand(routeAbs: string): boolean { - const roots = reachabilityRoots(routeAbs); - const seen = new Set(roots); - let frontier = roots.map((file) => ({ file, depth: 0 })); + // Next renders a route's (and a layout's) default export. + let frontier = reachabilityRoots(routeAbs).map((file) => ({ + file, + depth: 0, + roots: new Set(["default"]) as MountRoots, + })); + const seen = new Set(frontier.map(({ file, roots }) => `${file}::${rootsKey(roots)}`)); while (frontier.length > 0) { - const next: Array<{ file: string; depth: number }> = []; - for (const { file, depth } of frontier) { + const next: typeof frontier = []; + for (const { file, depth, roots } of frontier) { let source: string; try { source = readFileSync(file, "utf8"); } catch { continue; } - if (rendersBand(source)) return true; + const graph = buildModuleGraph(source, file); + const reached = reachableLocals(graph, roots); + for (const local of graph.bandLocals) if (reached.has(local)) return true; if (depth >= MAX_IMPORT_DEPTH) continue; - for (const specifier of followableSpecifiers(source, file)) { - const target = resolveSpecifier(specifier, file); - if (!target || seen.has(target) || isMockupPath(target)) continue; - seen.add(target); - next.push({ file: target, depth: depth + 1 }); + for (const hop of nextHops(graph, reached, roots)) { + const target = resolveSpecifier(hop.source, file); + if (!target || isMockupPath(target)) continue; + const key = `${target}::${rootsKey(hop.roots)}`; + if (seen.has(key)) continue; + seen.add(key); + next.push({ file: target, depth: depth + 1, roots: hop.roots }); } } frontier = next; @@ -489,8 +665,9 @@ describe("band adoption detection", () => { ); 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. + // Same shape, but the child mounts the band — and via a lazy import, in the + // exact form the dashboard code-splits its mode workspaces with: + // `dynamic(() => import("…").then((m) => m.Named))`. writeFileSync( path.join(dir, "banded-child.tsx"), `export function Banded() { return <${BAND_IDENTIFIER} modeId="documents" />; }\n`, @@ -498,11 +675,102 @@ describe("band adoption detection", () => { ); writeFileSync( path.join(dir, "wired-route.tsx"), - 'const Lazy = () => import("./banded-child");\nexport default Lazy;\n', + 'const Lazy = dynamic(() => import("./banded-child").then((m) => m.Banded));\nexport default Lazy;\n', "utf8", ); expect(routeReachesBand(path.join(dir, "wired-route.tsx"))).toBe(true); + // A bare `dynamic(() => import("…"))` mounts the module's default, so this + // one reaches the band and the named-export module above would not have. + writeFileSync( + path.join(dir, "default-banded-child.tsx"), + `export default function Banded() { return <${BAND_IDENTIFIER} modeId="documents" />; }\n`, + "utf8", + ); + writeFileSync( + path.join(dir, "lazy-default-route.tsx"), + 'const Lazy = dynamic(() => import("./default-banded-child"));\nexport default Lazy;\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "lazy-default-route.tsx"))).toBe(true); + + // The gap this redesign closes: one lazy module exporting several + // workspaces. Mounting the non-band one must not reach the band one. + writeFileSync( + path.join(dir, "lazy-barrel.tsx"), + 'export const Plain = dynamic(() => import("./plain-child").then((m) => m.Plain));\n' + + 'export const Banded = dynamic(() => import("./banded-child").then((m) => m.Banded));\n', + "utf8", + ); + writeFileSync(path.join(dir, "plain-child.tsx"), "export function Plain() { return
; }\n", "utf8"); + writeFileSync( + path.join(dir, "mounts-plain-only-route.tsx"), + 'import { Plain } from "./lazy-barrel";\nexport default function Page() {\n return ;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "mounts-plain-only-route.tsx"))).toBe(false); + + // ...and mounting the band one still does reach it. + writeFileSync( + path.join(dir, "mounts-banded-route.tsx"), + 'import { Banded } from "./lazy-barrel";\nexport default function Page() {\n return ;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "mounts-banded-route.tsx"))).toBe(true); + + // The band rendered directly inside an exported helper of a module the route + // *does* reach, but never mounts. This is the case the reachability-scoped + // band check owns: hop filtering cannot catch it, because the module is + // legitimately reached for its other export. + writeFileSync( + path.join(dir, "mixed-exports.tsx"), + "export function Plain() {\n return
;\n}\n" + + `export function Helper() {\n return <${BAND_IDENTIFIER} modeId="documents" />;\n}\n`, + "utf8", + ); + writeFileSync( + path.join(dir, "mounts-plain-export-route.tsx"), + 'import { Plain } from "./mixed-exports";\nexport default function Page() {\n return ;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "mounts-plain-export-route.tsx"))).toBe(false); + + // Same module, mounting the export that does render it. + writeFileSync( + path.join(dir, "mounts-helper-export-route.tsx"), + 'import { Helper } from "./mixed-exports";\nexport default function Page() {\n return ;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "mounts-helper-export-route.tsx"))).toBe(true); + + // JSX inside an exported helper the route never renders is not a mount. + writeFileSync( + path.join(dir, "unmounted-helper-route.tsx"), + 'import { Banded } from "./banded-child";\n' + + "export function Helper() {\n return ;\n}\n" + + "export default function Page() {\n return
;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "unmounted-helper-route.tsx"))).toBe(false); + + // A bare side-effect import executes the module but renders nothing. + writeFileSync( + path.join(dir, "side-effect-route.tsx"), + 'import "./banded-child";\nexport default function Page() {\n return
;\n}\n', + "utf8", + ); + expect(routeReachesBand(path.join(dir, "side-effect-route.tsx"))).toBe(false); + + // A non-exported local the mounted component does render is reachable. + writeFileSync( + path.join(dir, "indirect-route.tsx"), + 'import { Banded } from "./banded-child";\n' + + "function Inner() {\n return ;\n}\n" + + "export default function Page() {\n return ;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "indirect-route.tsx"))).toBe(true); + // The regression this gate exists to catch: the results component is still // imported, but nothing renders it. A walk that follows every specifier // reports adoption here, which is how gutting a real page to `
` with From 7c5c613efd8e9964bd0f37b4b197896a1137c84d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 05:05:01 +0000 Subject: [PATCH 09/11] fix(test,guard): handle the object-wrapper lazy shape, discarded imports, and policy removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on 0ce9f5e6, all valid: - `then((mod) => ({ default: mod.Foo }))` — the Next.js wrapper for a named export, which global-search-shell.tsx uses for ClinicalDashboard on the root dashboard chain. thenExportName returned null for it, so the walk fell back to following every export of that module: the exact over-approximation the redesign exists to remove. - A dynamic import whose result is discarded (`void import(…)` in an effect, or a bare `import(…);` statement) is a preload and cannot mount anything. Scoped to discarded *results* rather than requiring lexical containment in `dynamic()`, because clinical-dashboard-lazy.tsx also writes the loader as a separate binding and requiring containment would report that unreachable. - guard-push inspected only the pushed package.json for a `prettier` field, so *adding* one escalated to a whole-tree check but *removing* one did not — drop `tabWidth: 4` and four-space-formatted source starts failing CI. Both endpoints are now inspected via git rather than the checkout. Each fix verified by mutation: unhandling the object wrapper makes the wrapper-plain fixture reach the band; recording discarded imports makes the preload fixture reach it. Both fail as they should. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- scripts/guard-push.mjs | 36 ++++++--- tests/search-results-band-adoption.test.ts | 94 +++++++++++++++++++--- 2 files changed, 110 insertions(+), 20 deletions(-) diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index 4e17ed4701..c4dd75f550 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -207,21 +207,35 @@ function resolvePrettierBin() { * while a changed-paths-only check passes — so a policy change escalates to a * whole-tree check. */ -function isPrettierPolicyFile(file, checkoutDir) { +/** Does `:` parse as JSON carrying a top-level `prettier` field? */ +function carriesPrettierField(ref, file) { + const contents = tryGit(["show", `${ref}:${file}`]); + if (contents === undefined) return false; // absent at this ref (or no such ref) + try { + return JSON.parse(contents).prettier !== undefined; + } catch { + // Unparseable: assume it is policy rather than assume it is not. + return true; + } +} + +/** + * Does this path decide Prettier's verdict for files other than itself? + * + * A package.json counts only when it actually carries a `prettier` field — + * matching every package.json would send each routine dependency bump through a + * whole-tree check it cannot possibly need. **Both endpoints are inspected**, not + * just the pushed one: adding a field and removing one each re-decide the verdict + * for untouched files, and reading only the pushed side misses the removal (drop + * `tabWidth: 4` and four-space-formatted source starts failing CI). + */ +function isPrettierPolicyFile(file, sha) { const base = path.basename(file); if (/^(?:\.prettierrc(?:\..+)?|prettier\.config\.(?:js|cjs|mjs|ts)|\.prettierignore|\.editorconfig)$/.test(base)) { return true; } - // A package.json is policy only when it actually carries a `prettier` field. - // Matching every package.json would send each routine dependency bump through a - // whole-tree check it cannot possibly need. if (base !== "package.json") return false; - try { - return JSON.parse(readFileSync(path.join(checkoutDir, file), "utf8")).prettier !== undefined; - } catch { - // Unreadable or unparseable: assume it is policy rather than assume it is not. - return true; - } + return carriesPrettierField(sha, file) || carriesPrettierField(`${sha}^`, file); } /** @@ -264,7 +278,7 @@ function checkPushedCommit(prettierBin, sha, files) { // Already present, or symlinks unavailable — prettier still loads static configs. } } - const policyChanged = files.some((file) => isPrettierPolicyFile(file, dir)); + const policyChanged = files.some((file) => isPrettierPolicyFile(file, sha)); // Deleted paths are gone from the checkout, and prettier errors on a missing // argument, so ask only for what is actually there. const present = files.filter((file) => existsSync(path.join(dir, file))); diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index e6ec17b546..a3d23ee3bb 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -203,13 +203,57 @@ function declaredNames(node: Record | undefined): string[] { return []; } -/** `import("x").then(m => m.Foo)` -> "Foo"; anything else -> null. */ +/** + * Which export a `.then(…)` selects out of a dynamic import. + * + * Two shapes, both live in this repo: + * - `.then((m) => m.Foo)` -> "Foo" (every entry in clinical-dashboard-lazy.tsx) + * - `.then((mod) => ({ default: mod.Foo }))` -> "Foo" — the Next.js wrapper for a + * named export, used by `global-search-shell.tsx` for `ClinicalDashboard`. Left + * unhandled this returned null and fell back to following *every* export of that + * module, which is precisely the over-approximation the walk is meant to avoid. + */ function thenExportName(args: unknown): string | null { const first = ((args ?? []) as Array>)[0]; const body = first?.body as Record | undefined; - if (body?.type !== "MemberExpression") return null; - const property = body.property as { name?: string } | undefined; - return typeof property?.name === "string" ? property.name : null; + if (body?.type === "MemberExpression") { + const property = body.property as { name?: string } | undefined; + return typeof property?.name === "string" ? property.name : null; + } + if (body?.type === "ObjectExpression") { + for (const property of (body.properties ?? []) as Array>) { + if ((property.key as { name?: string })?.name !== "default") continue; + const value = property.value as Record | undefined; + if (value?.type === "MemberExpression") { + const selected = value.property as { name?: string } | undefined; + return typeof selected?.name === "string" ? selected.name : null; + } + } + } + return null; +} + +/** + * A dynamic import whose result is thrown away cannot mount anything. + * + * `void import("x")` in an effect, or a bare `import("x");` statement, is a + * preload or a side effect. Scoped to discarded *results* rather than to "must be + * lexically inside dynamic()", because this repo also writes the loader as a + * separate binding — `const load = () => import("…").then(…)` passed to + * `dynamic(load)` — and requiring lexical containment would report that as + * unreachable. + */ +function discardsItsResult(node: Record): boolean { + if (node.type === "UnaryExpression" && node.operator === "void") return true; + if (node.type !== "ExpressionStatement") return false; + let expression = node.expression as Record | undefined; + // Unwrap a `.then(…)`/`.catch(…)` chain back to its root. + while (expression?.type === "CallExpression") { + const callee = expression.callee as Record | undefined; + if (callee?.type !== "MemberExpression") break; + expression = callee.object as Record | undefined; + } + return Boolean(expression && dynamicImportSource(expression)); } function dynamicImportSource(node: Record | undefined): string | null { @@ -247,14 +291,15 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { const scan = (root: unknown, owner: string) => { const refs = graph.localRefs.get(owner) ?? new Set(); graph.localRefs.set(owner, refs); - const visit = (value: unknown) => { + const visit = (value: unknown, discarded = false) => { if (!value || typeof value !== "object") return; if (Array.isArray(value)) { - for (const item of value) visit(item); + for (const item of value) visit(item, discarded); return; } const node = value as Record; const type = node.type; + const resultDropped = discarded || discardsItsResult(node); if (type === "JSXOpeningElement") { let name = node.name as Record | undefined; while (name && name.type === "JSXMemberExpression") name = name.object as Record; @@ -268,7 +313,7 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { // which is how every lazy workspace in clinical-dashboard-lazy.tsx is written. // Matched on the outer `.then(…)` so the inner bare-import branch below can // skip it and not also enqueue the module's default. - if (type === "CallExpression") { + if (type === "CallExpression" && !resultDropped) { const callee = node.callee as Record | undefined; if (callee?.type === "MemberExpression" && (callee.property as { name?: string })?.name === "then") { const inner = dynamicImportSource(callee.object as Record | undefined); @@ -277,14 +322,14 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { } } } - const bare = dynamicImportSource(node); + const bare = resultDropped ? null : dynamicImportSource(node); if (bare && !graph.lazyImports.some((entry) => entry.owner === owner && entry.source === bare)) { // A plain `import("x")` in `dynamic()` mounts the module's default export. graph.lazyImports.push({ owner, source: bare, imported: "default" }); } for (const key of Object.keys(node)) { if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; - visit(node[key]); + visit(node[key], resultDropped); } }; visit(root); @@ -761,6 +806,37 @@ describe("band adoption detection", () => { ); expect(routeReachesBand(path.join(dir, "side-effect-route.tsx"))).toBe(false); + // The Next.js object-wrapper shape, which global-search-shell.tsx uses for + // ClinicalDashboard. Selecting the plain export must not drag in the sibling + // that renders the band; before this was handled the wrapper returned no name + // and the walk fell back to following every export. + writeFileSync( + path.join(dir, "wrapper-plain-route.tsx"), + 'const Lazy = dynamic(() => import("./lazy-barrel").then((mod) => ({ default: mod.Plain })));\n' + + "export default Lazy;\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "wrapper-plain-route.tsx"))).toBe(false); + + // Same wrapper selecting the banded export does reach it. + writeFileSync( + path.join(dir, "wrapper-banded-route.tsx"), + 'const Lazy = dynamic(() => import("./mixed-exports").then((mod) => ({ default: mod.Helper })));\n' + + "export default Lazy;\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "wrapper-banded-route.tsx"))).toBe(true); + + // A dynamic import whose result is discarded is a preload, not a mount. + writeFileSync( + path.join(dir, "preload-route.tsx"), + "export default function Page() {\n" + + ' useEffect(() => {\n void import("./default-banded-child");\n }, []);\n' + + " return
;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "preload-route.tsx"))).toBe(false); + // A non-exported local the mounted component does render is reachable. writeFileSync( path.join(dir, "indirect-route.tsx"), From 0258f336199df6970c56e6925f3645825fa8257e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 05:16:56 +0000 Subject: [PATCH 10/11] fix(guard,test): pushed-lockfile Prettier parity, and skip type-only imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two further review findings: - The guard formats with this checkout's Prettier while CI installs the pushed lockfile, so a push that bumps Prettier itself would be judged by the wrong version. When the push touches package.json or package-lock.json, the pinned and installed versions are compared and a difference blocks with an actionable `npm ci` message. Verified: a push changing the lockfile to a different pin blocks; a matching pin passes; and — the non-regression that matters — a mismatched lockfile the push does not touch still passes, so ordinary pushes are unaffected by node_modules drift. - `import type { Banded } from "…"` is erased at runtime and cannot mount anything, but was recorded as a component edge, so a route with a type-only prop reached the band. Both spellings are skipped (declaration-level `importKind` and the per-specifier one from `import { type X }`), plus `export type { X } from`. Pinned by a fixture that fails without the skip. This closes the last open Codex finding on the branch. Its own fixes for it exist only as unreachable commits on a branch named `work` — seven SHAs, none fetchable — so they could not be used. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- scripts/guard-push.mjs | 33 ++++++++++++++++++++++ tests/search-results-band-adoption.test.ts | 19 +++++++++++++ 2 files changed, 52 insertions(+) diff --git a/scripts/guard-push.mjs b/scripts/guard-push.mjs index c4dd75f550..ae224ba95d 100644 --- a/scripts/guard-push.mjs +++ b/scripts/guard-push.mjs @@ -269,6 +269,14 @@ function checkPushedCommit(prettierBin, sha, files) { }; } try { + // The Prettier doing the checking is this checkout's, not the pushed + // lockfile's. That only matters when the push changes dependencies — then a + // version difference can make CI disagree with this verdict, so say so rather + // than answer confidently with the wrong Prettier. + if (files.some((file) => ["package.json", "package-lock.json"].includes(path.basename(file)))) { + const mismatch = prettierVersionMismatch(prettierBin, dir); + if (mismatch) return { verdict: "error", detail: mismatch }; + } // A dynamic config may import plugins; without this it cannot load at all. const modules = path.resolve("node_modules"); if (existsSync(modules)) { @@ -303,6 +311,31 @@ function chunk(items, size) { return out; } +/** + * Does the pushed lockfile pin a different Prettier than the one installed? + * + * The guard formats with this checkout's Prettier while CI installs the pushed + * lockfile, so a push that bumps Prettier itself would be judged by the wrong + * version. Returns a message when they disagree, and null when they agree or when + * either version cannot be read — an unknown must not manufacture a block. + */ +function prettierVersionMismatch(prettierBin, checkoutDir) { + const readJson = (file) => { + try { + return JSON.parse(readFileSync(file, "utf8")); + } catch { + return undefined; + } + }; + const installed = readJson(path.join(path.dirname(path.dirname(prettierBin)), "package.json"))?.version; + const pinned = readJson(path.join(checkoutDir, "package-lock.json"))?.packages?.["node_modules/prettier"]?.version; + if (!installed || !pinned || installed === pinned) return null; + return ( + `node_modules has prettier ${installed}, but the pushed lockfile pins ${pinned}, so this ` + + `check would not match CI. Run \`npm ci\` and push again.` + ); +} + /** Prettier: 0 clean, 1 unformatted, anything else a real failure. */ function runPrettierCheck(prettierBin, cwd, args) { try { diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index a3d23ee3bb..97ab5e29dc 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -353,9 +353,14 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { if (type === "ImportDeclaration") { const specifier = (raw.source as { value?: string })?.value; if (typeof specifier !== "string") continue; + // `import type { X }` is erased at runtime and cannot mount anything, so it + // is not an edge. Both spellings matter: the declaration-level `importKind` + // and the per-specifier one from `import { type X }`. + if (raw.importKind === "type") continue; // A bare `import "x"` executes the module but renders nothing, so it is not // a mount and is deliberately not followed. for (const spec of (raw.specifiers ?? []) as Array>) { + if (spec.importKind === "type") continue; const local = (spec.local as { name?: string })?.name; if (!local) continue; const imported = @@ -378,7 +383,10 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { if (type === "ExportNamedDeclaration") { const specifier = (raw.source as { value?: string })?.value; if (typeof specifier === "string") { + // `export type { X } from "…"` is erased too. + if (raw.exportKind === "type") continue; for (const spec of (raw.specifiers ?? []) as Array>) { + if (spec.exportKind === "type") continue; const exported = (spec.exported as { name?: string })?.name; const imported = (spec.local as { name?: string })?.name ?? exported; if (exported && imported) graph.reexports.push({ source: specifier, exported, imported }); @@ -837,6 +845,17 @@ describe("band adoption detection", () => { ); expect(routeReachesBand(path.join(dir, "preload-route.tsx"))).toBe(false); + // A type-only import is erased at runtime, so it cannot mount the band even + // though the identifier appears in the mounted component's annotations. + writeFileSync( + path.join(dir, "type-only-route.tsx"), + 'import type { Banded } from "./banded-child";\n' + + "export default function Page({ render }: { render?: typeof Banded }) {\n" + + " void render;\n return
;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "type-only-route.tsx"))).toBe(false); + // A non-exported local the mounted component does render is reachable. writeFileSync( path.join(dir, "indirect-route.tsx"), From e869cb9d7ab20939361b277d1c6fcc07bbb6ca45 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 05:32:10 +0000 Subject: [PATCH 11/11] fix(test): do not count identifiers in type positions as component references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found in my own review pass, not by a reviewer. The earlier fix skipped `import type` declarations, but a *value* import used only in a type position was still recorded as a runtime edge — and this repo does not enforce `consistent-type-imports`, so `ComponentProps` is a legitimate value import that never mounts anything. References are no longer collected from typeAnnotation / returnType / typeParameters / typeArguments subtrees. Value expressions never live under those keys, so nothing real is lost; `x as Foo` still contributes `x`. The first fixture I wrote for this passed for the wrong reason — a top-level `type Props = …` alias is never scanned at all, so it proved nothing. Rewritten with the annotation inline on the scanned declaration, and confirmed to fail when the skip is removed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY --- tests/search-results-band-adoption.test.ts | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/search-results-band-adoption.test.ts b/tests/search-results-band-adoption.test.ts index 97ab5e29dc..e69a2f3c3c 100644 --- a/tests/search-results-band-adoption.test.ts +++ b/tests/search-results-band-adoption.test.ts @@ -186,6 +186,19 @@ type ModuleGraph = { /** Which exports of a module the importer actually mounts. `"*"` means any. */ type MountRoots = Set | "*"; +/** + * AST keys whose subtrees are type-only, so identifiers inside them are erased at + * runtime and must not count as component references. + */ +const TYPE_POSITION_KEYS = new Set([ + "typeAnnotation", + "returnType", + "typeParameters", + "typeArguments", + "superTypeParameters", + "implements", +]); + /** `export default () => …` has no name to hang references off. */ const ANONYMOUS_DEFAULT = "__default__"; @@ -329,6 +342,12 @@ function buildModuleGraph(source: string, filename: string): ModuleGraph { } for (const key of Object.keys(node)) { if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; + // Type positions are erased, so an identifier that appears only there is + // not a runtime reference. `ComponentProps` needs the value + // import but never mounts it, and this repo does not enforce + // `consistent-type-imports`, so such an import is not necessarily written + // as `import type`. Value subtrees never live under these keys. + if (TYPE_POSITION_KEYS.has(key)) continue; visit(node[key], resultDropped); } }; @@ -856,6 +875,22 @@ describe("band adoption detection", () => { ); expect(routeReachesBand(path.join(dir, "type-only-route.tsx"))).toBe(false); + // A *value* import used only in a type position is erased too. This repo does + // not enforce `consistent-type-imports`, so this is not necessarily spelled + // `import type` — and `ComponentProps` genuinely needs the value + // import while never mounting it. + // The annotation must be inline on the scanned declaration: a top-level + // `type Props = …` alias is never scanned at all, so it would pass for the + // wrong reason and prove nothing. + writeFileSync( + path.join(dir, "type-position-route.tsx"), + 'import { Banded } from "./banded-child";\n' + + "export default function Page(props: { render?: typeof Banded }) {\n" + + " void props;\n return
;\n}\n", + "utf8", + ); + expect(routeReachesBand(path.join(dir, "type-position-route.tsx"))).toBe(false); + // A non-exported local the mounted component does render is reachable. writeFileSync( path.join(dir, "indirect-route.tsx"),