From db7e6635d1891faed0aabd637c932479c242cc45 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 20:24:59 +0000 Subject: [PATCH 1/3] fix(hooks): keep the session-start marker out of the install inventory The SessionStart hook wrote its lockfile marker to node_modules/.session-start-lock-hash right after npm ci. That same npm ci already ran check-installed-lock-parity.mjs --write-stamp in postinstall, so the marker landed one file after the trusted inventory was taken: 51567 files against a stamp of 51566. check:installed-lock-parity therefore failed deterministically in every Claude Code web session, aborting verify:pr-local before format, lint, typecheck and test. Its printed remedy could not help either, since a fresh npm ci re-runs the hook and recreates the same off-by-one. Move the marker into node_modules/.cache, which is already in the check's VOLATILE_DIRECTORIES set and is still wiped by npm ci, so the hook's staleness semantics are unchanged. Verified: check:installed-lock-parity exits 0 at 51566 files, and the full verify:pr-local run now completes all 14 steps (6148 tests passed). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01STG6AU5J4gxrFJagP4pRti --- .claude/hooks/session-start.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh index ba351dbd28..afbf9d0ac7 100755 --- a/.claude/hooks/session-start.sh +++ b/.claude/hooks/session-start.sh @@ -64,15 +64,25 @@ cd "$CLAUDE_PROJECT_DIR" # merges, which surfaces as fake typecheck/test regressions (2026-07-19 audit). # Stamp the lockfile hash after a successful install and reinstall whenever the # lockfile no longer matches the stamp. -LOCK_STAMP="node_modules/.session-start-lock-hash" +# Keep this marker inside node_modules/.cache. npm's postinstall records a +# trusted file inventory of node_modules (scripts/check-installed-lock-parity.mjs +# --write-stamp), and this hook writes its own marker *after* that runs — so a +# marker written directly into node_modules/ leaves the tree one file ahead of +# the stamp and fails check:installed-lock-parity, which is the first real step +# of verify:pr-local. `.cache` is in that check's VOLATILE_DIRECTORIES set, so a +# marker there is ignored by the inventory while still being wiped by npm ci +# along with the rest of node_modules, preserving the staleness semantics below. +LOCK_STAMP="node_modules/.cache/session-start-lock-hash" lock_hash="$(sha256sum package-lock.json | cut -d' ' -f1)" if [ ! -d node_modules ]; then npm ci --no-audit --no-fund + mkdir -p "$(dirname "$LOCK_STAMP")" echo "$lock_hash" > "$LOCK_STAMP" echo "[session-start] Dependencies installed" elif [ ! -f "$LOCK_STAMP" ] || [ "$(cat "$LOCK_STAMP")" != "$lock_hash" ]; then echo "[session-start] node_modules is stale for the current lockfile, reinstalling" npm ci --no-audit --no-fund + mkdir -p "$(dirname "$LOCK_STAMP")" echo "$lock_hash" > "$LOCK_STAMP" echo "[session-start] Dependencies reinstalled" else From b83e518480eff3511709539289faf486a11741bb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 20:25:12 +0000 Subject: [PATCH 2/3] docs(issues): answer the offline half of #248 and correct #210 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #248 — settled without touching live. check:drift does cover the missing-index class: all four 20260705180000 indexes are in schema.sql and in the drift manifest, indexes compare by name on table + def_hash, and the drift allowlist is empty, so a run would have failed with four missing_live findings. The gap is cadence, not coverage — live-drift.yml is weekly plus manual and gates nothing. Also records a genuinely uncovered sub-class: schema_drift_snapshot() reads pg_get_indexdef but never indisvalid/indisready, so an index left invalid by a failed CREATE INDEX CONCURRENTLY compares byte-identical while the planner cannot use it. The live forensic (partial apply vs manual drop vs history repair) remains operator work behind the approved window. #210 — half already fixed, and its prescribed fix refuted. npm run typecheck has used tsconfig.typecheck.json since 450690f, which excludes .next entirely; verified green with .next/dev/types/validator.ts present. Dropping the dev-types glob from tsconfig.json does not hold: Next 16 emits it deliberately and writeConfigurationDefaults pushes it back into an existing include on every next dev/build. What remains is narrower — the Playwright isolated tsconfig inherits the repo-root globs, confirmed by probe, and Next's own dev-types filter does not apply because useTypeScriptCli defaults true. Recorded as not yet proven end-to-end. Also records the session-start marker defect fixed in the previous commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01STG6AU5J4gxrFJagP4pRti --- docs/database-drift-detection.md | 26 ++++++++++++++++++++++++++ docs/outstanding-issues.md | 7 ++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/database-drift-detection.md b/docs/database-drift-detection.md index 878c572c28..b957463a8f 100644 --- a/docs/database-drift-detection.md +++ b/docs/database-drift-detection.md @@ -52,6 +52,32 @@ storage bucket rows + storage.objects policies. carries the _identical_ name-stripped index definition under a legacy name (the machine-checked version of `search_schema_health()`'s `index_aliases`). +### Known coverage limits + +What the check does **not** see — established offline on 2026-08-12 while +answering `/issues` `#248` (the 20260705180000 search-health indexes that were +missing on live despite an applied history): + +- **Invalid indexes read as healthy.** `schema_drift_snapshot()` + (`20260706200000`) builds its index rows from `pg_index` via + `pg_get_indexdef` + an md5 `def_hash`, and never reads `indisvalid` / + `indisready`. An index left behind by a failed `CREATE INDEX CONCURRENTLY` + still has a definition, so it compares byte-identical and drift stays green + while the planner refuses to use it. `20260804110240` checks both flags at + apply time, so the guard exists for that one migration but not for the + ongoing probe. +- **Nothing gates on it.** `check:drift` runs only from + `.github/workflows/live-drift.yml` — `workflow_dispatch` plus a weekly Sunday + 18:30 UTC cron — and blocks no PR or release. Coverage of the plain + missing-index class is genuine (indexes compare by name on `table` + + `def_hash`, and `supabase/drift-allowlist.json` is empty, so a missing index + fails the run), but nothing forces a run between the drift appearing and + runtime `search_schema_health()` noticing. + +Both are decisions rather than defects: adding validity to the snapshot RPC is +a migration, and raising the cadence spends provider budget. Recorded so the +gap is chosen, not assumed away. + ### Workflow - Change `supabase/schema.sql` → run `npm run drift:manifest` (Docker) in the diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 67556e148d..ccfbcd2243 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -137,7 +137,7 @@ removed after current-main verification; it is not missing recommended work. | 82 | `#257` | Optional | High — formulation/specifiers flake | Standing until second reproduction | 15–30 min | Single unreproduced ui-formulation flake when run with ui-specifiers — record a second sighting only; do not quarantine until three on the same SHA. **Stop:** do not weaken assertions. | - + ## Open items > **Merged-main canary update (2026-07-23, run `30018289898`):** the new structured report correctly recorded evaluated tree `c24f2e8f2d30d0c59fc1eba025d3dcd63478137e`, run/attempt identity and `cross-region-runner` latency context. Golden retrieval remained 36/36 with document/content recall 1.0 and no failed cases. The 44-case answer gate had grounded-supported and unsupported-correct rates of 1.0, but failed because `neuroleptic-side-effect-escalation` again returned one citation where two are required (citation-failure rate 0.0227). `admission-discharge-comparison` again omitted the specific AKG admission document after `comparison_source_extractive_fallback`; `admission-discharge-coverage-paraphrase` was advisory-only at 24,870 ms. Answer cost was reported as `$0.234736`. Do not retry immediately: retain this as the first structured datapoint, compare it with the scheduled 2026-07-26 report, and keep retrieval/ranking unchanged. @@ -214,7 +214,7 @@ removed after current-main verification; it is not missing recommended work. | #200 | P3 | task | DR: Re-enter dashboard config after schema restore | **Outcome:** auth providers/SSO redirect URLs, connection-pool caps, per-project keys, and `E2E_USER_*` are re-entered in the Supabase/Railway dashboards after restore. **Next:** operator checklist in `docs/operator-backlog.md`. Parent `#188`. **Stop:** do not commit dashboard secrets. | docs/operator-backlog.md; #188 | 2026-07-31 | | #206 | P2 | task | AnswerState partial_retrieval has no app-facing producer | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: `partial_retrieval` is declared in src/lib/answer-state-types.ts:63 and handled in answer-clipboard.ts:75, but nothing in src/app or the retrieval path produces it — still no app-facing producer, as the row says. Do not synthesise it from candidate counts. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. PR-E step 0 found nothing in the client payload names which expected sources were unavailable (retrievalDiagnostics = candidate counts; conflictsOrGaps = prose). RetrievalStateBanner supports the state but PR-J adoption can only emit ready/stale_evidence/source_only. Next action: decide whether a separate RAG contract PR should add a named missing-source signal (governance preflight + RAG impact line + offline eval); until then do not synthesise the state from counts. Pinned by tests/answer-state-contract.test.ts and SPEC 13 / COMPONENTS 2. | PR-E step 0, session 2026-08-02 | 2026-08-02 | | #209 | P3 | task | DS V2 Gate 1: add contrast pair for --warning used as body text | IN FLIGHT 2026-08-12 in PR #1841 (adds an explicit --warning body-text contrast assertion in tests/design-token-contract.test.ts). Checked against the open-PR list during the full ledger sweep. Do NOT start this row while that PR is open — duplicating a queued conversion is the exact failure #292 records, and it has happened twice. Re-verify this row against main after that PR merges, and close it there rather than here. VerificationNotice's caution variant and DoseLine's overdue label use --warning at text tier — the only place a status hue is used as body-text colour rather than a --text-* token. Gate 1's contrast checking must add that pair explicitly rather than assuming the text tiers cover it. Also note: the logged-once Sets in missing-value, date-display, verification-notice, answer-state and retrieval-state-banner are module-level, so on the server they are per-process and unbounded; a persistent data defect logs once at boot then is swallowed. Acceptable while unregistered. | clinical-governance-reviewer P3 findings on PR 6; recorded in docs/design-system/SPEC.md PR 6 clinical review note | 2026-08-02 | -| #210 | P2 | task | npm run ensure generates .next/dev types that break typecheck and every Playwright build | RETITLED AND RE-SCOPED 2026-08-12 — the original title 'Restore the npm run typecheck gate' is wrong and cost this row its clarity: the gate was never missing. `npm run typecheck` IS in `verify:cheap:internal` (package.json:71) and runs today. The live defect is the generated-types include: tsconfig.json:28 still lists `.next/dev/types/**/*.ts` in `include`, so running `npm run ensure` — which the repo's own docs tell you to do before any browser work — makes the dev server write .next/dev/types/validator.ts, and that file then breaks BOTH repo-wide `npm run typecheck` AND every Playwright production build, because scripts/run-playwright.mjs writes an isolated tsconfig extending the root one ('Type error: Cannot find name __IsExpected'). `rm -rf .next/dev` restores both. Next: stop including dev-generated types in the checked project (drop `.next/dev/types/**/*.ts` from include, or give the Playwright isolated tsconfig its own include list), then confirm typecheck stays clean after `npm run ensure`. Stop: do not 'fix' this by removing typecheck from the gate — the gate is not the problem. | session 2026-08-02 /ledger sweep; docs/review-findings-2026-08-02.md | 2026-08-02 | +| #210 | P2 | task | npm run ensure generates .next/dev types that break typecheck and every Playwright build | RE-SCOPED AGAIN 2026-08-12 — half of this row is already fixed and its prescribed fix is refuted. (1) FIXED: the repo-wide typecheck gate no longer breaks. `npm run typecheck` runs `tsconfig.typecheck.json` (added in 450690f, citing this row), which sets its own include and excludes `.next/**`; verified green with `.next/dev/types/validator.ts` present. (2) REFUTED — do not apply this row's first suggestion. Dropping `.next/dev/types/**/*.ts` from tsconfig.json include does NOT hold: Next 16 writes that glob itself. `getTypeDefinitionGlobPatterns` (node_modules/next/dist/lib/typescript/type-paths.js) emits both `.next/types` and `.next/dev/types` deliberately 'to avoid tsconfig churn when switching between dev/build modes', and `writeConfigurationDefaults` (same dir, lines 302-316) pushes any missing glob back into an existing include on every `next dev`/`next build`. Deleting the line just re-creates an uncommitted change. (3) STILL OPEN, narrower than written: `scripts/run-playwright.mjs` writes an isolated tsconfig with `extends: '../../tsconfig.json'` and no include of its own, so it inherits the repo-root globs. Probe evidence: `tsc --showConfig` on an equivalent config resolves include `['../../.next/types/**/*.ts', ..., '../../.next/dev/types/**/*.ts']` and `--listFilesOnly` pulls in all four root dev-type files including validator.ts. Next's own dev-types filter (`getDevTypesPath`, applied in runTypeCheck.js:36-39) does NOT cover this, because `useTypeScriptCli` defaults true (config-shared.js:257) and the CLI checker honours tsconfig include verbatim — the Next 16 TypeScript doc says so explicitly. Next: give the isolated tsconfig its own include/exclude (the run root is `.next-playwright/`, not under `.next/`, so excluding the repo-root `.next` keeps the run's own dist types). NOT YET PROVEN end-to-end: the failing Playwright build was not reproduced here, because `next build` mutates that isolated config via writeConfigurationDefaults before typechecking; confirm with one `verify:ui` build before and after. Stop unchanged: do not remove typecheck from the gate, and do not re-try the include deletion. | session 2026-08-02 /ledger sweep; docs/review-findings-2026-08-02.md | 2026-08-02 | | #211 | P2 | task | Plan and start the noUncheckedIndexedAccess migration | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: `noUncheckedIndexedAccess` is absent from tsconfig.json — the migration has not begun. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. Enable noUncheckedIndexedAccess in a branch and remediate the 1,266 errors, starting with the 15-20 highest-risk source files. Hot spots include worker/main.ts:901-942, src/lib/rag/rag-extractive-answer.ts, and src/lib/answer-verification.ts. Prefer ?. or ?? guards, or non-null assertions only where invariants are provable. Re-run npm run test and npm run typecheck before merge. See docs/review-findings-2026-08-02.md section 6. | session 2026-08-02 /ledger sweep — docs/review-findings-2026-08-02.md | 2026-08-02 | | #212 | P2 | task | Replace as unknown as casts and unvalidated JSON.parse with Zod or runtime guards | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: 40 `as unknown as` casts remain under src/ — the row's population is intact. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. 48 as unknown as casts and ~24 unvalidated JSON.parse calls across src/ trust Supabase, OpenAI, localStorage, file metadata and extraction boundaries. Start with src/lib/rag/rag.ts and src/app/api/* routes, mirroring existing Zod use in src/lib/validation/body.ts and src/lib/extractors/document.ts. See docs/review-findings-2026-08-02.md sections 2.2, 2.3 and 8. | session 2026-08-02 /ledger sweep — docs/review-findings-2026-08-02.md | 2026-08-02 | | #213 | P2 | task | Stop swallowing fetch and stream errors with empty catch handlers | SCOPE RE-MEASURED 2026-08-12 on merged main: only **3** empty catch handlers remain under src/ (`catch {}` / `catch (e) {}`), down from the audit population this row was opened against. The principle is unchanged and the remaining three still need dispositioning — each should either handle, log through the observability path, or carry a comment saying why swallowing is correct — but this is now a small, closeable job rather than a sweep. Companion rows measured in the same pass for sequencing: #212 has 40 `as unknown as` casts left, #211's `noUncheckedIndexedAccess` is still absent from tsconfig.json. Do the three catches first; it is the cheapest of the three and no longer blocked behind the other two. | session 2026-08-02 /ledger sweep — docs/review-findings-2026-08-02.md | 2026-08-02 | @@ -234,7 +234,7 @@ removed after current-main verification; it is not missing recommended work. | #242 | P2 | task | Commit approved Linux visual baselines and promote adoption not-committed → committed | VERIFIED CORRECT 2026-08-12 — re-checked against merged main during the full ledger sweep and left unchanged: Six linux/ PNGs are committed, but the adoption manifest still carries 68 `not-committed` entries — the surfaces flip is the remaining work, as stated. This stamp exists so a later reader can tell "checked and still true" from "never looked at"; the two were indistinguishable before. Baselines and provenance are DONE as of PR #1729 (branch claude/ds-adopt-visual-baselines): all six linux/ PNGs committed from ubuntu artifact visual-baseline-31251091603 (main @ bc33d414e), AWAITING_BASELINE emptied, and tests/__screenshots__/linux/provenance.json written with per-candidate SHA-256 + dimensions and an approved human review. Proven by that PR's own run: visual-junit tests=9 failures=0 skipped=0, and no visual-candidates/ directory, i.e. all six compared rather than skipped. REMAINING: only the surfaces flip to baseline.status committed. Blocked on ordering, measured 2026-08-08: validateLinuxVisualBaselineSet short-circuits on declaredPaths.length===0, so declaring files activates its rule that no non-allowlisted path may change since candidateSourceHead — and PR #1729 necessarily changed tests/design-system-adoption.test.ts, whose initialiseCandidateRepository seeded fixtures from the LIVE spec and so failed the moment AWAITING_BASELINE emptied. The two cannot land together. Next: after #1729 merges, re-capture candidates from a main run that already contains that fixture fix, then flip the surfaces against that head. Note this does not affect whether pixels compare — Playwright compares because the goldens exist on disk. | PR #1616 review findings; session 2026-08-05 | 2026-08-05 | | #244 | P3 | rec | Forced-colours v2 mapping depends on grouped dark selectors staying in the media block | IN FLIGHT 2026-08-12 in PR #1841 (forced-colours selector specificity). Checked against the open-PR list during the full ledger sweep. Do NOT start this row while that PR is open — duplicating a queued conversion is the exact failure #292 records, and it has happened twice. Re-verify this row against main after that PR merges, and close it there rather than here. ckb-v2-tokens.css forced-colours block lists .ckb-v2.ckb-v2, .dark .ckb-v2.ckb-v2, and .ckb-v2.dark.ckb-v2 so specificity matches dark rules. Trimming to a single .ckb-v2.ckb-v2 silently drops dark HCM. Next: keep the contract test pin; never trim that selector group. | PR #1616 review findings; session 2026-08-05 | 2026-08-05 | | #245 | P3 | rec | responsive-compact CrossModeLinks keeps duplicate rails in the DOM | IN FLIGHT 2026-08-12 in PR #1842 (CrossModeLinks rail behaviour). Checked against the open-PR list during the full ledger sweep. Do NOT start this row while that PR is open — duplicating a queued conversion is the exact failure #292 records, and it has happened twice. Re-verify this row against main after that PR merges, and close it there rather than here. Phone chip rail and md+ card rail both mount; display:none removes the inactive from the a11y tree. Tests/analytics counting role=link see doubles; cross-mode-links-rail is phone-only. Next: prefer the variant test ids; do not collapse to one rail with JS breakpoints (hydration risk). | PR #1616 review findings; session 2026-08-05 | 2026-08-05 | -| #248 | P2 | issue | Investigate why 20260705180000 search-health indexes were missing on live despite applied history | PR #1614 repairs the symptom with a mark-applied guard only. Confirm out-of-band whether the earlier reconcile migration partially applied, indexes were manually dropped, or schema_migrations was repaired — and whether check:drift should have caught this class before runtime search_schema_health. Renumbered from this PR's original #237 → #246 because main already used #237–#247 (PR #1616 findings plus the results-bar rows from PR #1615). | PR #1614 review / session 2026-08-05 (renumbered on main merge) | 2026-08-05 | +| #248 | P2 | issue | Investigate why 20260705180000 search-health indexes were missing on live despite applied history | OFFLINE HALF ANSWERED 2026-08-12; live forensic still open. The row asked two questions; the second is now settled without touching live. (a) DOES check:drift cover this class? Yes, structurally. All four 20260705180000 indexes (document_labels_label_trgm_idx, document_summaries_summary_trgm_idx, document_index_units_owner_chunk_type_idx, rag_retrieval_logs_miss_idx) are present in supabase/schema.sql and in supabase/drift-manifest.json's expected snapshot, indexes are a compared category in check-drift.ts (keyed by name, content-compared on table + def_hash), and supabase/drift-allowlist.json is EMPTY — nothing was suppressing them. A check:drift run would have emitted four missing_live findings and failed. So coverage was never the gap. (b) WHY didn't it catch it in time? Cadence and gating, not coverage: check:drift only runs from .github/workflows/live-drift.yml, which is workflow_dispatch + a weekly Sunday 18:30 UTC cron, is provider-backed (service-role secret), and gates no release or PR. Nothing blocks on it, so live indexes can go missing and stay missing until the next weekly run — or until runtime search_schema_health notices first, which is what happened. (c) NEW, uncovered sub-class worth deciding on: schema_drift_snapshot() (20260706200000) builds its index rows from pg_index using pg_get_indexdef + md5 def_hash and does NOT capture indisvalid/indisready. An index left invalid by a failed CREATE INDEX CONCURRENTLY therefore appears to check:drift as present and byte-identical while the planner cannot use it — drift stays green, search degrades. That is exactly the failure shape 20260804110240_restore_rag_search_health_indexes.sql guards against at apply time (it checks indisvalid/indisready explicitly), so the guard exists for the migration but not for the ongoing drift probe. Next (operator, needs the approved live/history window): determine which of partial-apply / manual-drop / schema_migrations-repair actually occurred, and decide whether to (i) add validity to the snapshot RPC and (ii) raise the drift cadence or gate something on it. Stop unchanged: no hosted mutation without approval. | PR #1614 review / session 2026-08-05 (renumbered on main merge) | 2026-08-05 | | #250 | P2 | task | Execute the fastest-wins multi-wave plan (Wave 0–4) | SUPERSEDED IN LARGE PART 2026-08-12 — this row is a wave plan whose contents have been overtaken, and as written it now misdirects. Rows it names as live A1 work are CLOSED: #207 and #226 were archived on main, and #166 (its sibling in that cluster) archived in the 2026-08-12 sweep. Of its engineering waves: 1B's gate-integrity set is done (#149, #210 re-scoped, #204/#167 covered by in-flight PR #1837); 1C's hygiene set is largely done or in flight (#232, #151, #154, #187, #142, #156, #186 all now carry IN FLIGHT notes against PR #1835/#1836); Wave 0/#202 is in flight in PR #1840. What genuinely remains of the plan is Wave 1A (#147 phone CLS, still open and still reproducible offline at zero provider cost), Wave 2 (#117 then #118), and Wave 3 (#098 then #189). The A1 track is now just #059, #053 and #231 — two operator rows and one live investigation. Next: either re-cut this row against that much smaller remainder, or close it and let #147/#117/#118/#098/#189 stand on their own, which is probably the honest move now that the multi-agent framing has served its purpose. Stop unchanged: no RAG behaviour change without flag plus canary, no provider gates without approval, and do not mix operationalRisk with clinical or UI in one squash. | session 2026-08-05 fastest-wins plan | 2026-08-05 | | #253 | P3 | task | #1606 needs a hand-merge against merged PR #1615, not a rebase | SUPERSEDED IN PART 2026-08-07: the component both PRs rewrite no longer exists. `MobileResultFilterControl` — the native `