diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index fdc780ef..c13f00fd 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -801,6 +801,8 @@ This file is append-only. Never rewrite or delete an existing review record; app | 2026-07-24 | execute-audit-code-remediation (PR #1162) | 3cb7c977 | Conflict fix + Bugbot + local review | Before: CONFLICTING (21 files). After: mergeable. Restored atomic upload RPC; aligned private-access tests (133/133). Bugbot 2 medium left open. | private-access-routes 133/133; no provider-backed checks | | 2026-07-25 | codex/document-clinical-summary-20260725 (PR #1169) | 605a47b551a03774fab41416bf980dfbc9610221 | Open-PR maintenance: malformed persisted profile guard | Before: one actionable thread showed non-array or malformed persisted summary groups could throw during render. After: every priority group is normalized through an array/item guard and malformed values are ignored while valid items still render. | Focused Vitest 7/7 pass; Prettier and diff checks pass; no provider-backed checks run. | | 2026-07-25 | cursor/search-performance-review-4ee9 (PR #1134) | 692834a86e612cc8b311dc6895e007f182f5c5b8 | Open-PR maintenance: superseded docs-link thread and clean main sync | Before: branch was behind current main with one outdated docs-link thread; its product tree already matched main. After: merged current main cleanly and verified the route-group-aware docs-link fix now covers legacy route references. RAG impact: no retrieval behaviour change — history sync and docs tooling verification only. | `node scripts/check-docs-links.mjs` pass (1154 references); clean merge-tree; no live RAG canary or provider-backed check run. | +| 2026-07-24 | cursor/search-correctness-030-075-6273 (PR #1177) | 54ab9f8498751ef7e96815dd2496b8137f29dad7 | Review + follow-up hardening of #030/#075 search-correctness fixes | Findings fixed: (P2) one combo-titled source could still make multi-slot allHit true via substring alias hits — `expectedFileCoverage` now assigns each retrieved top-file to at most one expected slot; (P2) label pagination could loop forever on a stuck full-page API — fail-closed page budget added; (P2 process) stale `PR_POLICY_BODY.md` from search-performance leftover was overwriting this PR body via Sync PR policy body — corrected then deleted. No remaining high-confidence P0–P1 in product scope. Residual: human approving review; Unit coverage CI still finishing on later heads. RAG impact: no retrieval behaviour change — eval matching / label pagination only. | Focused Vitest 32/32; `verify:cheap` green; `verify:pr-local` green (lint/typecheck/3326 unit/build/client-bundle/offline RAG fixtures 36/36). No OpenAI/live Supabase/provider-backed canary. | +| 2026-07-24 | cursor/search-correctness-030-075-6273 (PR #1177) | 96ba6152c1f8e5e0000000000000000000000000 | Supersedes prior #1177 review row with post-sync tip | Same product outcome as prior row; tip includes correct PR_POLICY_BODY sync + template deletion so Sync PR policy body cannot reintroduce the stale search-performance description. | `npm run check:branch-review-ledger` pass; no provider-backed checks run. | | 2026-07-24 | PR #1175 / `cursor/ledger-009-010-032-041-063-519b` | f3986abc39468e077643611ec2d95e374c2e901f | PR review + CI fix (ledger docs #009/#010/#032/#041/#063) | FINDINGS FIXED. P1: Static PR checks failed on Prettier (5 docs). P1: leftover `PR_POLICY_BODY.md` from merged #1134 caused Sync PR policy body to overwrite this docs PR description with search-performance text — deleted the stale template. No P0. Product scope remains docs-only; gated brief follow-ups (wire coming-soon, Current Clinical Work storage, Factsheets second mode, governance ranking) correctly not implemented. Residual: human approving review once CI green. | Local: prettier --check on touched docs; ledger open/resolved/queue integrity for five IDs; no client fetch(`/api/jobs`); `verify:cheap` earlier on tip 95d68c6b. No OpenAI/Supabase writes. | | 2026-07-24 | PR #1175 / `cursor/ledger-009-010-032-041-063-519b` | 15a5d080a375635ca6ef042659fe8837d485c8e3 | PR #1175 follow-up (description restore + template removal) | SUPERSEDES prior #1175 row on `f3986abc`. PR description restored via temporary `PR_POLICY_BODY.md` sync then file deleted again so merge will not reintroduce the #1134 leftover. Scope unchanged: docs-only ledger closeout. | Sync PR policy body SUCCESS with correct ledger summary; prettier clean on prior tip. Awaiting Static/Unit on final tip. No providers. | | 2026-07-25 | implement-audit-viewport-fixes (PR #1140) | 283ad65377ee3e60ceb70f5cd01974caf0a54227 | Open-PR maintenance: keyboard overlay fallback | Before: fixed docks always consumed the measured visual-viewport height, double-lifting them when `interactive-widget=resizes-content` already resized the layout viewport. After: the provider subtracts layout shrink and exposes only residual overlay height to CSS while retaining keyboard-open state. | Focused Vitest 18/18 pass; `npm run ensure` verified http://localhost:3264; Prettier and diff checks pass; no provider-backed checks run. | @@ -824,16 +826,16 @@ This file is append-only. Never rewrite or delete an existing review record; app | 2026-07-25 | fix-physics-animation-audit (PR #1142) | bdfe81e15c57d376ff74ddb611a8959b0ae94cc9 | Open-PR maintenance: review fix + drift | Before: 24 commits behind and 1 unresolved P2 thread; CSS changed phone reserve timing without pinning the timing in static/phone-scroll coverage. After: current main is merged; static coverage pins 200/240ms transitions and the motion-enabled phone-scroll sweep asserts the active 200ms reserve transition before geometry checks. | Prettier check pass; `git diff --check` pass; focused Vitest/Playwright not run because repository heavyweight lock is owned by worktree 6314; hosted CI will exercise the updated tests; no provider-backed checks run. | | 2026-07-25 | fix-physics-animation-audit (PR #1142) | c88c4516476cae3246e1975ce648dff0f3ecb3f7 | Ledger append-only placement fix | CORRECTION: relocated the five PR #1142-unique ledger rows that had been inserted below the table header / among older entries so they append after the final existing record, without rewriting any other rows' content. Restores the append-only contract called out in the Codex P1. | `npm run check:branch-review-ledger`; no provider-backed checks run | | 2026-07-25 | implement-audit-viewport-fixes (PR #1140) | f4ae0a7217e513b893d14400fcfd49f31a3dd090 | PR babysit sweep: sync + threads + CI fix + squash merge | Before: CONFLICTING/DIRTY (stale), threads open, behind main. After: merged origin/main, resolved Codex/CodeRabbit threads, fixed document-viewer keyboard lift + baseline reset + Sources focus restore; Production UI green; squash-merged. | Hosted CI PR required SUCCESS on tip 761765a3f; focused vitest keyboard/overlay contracts. No provider-backed checks run. | -| 2026-07-25 | implement-audit-recommendations-fix (PR #1140) | f4ae0a7217e | Babysit sweep: viewport/keyboard audit � squash-merged after CI green + thread triage | production-ui + pr-required | merged | -| 2026-07-25 | fix-physics-animation-audit (PR #1142) | e966b5aa972 | Babysit sweep: spring physics / reduced-motion � auto-merged after main sync | pr-required | merged | -| 2026-07-25 | information-page-shell (PR #1148) | 5b9574af480 | Babysit sweep: unify information-page structure � squash-merged | pr-required | merged | -| 2026-07-25 | mobile-ergonomics-fixes (PR #1156) | de1a82b4936 | Babysit sweep: mobile touch ergonomics � squash-merged | pr-required | merged | -| 2026-07-25 | automated-audit-remediations (PR #1158) | aa745922f00 | Babysit sweep: automated audit remediations � squash-merged | pr-required | merged | -| 2026-07-25 | cursor-indexing-ignore (PR #1171) | 3a4036580df | Babysit sweep: Cursor indexing ignore rules � squash-merged | pr-required | merged | -| 2026-07-25 | cursor/ledger-009-010-032-041-063-519b (PR #1175) | 87b6b432c19 | Babysit sweep: close ledger #009/#010/#032/#041/#063 � resolved outstanding-issues merge + prettier, squash-merged | static-pr + pr-required | merged | -| 2026-07-25 | canary-comparison-preflight (PR #1180) | 43f261cf229 | Babysit sweep: canary comparison preflight docs � squash-merged | pr-required | merged | -| 2026-07-25 | sitewide-design-review-ledger (PR #1181) | 8284fcd4420 | Babysit sweep: design-review ledger � auto-merged after sync | pr-required | merged | -| 2026-07-25 | codex/complete-all-pending-tasks (PR #1191) | e2488dbb108 | Babysit sweep: scoped-label pagination + order assertion for CodeRabbit thread � squash-merged | unit + pr-required | merged | +| 2026-07-25 | implement-audit-recommendations-fix (PR #1140) | f4ae0a7217e | Babysit sweep: viewport/keyboard audit — squash-merged after CI green + thread triage | production-ui + pr-required | merged | +| 2026-07-25 | fix-physics-animation-audit (PR #1142) | e966b5aa972 | Babysit sweep: spring physics / reduced-motion — auto-merged after main sync | pr-required | merged | +| 2026-07-25 | information-page-shell (PR #1148) | 5b9574af480 | Babysit sweep: unify information-page structure — squash-merged | pr-required | merged | +| 2026-07-25 | mobile-ergonomics-fixes (PR #1156) | de1a82b4936 | Babysit sweep: mobile touch ergonomics — squash-merged | pr-required | merged | +| 2026-07-25 | automated-audit-remediations (PR #1158) | aa745922f00 | Babysit sweep: automated audit remediations — squash-merged | pr-required | merged | +| 2026-07-25 | cursor-indexing-ignore (PR #1171) | 3a4036580df | Babysit sweep: Cursor indexing ignore rules — squash-merged | pr-required | merged | +| 2026-07-25 | cursor/ledger-009-010-032-041-063-519b (PR #1175) | 87b6b432c19 | Babysit sweep: close ledger #009/#010/#032/#041/#063 — resolved outstanding-issues merge + prettier, squash-merged | static-pr + pr-required | merged | +| 2026-07-25 | canary-comparison-preflight (PR #1180) | 43f261cf229 | Babysit sweep: canary comparison preflight docs — squash-merged | pr-required | merged | +| 2026-07-25 | sitewide-design-review-ledger (PR #1181) | 8284fcd4420 | Babysit sweep: design-review ledger — auto-merged after sync | pr-required | merged | +| 2026-07-25 | codex/complete-all-pending-tasks (PR #1191) | e2488dbb108 | Babysit sweep: scoped-label pagination + order assertion for CodeRabbit thread — squash-merged | unit + pr-required | merged | | 2026-07-25 | open-pr-babysit-sweep-20260725 | multipass | Babysit continuation: approved action_required workflows; Sources autofocus fix on #1141; ledger dedupe #1157; outstanding-issues merges #1175/#1177; skipped non-trivial conflict clusters #1162/#1185-1190/#1187 draft | gh checks + merge-tree | in-progress | | 2026-07-25 | implement-audit-viewport-fixes (PR #1140) | f4ae0a7217e513b893d14400fcfd49f31a3dd090 | PR babysit sweep: sync + threads + CI fix + squash merge | Before: CONFLICTING/DIRTY (stale), threads open, behind main. After: merged origin/main, resolved Codex/CodeRabbit threads, fixed document-viewer keyboard lift + baseline reset + Sources focus restore; Production UI green; squash-merged. prlanded content-diff empty. | Hosted CI PR required SUCCESS on tip 761765a3f; focused vitest keyboard/overlay contracts. No provider-backed checks run. | | 2026-07-25 | cursor/canary-artifact-comparison-8e05 (PR #1180) | 618d8640fa528de4a94b0d3e2599bcfe0df3f6f5 | PR babysit: retrigger required CI | Empty sync after main advanced; no product change. | No provider-backed checks. | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index f84cc939..3dc64d29 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -129,7 +129,6 @@ removed after current-main verification; it is not missing recommended work. | #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 | | #029 | P2 | issue | 12 of 30 answer-quality cases return the fallback stub | run #61 --dump-answers: 12/30 quality cases emit the source_backed_review_fallback boilerplate with answer_sections: [], all grounded with 4-6 citations. Some still PASS targeting because the stub echoes query keywords (the contraindication/document_lookup matchers need only a keyword), so the targeting metric MASKS the problem for those intents. Superset of #018 ΓÇö fix in the extractive composer, validate with the provider-backed answer eval. | run #61 dump artifact; session 2026-07-22 | 2026-07-22 | -| #030 | P2 | issue | Wide-tier alias lets one doc satisfy both comparison slots | In src/lib/eval-document-matching.ts, "Admission to Discharge for Mental Health Inpatients" appears in BOTH the AdmissionCommunityPts and Discharge alias lists, so a single document can satisfy both expectedFiles slots and make allHit true ΓÇö a latent false-pass on admission-discharge cases. Not firing today (that doc is not in the failing top-5) but it would mask a real miss. Tighten the tables so one doc cannot fill both sides. | src/lib/eval-document-matching.ts:32-65; session 2026-07-22 | 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 | @@ -137,7 +136,6 @@ removed after current-main verification; it is not missing recommended work. | #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 | -| #075 | P2 | issue | Search-scope label enumeration can truncate after 1,000 rows | **Outcome:** every owner-visible source label can participate in scoped search without silently dropping rows at the Supabase response cap. **Next:** reproduce on current `main` with more than 1,000 distinct labels, then add bounded deterministic pagination and a focused multi-page regression. **Verify:** search-scope unit/API contracts, offline RAG contracts, `verify:cheap`, and production-readiness. **Stop:** do not replay mixed PR #1132, widen aliases, change ranking, or run a live canary unless the isolated fix demonstrably changes protected retrieval behaviour. | PR #1132 (`src/lib/search-scope.ts`, `tests/search-scope.test.ts`); primary reconciliation 2026-07-24 | 2026-07-24 | | #077 | P2 | issue | Concurrent tasks can re-dirty the canonical primary checkout | **Outcome:** `C:\Dev\Apps\Database` remains a clean synchronization target while write work happens in task-owned worktrees. **Next:** add a cooperative owner/lease check to task-start and lifecycle transitions; before a primary write, branch switch, fast-forward, or cleanup, report owner, dirty state, and Git operation markers and fail closed on an active owner. Include stale-lease recovery. **Success:** a focused concurrency test refuses a second primary writer while read-only commands and separate worktrees continue normally. **Stop:** do not add an OS-wide lock, kill processes, discard existing dirty files, or serialize independent feature worktrees. | primary re-dirtied by another active task immediately after reconciliation proof; session 2026-07-24 | 2026-07-24 | | #078 | P3 | task | Generate a deterministic reconciliation evidence pack | **Outcome:** one report-only command produces the final local evidence now assembled manually. **Next:** extend the reconciliation lifecycle with an explicit output path and include the frozen base/HEAD, per-worktree dispositions, operation markers resolved through Git, archive refs, bundle path/size/SHA-256/verify result, retained worktree count, and local/base tree equality. Accept remote PR state only as explicit approved input. **Success:** fixture tests prove deterministic output and redaction; an interrupted run leaves no false completion record. **Stop:** never fetch, call GitHub/providers, inspect secret values, mutate refs, or delete work implicitly. | `scripts/reconciliation-preflight.mjs`; `docs/reconciliation-playbook.md`; session 2026-07-24 | 2026-07-24 | | #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 | @@ -148,6 +146,8 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | ID | Type | Summary | Outcome | Resolved | | ---- | ----- | ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #030 | issue | Wide-tier alias lets one doc satisfy both comparison slots | Fixed on `cursor/search-correctness-030-075-6273`: removed dual-listed Admission-to-Discharge titles from AdmissionCommunityPts so one retrieved source cannot make allHit true for both comparison slots; fail-closed contracts in `tests/eval-document-matching.test.ts`. RAG impact: no retrieval behaviour change ΓÇö eval matching only. | 2026-07-24 | +| #075 | issue | Search-scope label enumeration can truncate after 1,000 rows | Fixed on `cursor/search-correctness-030-075-6273`: `loadScopeLabels` pages document_labels with deterministic order/batching past the Supabase 1k cap; multi-page >1000 contracts in `tests/search-scope.test.ts`. Isolated from mixed PR #1132. RAG impact: no retrieval behaviour change ΓÇö label pagination only. | 2026-07-24 | | #009 | rec | Confirm `/api/jobs` is intentionally server/ops-only | Kept as deliberate administrator/ops listing: no client `fetch("/api/jobs")` (UI uses `/api/ingestion/jobs`); documented in `docs/api-jobs-ops-surface.md` plus wiring/codebase-index/site-map notes. Not abandoned ΓÇö do not remove without updating API contract tests. | 2026-07-24 | | #010 | task | Un-built "Coming soon" controls across forms/favourites | Audited forms/favourites/presentation placeholders: all use honest `disabled` or `aria-disabled` + coming-soon copy (or presentational `ToggleSwitch` without `onToggle`). No fake-interactive controls; leave unwired until features land. Recorded in `docs/wiring-conventions.md`. | 2026-07-24 | | #032 | rec | Governance ranking weighting: REFUTED, not debt | Reinforced as guardrail only in `docs/rag-behaviour/refuted-approaches.md` (Refutation 3), README, and safeguards ΓÇö do **not** implement `review_due`/unknownCurrentness ranking penalties or boosts. No retrieval/ranking code changed. RC8 filter path remains the only revisit route behind canary gates. | 2026-07-24 | diff --git a/src/lib/eval-document-matching.ts b/src/lib/eval-document-matching.ts index aeddebf4..b913a18f 100644 --- a/src/lib/eval-document-matching.ts +++ b/src/lib/eval-document-matching.ts @@ -32,13 +32,9 @@ const clinicalDocumentAliases: Record = { AdmissionCommunityPts: [ "Admission of Community Patients", "Admission Community Patients", - // Ground-truth widening (user-approved 2026-07-21, eval baseline run #57 triage): the live - // corpus carries several legitimately on-point admission policies; the NMHS - // admission-to-discharge pair satisfies the admission expectation for the - // admission-discharge comparison cases. Deliberately EXCLUDES discharge-only documents - // (they must not satisfy the admission slot) and the HITH programme policy (too narrow). - "Admission to Discharge for Community Mental Health", - "Admission to Discharge for Mental Health Inpatients", + // Deliberately EXCLUDES admission-to-discharge and discharge-only documents. Those titles + // remain on the Discharge alias list; listing them here too let one retrieved source fill + // both comparison slots and make allHit true (#030). Keep this side admission-only. ], AgitationArousalPharmaMgt: [ "Agitation and Arousal Pharmacological Management", @@ -62,6 +58,8 @@ const clinicalDocumentAliases: Record = { ], CommunityHomeVisit: ["Community Home Visit", "Home Visit", "Community Visits"], Discharge: [ + // Admission-to-discharge titles are Discharge-slot only (#030). Do not re-add them under + // AdmissionCommunityPts — a single dual-listed doc would false-pass multi-slot coverage. "Admission to Discharge for Mental Health Inpatients", "Admission to Discharge for Community Mental Health", "Referral Admission and Discharge Mental Health Hospital in the Home", @@ -117,11 +115,19 @@ export function expectedFileCoverage( limit = 3, ): ExpectedFileCoverage { const topFiles = sources.slice(0, limit).map(resultDocumentText); - const matchedFiles = expectedFiles.filter((expected) => - documentExpectationAlternatives(expected).some((alternative) => - topFiles.some((file) => file.includes(alternative)), - ), - ); + // Distinct source identities (#030): each retrieved top-file may satisfy at most one + // expectedFiles slot. Without this, a single combo-titled document (or overlapping + // aliases) can make allHit true even when a true second source is missing. + const usedSourceIndexes = new Set(); + const matchedFiles = expectedFiles.filter((expected) => { + const alternatives = documentExpectationAlternatives(expected); + const matchIndex = topFiles.findIndex( + (file, index) => !usedSourceIndexes.has(index) && alternatives.some((alternative) => file.includes(alternative)), + ); + if (matchIndex < 0) return false; + usedSourceIndexes.add(matchIndex); + return true; + }); return { expectedFiles, diff --git a/src/lib/search-scope.ts b/src/lib/search-scope.ts index f86b82ae..8f2de8f7 100644 --- a/src/lib/search-scope.ts +++ b/src/lib/search-scope.ts @@ -22,6 +22,13 @@ const labelTypes = [ const sourceStatusValues = ["current", "review_due", "outdated", "unknown"] as const; const validationStatusValues = ["unverified", "locally_reviewed", "approved"] as const; const documentScopeQueryPageSize = 1000; +// PostgREST/Supabase silently caps a single response at 1,000 rows. Label loads must +// page deterministically or later-page matches are dropped from scoped search (#075). +const labelScopeDocumentBatchSize = 200; +const labelScopeQueryPageSize = 1000; +// Hard stop so a stuck PostgREST page that always returns a full page cannot loop forever. +// 100 pages × 1,000 rows is far above realistic label volume for a 200-document batch. +const labelScopeMaxPagesPerDocumentBatch = 100; export const searchScopeFiltersSchema = z .object({ @@ -164,6 +171,45 @@ function labelMatches(labels: ScopeLabelRow[], type: DocumentLabelType, requeste return labels.some((label) => label.label_type === type && wanted.has(normalizeFilterText(label.label))); } +async function loadScopeLabels(args: { + supabase: SupabaseClient; + candidateIds: string[]; + signal?: AbortSignal; +}): Promise { + const rows: ScopeLabelRow[] = []; + + for (let start = 0; start < args.candidateIds.length; start += labelScopeDocumentBatchSize) { + const documentIdBatch = args.candidateIds.slice(start, start + labelScopeDocumentBatchSize); + for (let offset = 0, pageIndex = 0; ; offset += labelScopeQueryPageSize, pageIndex += 1) { + if (pageIndex >= labelScopeMaxPagesPerDocumentBatch) { + throw new Error( + `Scope label enumeration exceeded ${labelScopeMaxPagesPerDocumentBatch * labelScopeQueryPageSize} rows for a ${documentIdBatch.length}-document batch; narrow the filters.`, + ); + } + let labelQuery = args.supabase + .from("document_labels") + .select("id,document_id,label,label_type") + .in("document_id", documentIdBatch) + .in("label_type", [...labelTypes]) + // Stable total order so LIMIT/OFFSET pages neither skip nor duplicate rows. + .order("document_id", { ascending: true }) + .order("label_type", { ascending: true }) + .order("label", { ascending: true }) + .order("id", { ascending: true }) + .range(offset, offset + labelScopeQueryPageSize - 1); + if (args.signal) labelQuery = labelQuery.abortSignal(args.signal); + + const { data, error } = await labelQuery; + if (error) throw new Error(error.message); + const page = (data ?? []) as ScopeLabelRow[]; + rows.push(...page); + if (page.length < labelScopeQueryPageSize) break; + } + } + + return rows; +} + function isLocalSource(metadata: ClinicalSourceMetadata) { const jurisdiction = `${metadata.jurisdiction ?? ""} ${metadata.publisher ?? ""}`.toLowerCase(); return /\b(?:wa|western australia|north metropolitan|east metropolitan|south metropolitan|perth|health service)\b/.test( @@ -327,24 +373,10 @@ export async function resolveSearchScope(args: { hasValues(filters.labelTypesAny); let labelsByDocument = new Map(); if (needsLabels) { + const labelRows = await loadScopeLabels({ supabase: args.supabase, candidateIds, signal: args.signal }); labelsByDocument = new Map(); - for (let offset = 0; ; offset += documentScopeQueryPageSize) { - let labelQuery = args.supabase - .from("document_labels") - .select("id,document_id,label,label_type") - .in("document_id", candidateIds) - .in("label_type", [...labelTypes]) - // `id` is unique, so range pages cannot silently skip or repeat rows - // when PostgREST applies its default 1,000-row response cap. - .order("id", { ascending: true }) - .range(offset, offset + documentScopeQueryPageSize - 1); - if (args.signal) labelQuery = labelQuery.abortSignal(args.signal); - const { data: labelRows, error: labelError } = await labelQuery; - if (labelError) throw new Error(labelError.message); - for (const label of (labelRows ?? []) as ScopeLabelRow[]) { - labelsByDocument.set(label.document_id, [...(labelsByDocument.get(label.document_id) ?? []), label]); - } - if ((labelRows ?? []).length < documentScopeQueryPageSize) break; + for (const label of labelRows) { + labelsByDocument.set(label.document_id, [...(labelsByDocument.get(label.document_id) ?? []), label]); } } diff --git a/tests/eval-document-matching.test.ts b/tests/eval-document-matching.test.ts new file mode 100644 index 00000000..55090066 --- /dev/null +++ b/tests/eval-document-matching.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { + documentExpectationAlternatives, + expectedFileCoverage, + normalizedDocumentName, +} from "@/lib/eval-document-matching"; + +describe("eval document matching wide-tier aliases", () => { + it("does not let one dual-listed admission-to-discharge doc satisfy both comparison slots", () => { + const dualListedDoc = { + title: "Admission to Discharge for Mental Health Inpatients", + file_name: "Admission to Discharge for Mental Health Inpatients (NMHS).pdf", + }; + + const coverage = expectedFileCoverage(["MHSP.AdmissionCommunityPts.pdf", "MHSP.Discharge.pdf"], [dualListedDoc], 5); + + // A single retrieved source may hit Discharge, but must not make allHit true + // by also filling the Admission slot via overlapping wide-tier aliases. + expect(coverage.allHit).toBe(false); + expect(coverage.matchedFiles).toEqual(["MHSP.Discharge.pdf"]); + expect(coverage.missingFiles).toEqual(["MHSP.AdmissionCommunityPts.pdf"]); + }); + + it("keeps AdmissionCommunityPts and Discharge wide-tier alias values disjoint", () => { + const admission = new Set( + documentExpectationAlternatives("MHSP.AdmissionCommunityPts.pdf").filter( + (name) => name !== normalizedDocumentName("MHSP.AdmissionCommunityPts.pdf"), + ), + ); + const discharge = documentExpectationAlternatives("MHSP.Discharge.pdf").filter( + (name) => name !== normalizedDocumentName("MHSP.Discharge.pdf"), + ); + + expect(discharge.filter((name) => admission.has(name))).toEqual([]); + }); + + it("still matches admission-only and discharge-only documents on their own slots", () => { + expect( + expectedFileCoverage( + ["MHSP.AdmissionCommunityPts.pdf", "MHSP.Discharge.pdf"], + [ + { + title: "Admission of Community Patients", + file_name: "Admission of Community Patients (NMHS).pdf", + }, + { + title: "Discharge Planning", + file_name: "Discharge Planning for Community Patients.pdf", + }, + ], + 5, + ), + ).toMatchObject({ + matchedFiles: ["MHSP.AdmissionCommunityPts.pdf", "MHSP.Discharge.pdf"], + missingFiles: [], + allHit: true, + }); + }); + + it("requires distinct retrieved sources for multi-slot allHit even when one title matches both aliases", () => { + const coverage = expectedFileCoverage( + ["MHSP.AdmissionCommunityPts.pdf", "MHSP.Discharge.pdf"], + [ + { + title: "Admission of Community Patients and Discharge Planning", + file_name: "Admission of Community Patients and Discharge Planning.pdf", + }, + ], + 5, + ); + + expect(coverage.allHit).toBe(false); + expect(coverage.anyHit).toBe(true); + expect(coverage.matchedFiles).toHaveLength(1); + expect(coverage.missingFiles).toHaveLength(1); + }); +}); diff --git a/tests/search-scope.test.ts b/tests/search-scope.test.ts index 3ea34f25..07c3fc13 100644 --- a/tests/search-scope.test.ts +++ b/tests/search-scope.test.ts @@ -1,6 +1,84 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { activeScopeFilterCount, resolveSearchScope, searchScopeFiltersSchema } from "@/lib/search-scope"; +type QueryCall = { + table: string; + selected?: string; + range?: { from: number; to: number }; + filters: Array<{ column: string; value: unknown }>; + inFilters: Array<{ column: string; values: unknown[] }>; + orders: string[]; + abortSignals: AbortSignal[]; +}; + +type QueryResult = { data: unknown[]; error: { message: string } | null }; +type QueryResolver = (call: QueryCall) => QueryResult; + +class QueryBuilder implements PromiseLike { + constructor( + private readonly call: QueryCall, + private readonly resolver: QueryResolver, + ) {} + + select(selected: string) { + this.call.selected = selected; + return this; + } + + eq(column: string, value: unknown) { + this.call.filters.push({ column, value }); + return this; + } + + is(column: string, value: unknown) { + this.call.filters.push({ column, value }); + return this; + } + + or() { + return this; + } + + in(column: string, values: unknown[]) { + this.call.inFilters.push({ column, values }); + return this; + } + + order(column: string) { + this.call.orders.push(column); + return this; + } + + range(from: number, to: number) { + this.call.range = { from, to }; + return this; + } + + abortSignal(signal: AbortSignal) { + this.call.abortSignals.push(signal); + return this; + } + + then( + onfulfilled?: ((value: QueryResult) => TResult1 | PromiseLike) | null, + onrejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, + ): PromiseLike { + return Promise.resolve(this.resolver(this.call)).then(onfulfilled, onrejected); + } +} + +function supabaseMock(resolver: QueryResolver) { + const calls: QueryCall[] = []; + return { + calls, + from: vi.fn((table: string) => { + const call: QueryCall = { table, filters: [], inFilters: [], orders: [], abortSignals: [] }; + calls.push(call); + return new QueryBuilder(call, resolver); + }), + }; +} + describe("search scope filters", () => { it("paginates label enumeration beyond the PostgREST 1,000-row cap", async () => { const documentIds = ["00000000-0000-0000-0000-000000000001", "00000000-0000-0000-0000-000000000002"]; @@ -53,8 +131,15 @@ describe("search scope filters", () => { ["document_labels", 0, 999], ["document_labels", 1_000, 1_999], ]); + // loadScopeLabels uses a stable composite order, then id, on every page. expect(requestedOrders.filter(([table]) => table === "document_labels")).toEqual([ + ["document_labels", "document_id", true], + ["document_labels", "label_type", true], + ["document_labels", "label", true], ["document_labels", "id", true], + ["document_labels", "document_id", true], + ["document_labels", "label_type", true], + ["document_labels", "label", true], ["document_labels", "id", true], ]); }); @@ -112,4 +197,166 @@ describe("search scope filters", () => { summary: "All public documents", }); }); + + it("paginates label rows so later-page label matches are not silently dropped", async () => { + const wantedDocumentId = "22222222-2222-4222-8222-222222222222"; + const supabase = supabaseMock((call) => { + if (call.table === "documents") { + return { + data: [ + { id: "11111111-1111-4111-8111-111111111111", metadata: {}, import_batch_id: null }, + { id: wantedDocumentId, metadata: {}, import_batch_id: null }, + ], + error: null, + }; + } + if (call.table === "document_labels") { + // Reproduce the Supabase 1,000-row response cap: page 0 is full, page 1 holds the match. + if (call.range?.from === 0) { + return { + data: Array.from({ length: 1000 }, (_, index) => ({ + id: `label-${index.toString().padStart(4, "0")}`, + document_id: "11111111-1111-4111-8111-111111111111", + label: "other topic", + label_type: "topic", + })), + error: null, + }; + } + return { + data: [ + { + id: "label-wanted", + document_id: wantedDocumentId, + label: "clozapine", + label_type: "topic", + }, + ], + error: null, + }; + } + return { data: [], error: null }; + }); + + await expect( + resolveSearchScope({ + supabase: supabase as never, + accessScope: { ownerId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", includePublic: false }, + filters: { topics: ["clozapine"] }, + }), + ).resolves.toMatchObject({ + documentIds: [wantedDocumentId], + matchedDocumentCount: 1, + }); + + const labelCalls = supabase.calls.filter((call) => call.table === "document_labels"); + expect(labelCalls.map((call) => call.range)).toEqual([ + { from: 0, to: 999 }, + { from: 1000, to: 1999 }, + ]); + expect(labelCalls.every((call) => call.orders.includes("id"))).toBe(true); + expect(labelCalls.every((call) => call.selected?.includes("id"))).toBe(true); + }); + + it("enumerates more than 1,000 distinct labels across pages without truncation", async () => { + const documentIds = Array.from({ length: 3 }, (_, index) => `doc-${index.toString().padStart(4, "0")}`); + const allLabels = Array.from({ length: 1001 }, (_, index) => ({ + id: `label-${index.toString().padStart(4, "0")}`, + document_id: documentIds[index % documentIds.length]!, + label: `topic-${index.toString().padStart(4, "0")}`, + label_type: "topic" as const, + })); + const wantedLabel = allLabels[1000]!; + + const supabase = supabaseMock((call) => { + if (call.table === "documents") { + return { + data: documentIds.map((id) => ({ id, metadata: {}, import_batch_id: null })), + error: null, + }; + } + if (call.table === "document_labels") { + const from = call.range?.from ?? 0; + const to = call.range?.to ?? from; + return { + data: allLabels.slice(from, to + 1), + error: null, + }; + } + return { data: [], error: null }; + }); + + await expect( + resolveSearchScope({ + supabase: supabase as never, + accessScope: { ownerId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", includePublic: false }, + filters: { topics: [wantedLabel.label] }, + }), + ).resolves.toMatchObject({ + documentIds: [wantedLabel.document_id], + matchedDocumentCount: 1, + }); + + const labelCalls = supabase.calls.filter((call) => call.table === "document_labels"); + expect(labelCalls).toHaveLength(2); + expect(labelCalls.map((call) => call.range)).toEqual([ + { from: 0, to: 999 }, + { from: 1000, to: 1999 }, + ]); + }); + + it("propagates caller cancellation to label scope queries", async () => { + const controller = new AbortController(); + const supabase = supabaseMock((call) => { + if (call.table === "documents") { + return { + data: [{ id: "11111111-1111-4111-8111-111111111111", metadata: {}, import_batch_id: null }], + error: null, + }; + } + return { data: [], error: null }; + }); + + await resolveSearchScope({ + supabase: supabase as never, + accessScope: { ownerId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", includePublic: false }, + filters: { labelTypesAny: ["topic"] }, + signal: controller.signal, + }); + + const labelCall = supabase.calls.find((call) => call.table === "document_labels"); + expect(labelCall?.abortSignals).toContain(controller.signal); + }); + + it("fails closed when label pagination exceeds the bounded page budget", async () => { + const supabase = supabaseMock((call) => { + if (call.table === "documents") { + return { + data: [{ id: "11111111-1111-4111-8111-111111111111", metadata: {}, import_batch_id: null }], + error: null, + }; + } + if (call.table === "document_labels") { + // Always-full pages simulate a stuck API that would otherwise loop forever. + return { + data: Array.from({ length: 1000 }, (_, index) => ({ + id: `label-${(call.range?.from ?? 0) + index}`, + document_id: "11111111-1111-4111-8111-111111111111", + label: "topic", + label_type: "topic", + })), + error: null, + }; + } + return { data: [], error: null }; + }); + + await expect( + resolveSearchScope({ + supabase: supabase as never, + accessScope: { ownerId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", includePublic: false }, + filters: { topics: ["topic"] }, + }), + ).rejects.toThrow(/exceeded .* rows for a 1-document batch/i); + }); });