diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index a0ae562675..c986aaa00b 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -1262,3 +1262,5 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-29 | claude/latency-fixes-2026-07-29 | 0e30215e98328513e23342a8dceafc9d1728bf9d | pr-1376-ci-bugbot-repair | fixed-p1s-plus-followups;owner-scoped-epochs;shared-cache-race;stream-signal;empty-scope-timing;threads-resolved | vitest:15-pass;docs:check-links:pass;verify:cheap:earlier-pass | | 2026-07-29 | codex/remove-source-overlays | a08a81d320c9f8e1bbbe1facc266d8257213b1ad | PR #1378 babysit | FIXED Codex P1s (restore governance notice); overlays/Preview removed; merged main; verify:cheap PASS; Bugbot no open findings | verify:cheap 4273 pass; focused DOM 4/4; eslint/tsc/build PASS; hosted CI re-running after main sync | | 2026-07-29 | claude/clinical-design-system-update-e34ca9 | 0cdae091ad92f40e0ad7335b3e2d396c44188a4f | PR #1375 conflict fix + Bugbot | FIXED second CONFLICTING after #1378: took main removal of SelectedDocumentEvidencePanel; retained tracking-eyebrow on surviving document-search-results. Prior DocumentViewerRail + form-detail settlement retained. MERGEABLE; CI re-running. | local: document-search-record-fault + design-token tests; merge-tree CLEAN; prior Production UI PASS on 6903f51f; form-detail e2e 2/2. | +| 2026-07-29 | claude/test-coverage-analysis-2vcd8a | 0922d7f56624ef84be8abcb2bbc89205027cf9a6 | PR #1383 babysit | BLOCKER CLEARED: merged origin/main; renumbered coverage follow-ups #098/#099 -> #106/#107 (main claimed #098-#105). Before: CONFLICTING/DIRTY, 4 behind; CI green on prior tip; 0 review threads; 0 Bugbot findings. After: mergeable expected; verify:cheap 424 files/4371 passed; test:coverage exit 0; format:changed + check:rag:fixtures pass. | verify:cheap PASS (424 files, 4371 passed \| 4 skipped); test:coverage PASS (no threshold errors); format:changed PASS; check:rag:fixtures PASS (36 golden); Bugbot: no findings; no provider-backed checks | +| 2026-07-29 | claude/test-coverage-analysis-2vcd8a | 6f476b5f741627cb622af57d1b4665e3989789ca | PR #1383 babysit | CLOSEOUT at tip after ledger bookkeeping commit. Merge conflict cleared; coverage follow-ups live as #106/#107; local gates green; awaiting hosted CI on tip. | same as prior tip 0922d7f5 plus ledger append only; no product code change | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 75c164a027..d72da481b6 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -84,7 +84,7 @@ removed after current-main verification; it is not missing recommended work. | 36 | `#101` | A3 | Specialist — retrieval/ranking | Only after `#098` sizes the win | 1–2 days plus canary | Outcome: independent retrieval stages stop running serially. Gate: live eval-canary pair, explicit approval, ~$1–2. Verification: 36/36 golden, document/content recall 1.0, zero per-case rr regressions. Stop on any regression and revert in a single commit. Resolved `#075` and `#083` are the precedents for why this is gated rather than free. | | 37 | `#104` | A3 | Standard — ingestion worker | Any ingestion-touching session | 30–60 minutes | Outcome: ingestion reads each extracted image once instead of up to three times (`worker/main.ts:997`). Gate: `verify:cheap`. Verification: targeted worker test plus one ingestion smoke run. Throughput only — no clinician-facing latency, so do not prioritise it above anything above. Stop if the read is load-bearing for OCR retry semantics. | - + ## Open items @@ -147,6 +147,8 @@ removed after current-main verification; it is not missing recommended work. | #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 either add it to `schema.sql` or record it in `supabase/drift-allowlist.json` with the reason. 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-28 | | #104 | P3 | task | Worker reads each ingestion image up to three times | **Outcome:** ingestion reads each extracted image once. `worker/main.ts:997` `readFile`s the same image up to 3x per document — carried forward from the 2026-07-01 audit as finding `L11`, still CONFIRMED with no fix evidence. Ingestion throughput only; no clinician-facing latency, which is why it stays P3. **Next:** read once into a buffer and pass it through the caption/embed/store path. | `docs/audit/repo-audit-2026-07-01.md` L11; `docs/audit/latency-audit-2026-07-28.md` L4-2 | 2026-07-28 | | #105 | P3 | task | `#017`-exempt client latency wins | **Outcome:** zero-payload client latency fixes are not trapped behind the `#017` measurement gate. `#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. **Done 2026-07-28:** 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/`. **Next:** verify with `verify:ui` once the heavy-run lock is free. Sidebar dialogs intentionally excluded (they mount on open). | `docs/audit/latency-audit-2026-07-28.md` L3-4/L3-5; `src/components/clinical-dashboard/clinical-dashboard-lazy.tsx` | 2026-07-28 | +| #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 | ## Resolved / archive diff --git a/tests/calculator-scoring.test.ts b/tests/calculator-scoring.test.ts new file mode 100644 index 0000000000..bc0f6c4963 --- /dev/null +++ b/tests/calculator-scoring.test.ts @@ -0,0 +1,339 @@ +import { describe, expect, it } from "vitest"; + +import { calculators, type CalculatorFixture, type CalculatorItem } from "@/components/calculators/calculator-fixtures"; +import { + deriveCalculator, + formatResultSummary, + itemScore, + progressLabel, + type AnswerMap, +} from "@/components/calculators/calculator-ui"; + +/* + * Scoring guard for the eight validated instruments in the calculators mode. + * + * `calculator-ui.tsx` held the band lookup, the MDQ three-criterion rule and the + * band-suppression rules with no executing test — the only calculator test in the + * repo asserted mockup import boundaries. A wrong band is a clinical + * misclassification that renders as a normal result, so the floor belongs here + * rather than in a Playwright journey: these are pure functions, so the whole + * file costs about as much as one browser navigation. + * + * Everything below drives the exported pure functions only. No DOM, no mocks. + */ + +function fixture(id: string): CalculatorFixture { + const calc = calculators.find((entry) => entry.id === id); + if (!calc) throw new Error(`calculator fixture "${id}" not found`); + return calc; +} + +function lowestOptionIndex(item: CalculatorItem): number { + const options = item.options ?? []; + let lowest = 0; + for (let index = 1; index < options.length; index += 1) { + if (options[index].points < options[lowest].points) lowest = index; + } + return lowest; +} + +/** Every item at its floor: the minimum score the instrument can report. */ +function baselineAnswers(calc: CalculatorFixture): AnswerMap { + const answers: AnswerMap = {}; + for (const item of calc.items) { + answers[item.id] = item.kind === "options" ? lowestOptionIndex(item) : 0; + } + return answers; +} + +/** Every item at its ceiling: the maximum score the instrument can report. */ +function maximalAnswers(calc: CalculatorFixture): AnswerMap { + const answers: AnswerMap = {}; + for (const item of calc.items) { + if (item.kind !== "options") { + answers[item.id] = 1; + continue; + } + const options = item.options ?? []; + let highest = 0; + for (let index = 1; index < options.length; index += 1) { + if (options[index].points > options[highest].points) highest = index; + } + answers[item.id] = highest; + } + return answers; +} + +/** + * Build a complete answer map scoring exactly `target`. Starts from the floor and + * raises one item at a time. Every caller asserts the resulting score, so a helper + * that cannot reach the target fails loudly instead of weakening the assertion. + */ +function answersForScore(calc: CalculatorFixture, target: number): AnswerMap { + const answers = baselineAnswers(calc); + let remaining = target - deriveCalculator(calc, answers).score; + + for (const item of calc.items) { + if (remaining <= 0) break; + if (item.kind === "options") { + const options = item.options ?? []; + const currentIndex = answers[item.id] ?? 0; + const currentPoints = options[currentIndex]?.points ?? 0; + let best = currentIndex; + for (let index = 0; index < options.length; index += 1) { + const delta = options[index].points - currentPoints; + if (delta > 0 && delta <= remaining && options[index].points > options[best].points) best = index; + } + remaining -= (options[best]?.points ?? 0) - currentPoints; + answers[item.id] = best; + } else { + const points = item.points ?? 0; + if (points > 0 && points <= remaining) { + answers[item.id] = 1; + remaining -= points; + } + } + } + + return answers; +} + +describe("calculator band tables", () => { + it.each(calculators.map((calc) => [calc.id, calc] as const))( + "%s declares contiguous bands spanning its full score range", + (_id, calc) => { + const bands = [...calc.bands].sort((left, right) => left.min - right.min); + + expect(bands.length).toBeGreaterThan(0); + expect(bands[0].min).toBe(calc.minScore); + expect(bands[bands.length - 1].max).toBe(calc.maxScore); + + for (let index = 1; index < bands.length; index += 1) { + // A gap leaves a score with no band ("—"); an overlap makes the + // reported severity depend on declaration order. + expect(bands[index].min, `band ${index} of ${calc.id} must start one above the previous band`).toBe( + bands[index - 1].max + 1, + ); + } + }, + ); + + it.each(calculators.map((calc) => [calc.id, calc] as const))( + "%s maps every reachable score to exactly one band", + (_id, calc) => { + for (let score = calc.minScore; score <= calc.maxScore; score += 1) { + const matches = calc.bands.filter((band) => score >= band.min && score <= band.max); + expect(matches, `${calc.id} score ${score} matched ${matches.length} bands`).toHaveLength(1); + } + }, + ); + + it.each(calculators.map((calc) => [calc.id, calc] as const))( + "%s items can actually reach its declared score range", + (_id, calc) => { + // A declared range the items cannot produce means either an unreachable + // band or a score that falls off the end of the table. + expect(deriveCalculator(calc, baselineAnswers(calc)).score).toBe(calc.minScore); + expect(deriveCalculator(calc, maximalAnswers(calc)).score).toBe(calc.maxScore); + }, + ); +}); + +describe("itemScore", () => { + const checkbox: CalculatorItem = { id: "x", text: "x", kind: "checkbox", points: 3 }; + const options: CalculatorItem = { + id: "y", + text: "y", + kind: "options", + options: [ + { label: "None", short: "None", points: 0 }, + { label: "Some", short: "Some", points: 2 }, + ], + }; + + it("scores an unanswered item as zero", () => { + expect(itemScore(checkbox, undefined)).toBe(0); + expect(itemScore(options, undefined)).toBe(0); + }); + + it("scores a checkbox only when explicitly ticked", () => { + expect(itemScore(checkbox, 1)).toBe(3); + expect(itemScore(checkbox, 0)).toBe(0); + }); + + it("scores an options item by the selected option's points", () => { + expect(itemScore(options, 0)).toBe(0); + expect(itemScore(options, 1)).toBe(2); + }); + + it("treats an out-of-range option index as zero rather than NaN", () => { + expect(itemScore(options, 7)).toBe(0); + }); + + it("treats a checkbox with no declared points as zero", () => { + expect(itemScore({ id: "z", text: "z", kind: "checkbox" }, 1)).toBe(0); + }); +}); + +describe("PHQ-9 severity banding", () => { + const phq9 = fixture("phq9"); + + // Boundary scores on both sides of every published PHQ-9 cut point. These map + // to stepped treatment actions, so an off-by-one here changes clinical advice. + const boundaries = [ + [0, "Minimal"], + [4, "Minimal"], + [5, "Mild"], + [9, "Mild"], + [10, "Moderate"], + [14, "Moderate"], + [15, "Moderately severe"], + [19, "Moderately severe"], + [20, "Severe"], + [27, "Severe"], + ] as const; + + it.each(boundaries)("scores %i as %s", (score, label) => { + const state = deriveCalculator(phq9, answersForScore(phq9, score)); + expect(state.score).toBe(score); + expect(state.band?.label).toBe(label); + expect(state.result.label).toBe(label); + }); + + it("raises the item-9 self-harm flag only when that item scores above zero", () => { + const flagged = deriveCalculator(phq9, { ...baselineAnswers(phq9), p9: 1 }); + expect(flagged.flags).toEqual(["Item 9 endorsed — complete a structured suicide-risk assessment now."]); + + const unflagged = deriveCalculator(phq9, { ...baselineAnswers(phq9), p9: 0 }); + expect(unflagged.flags).toEqual([]); + }); + + it("publishes a provisional band while a zero-floor scale is still filling in", () => { + const partial = deriveCalculator(phq9, { p1: 3, p2: 3 }); + expect(partial.complete).toBe(false); + expect(partial.started).toBe(true); + expect(partial.band?.label).toBe("Mild"); + }); +}); + +describe("GAD-7 severity banding", () => { + const gad7 = fixture("gad7"); + + it.each([ + [0, "Minimal"], + [4, "Minimal"], + [5, "Mild"], + [9, "Mild"], + [10, "Moderate"], + [14, "Moderate"], + [15, "Severe"], + [21, "Severe"], + ] as const)("scores %i as %s", (score, label) => { + const state = deriveCalculator(gad7, answersForScore(gad7, score)); + expect(state.score).toBe(score); + expect(state.band?.label).toBe(label); + }); +}); + +describe("band suppression for scales that cannot read zero", () => { + it("withholds a K10 band until every item is answered", () => { + const k10 = fixture("k10"); + // K10 floors at 10, so a part-filled scale would otherwise fall below the + // table and read as the lowest band on incomplete data. + const partial = deriveCalculator(k10, { k1: 4, k2: 4 }); + expect(partial.complete).toBe(false); + expect(partial.band).toBeUndefined(); + expect(partial.result.label).toBe("—"); + + const complete = deriveCalculator(k10, answersForScore(k10, 30)); + expect(complete.complete).toBe(true); + expect(complete.score).toBe(30); + expect(complete.band?.label).toBe("Very high"); + }); + + it("withholds a CAGE band until every yes/no item is recorded", () => { + const cage = fixture("cage"); + // A half-ticked checkbox screen still has undefined items; reading it as + // "low risk" would be a false negative. + const partial = deriveCalculator(cage, { c1: 0, c2: 0 }); + expect(partial.complete).toBe(false); + expect(partial.band).toBeUndefined(); + + const allNegative = deriveCalculator(cage, { c1: 0, c2: 0, c3: 0, c4: 0 }); + expect(allNegative.complete).toBe(true); + expect(allNegative.score).toBe(0); + expect(allNegative.band?.label).toBeDefined(); + }); +}); + +describe("MDQ three-criterion screen", () => { + const mdq = fixture("mdq"); + + /** `count` symptom items endorsed, plus the two criterion items. */ + function mdqAnswers(count: number, extras: AnswerMap = {}): AnswerMap { + const answers: AnswerMap = {}; + for (let index = 1; index <= 13; index += 1) { + answers[`m${index}`] = index <= count ? 1 : 0; + } + return { ...answers, ...extras }; + } + + it("returns a positive screen only when all three criteria are met", () => { + const state = deriveCalculator(mdq, mdqAnswers(7, { mco: 1, mimp: 2 })); + expect(state.score).toBe(7); + expect(state.result.label).toBe("Positive screen"); + expect(state.result.tone).toBe("danger"); + }); + + it("withholds a positive screen when co-occurrence is not confirmed", () => { + const state = deriveCalculator(mdq, mdqAnswers(7, { mco: 0, mimp: 3 })); + expect(state.result.label).toBe("Symptom threshold met"); + expect(state.result.guidance).toContain("co-occurrence"); + }); + + it("withholds a positive screen when impairment is below moderate", () => { + const state = deriveCalculator(mdq, mdqAnswers(9, { mco: 1, mimp: 1 })); + expect(state.result.label).toBe("Symptom threshold met"); + expect(state.result.guidance).toContain("moderate-or-serious impairment"); + }); + + it("names both unmet criteria when neither is confirmed", () => { + const state = deriveCalculator(mdq, mdqAnswers(13, { mco: 0, mimp: 0 })); + expect(state.result.guidance).toContain("co-occurrence and moderate-or-serious impairment"); + }); + + it("returns a negative screen one symptom below the threshold", () => { + const state = deriveCalculator(mdq, mdqAnswers(6, { mco: 1, mimp: 3 })); + expect(state.score).toBe(6); + expect(state.result.label).toBe("Negative screen"); + expect(state.result.tone).toBe("success"); + }); + + it("scores the criterion items as zero points", () => { + // `mco` and `mimp` record state but must never inflate the symptom count. + const withCriteria = deriveCalculator(mdq, mdqAnswers(7, { mco: 1, mimp: 3 })); + const withoutCriteria = deriveCalculator(mdq, mdqAnswers(7, { mco: 0, mimp: 0 })); + expect(withCriteria.score).toBe(withoutCriteria.score); + }); +}); + +describe("result summary text", () => { + const phq9 = fixture("phq9"); + + it("reports score, maximum and band once the scale is complete", () => { + const state = deriveCalculator(phq9, answersForScore(phq9, 12)); + expect(formatResultSummary(phq9, state)).toBe("PHQ-9 12/27 — Moderate"); + }); + + it("appends progress while the scale is incomplete", () => { + const state = deriveCalculator(phq9, { p1: 3, p2: 3 }); + expect(state.complete).toBe(false); + expect(formatResultSummary(phq9, state)).toBe("PHQ-9 6/27 — Mild (2 of 9 answered)"); + }); + + it("counts endorsements rather than answers for checkbox-only scales", () => { + const cage = fixture("cage"); + const state = deriveCalculator(cage, { c1: 1, c2: 1, c3: 0, c4: 0 }); + expect(progressLabel(state)).toBe("2 of 4 endorsed"); + }); +}); diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index 863ad2d83c..11f2e67735 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -221,6 +221,13 @@ function createSupabaseMock(resolve: QueryResolver = defaultQueryResolver) { data: { signedUrl: `https://signed.local/${path}` }, error: null, })); + // Batch sibling of `createSignedUrl`, used by /api/images/signed-urls. Supabase + // returns one entry per requested path, so the route matches results back to + // images by path rather than by index. + const createSignedUrls = vi.fn(async (paths: string[]) => ({ + data: paths.map((path) => ({ path, signedUrl: `https://signed.local/${path}`, error: null })), + error: null, + })); const remove = vi.fn( async ( ...args: [string[]] @@ -232,7 +239,7 @@ function createSupabaseMock(resolve: QueryResolver = defaultQueryResolver) { return { data: [], error: null }; }, ); - const storageFrom = vi.fn(() => ({ upload, createSignedUrl, remove })); + const storageFrom = vi.fn(() => ({ upload, createSignedUrl, createSignedUrls, remove })); const getUser = vi.fn(async (receivedToken?: string) => receivedToken === token ? { data: { user: { id: userId, app_metadata: { site_role: "administrator" } } }, error: null } @@ -351,7 +358,7 @@ function createSupabaseMock(resolve: QueryResolver = defaultQueryResolver) { }), rpc, storage: { from: storageFrom }, - storageMocks: { upload, createSignedUrl, remove, storageFrom }, + storageMocks: { upload, createSignedUrl, createSignedUrls, remove, storageFrom }, }; return client; @@ -1227,6 +1234,196 @@ describe("private document API access", () => { expect(client.storageMocks.createSignedUrl).not.toHaveBeenCalled(); }); + /* + * Batch sibling of the five `/api/images/[id]/signed-url` cases above. + * `/api/images/signed-urls` mints up to 100 signed URLs per call and carries its + * OWN owner-scope and committed-generation implementation. `check:owner-scope-api` + * cannot cover the real protection here: `document_images` has no `owner_id` + * column, so the only thing keeping another owner's image out of the response is + * the `documentMap` join filter in the handler. These cases pin that filter. + */ + const otherImageId = "44444444-4444-4444-8444-444444444444"; + + function signedUrlsRequest(imageIds: string[], authenticated = true) { + const init: RequestInit = { + method: "POST", + body: JSON.stringify({ imageIds }), + headers: { "content-type": "application/json" }, + }; + return authenticated + ? authenticatedRequest("/api/images/signed-urls", init) + : request("/api/images/signed-urls", init); + } + + /** Two images on two documents with different owners, plus owner-scoped document reads. */ + function createBatchImageMock(options: { imageGeneration?: string; documentGeneration?: string } = {}) { + const imageGeneration = options.imageGeneration ?? "generation-a"; + const documentGeneration = options.documentGeneration ?? "generation-a"; + return createSupabaseMock((call) => { + if (call.table === "document_images") { + const requested = call.inFilters.find((filter) => filter.column === "id")?.values ?? []; + return ok( + [ + { + id: imageId, + document_id: documentId, + storage_path: `${userId}/images/${imageId}.png`, + mime_type: "image/png", + caption: "Owned image", + metadata: { index_generation_id: imageGeneration }, + }, + { + id: otherImageId, + document_id: otherDocumentId, + storage_path: `${otherUserId}/images/${otherImageId}.png`, + mime_type: "image/png", + caption: "Other owner's image", + metadata: { index_generation_id: imageGeneration }, + }, + ].filter((image) => requested.includes(image.id)), + ); + } + if (call.table === "documents" && matchesOwnerReadScope(call, userId)) { + // Only the caller's own document comes back from an owner-scoped read. + return ok([{ id: documentId, metadata: { index_generation_id: documentGeneration } }]); + } + return ok([]); + }); + } + + it("signs a batch of images whose parent documents are owned", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([imageId])); + const body = await payload(response); + + expect(response.status).toBe(200); + expect(body.urls).toMatchObject({ + [imageId]: { url: `https://signed.local/${userId}/images/${imageId}.png`, mimeType: "image/png" }, + }); + expect(client.storageMocks.createSignedUrls).toHaveBeenCalledWith([`${userId}/images/${imageId}.png`], 600); + }); + + it("omits images whose parent document belongs to another user", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([otherImageId])); + const body = await payload(response); + + expect(response.status).toBe(200); + expect(body.urls).toEqual({}); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + + it("signs only the owned image when a batch mixes owners", async () => { + // The batch-specific leak: one unowned id riding along with an owned one must + // not inherit the owned document's access. + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([imageId, otherImageId])); + const body = await payload(response); + + expect(response.status).toBe(200); + expect(Object.keys(body.urls as Record)).toEqual([imageId]); + expect(client.storageMocks.createSignedUrls).toHaveBeenCalledWith([`${userId}/images/${imageId}.png`], 600); + }); + + it("scopes the batch document read to the caller", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + await POST(signedUrlsRequest([imageId])); + + const documentReads = client.calls.filter((call) => call.table === "documents"); + expect(documentReads.length).toBeGreaterThan(0); + expect(documentReads.every((call) => matchesOwnerReadScope(call, userId))).toBe(true); + }); + + it("restricts an anonymous batch to public documents", async () => { + const client = createSupabaseMock((call) => { + if (call.table === "document_images") { + return ok([ + { + id: imageId, + document_id: documentId, + storage_path: `${userId}/images/${imageId}.png`, + mime_type: "image/png", + caption: "Owned image", + metadata: { index_generation_id: "generation-a" }, + }, + ]); + } + // No public document matches, so an anonymous caller gets nothing. + if (call.table === "documents" && matchesOwnerReadScope(call)) return ok([]); + return ok([]); + }); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([imageId], false)); + + expect(response.status).toBe(200); + expect(await payload(response)).toEqual({ urls: {} }); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + + it("omits batch images from an uncommitted replacement generation", async () => { + const client = createBatchImageMock({ imageGeneration: "generation-new", documentGeneration: "generation-old" }); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([imageId])); + + expect(response.status).toBe(200); + expect(await payload(response)).toEqual({ urls: {} }); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + + it("returns an empty map for an empty batch without touching storage", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest([])); + + expect(response.status).toBe(200); + expect(await payload(response)).toEqual({ urls: {} }); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + + it("rejects a batch of non-uuid image ids before querying", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const response = await POST(signedUrlsRequest(["not-a-uuid"])); + + expect(response.status).toBe(400); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + + it("rejects a batch larger than the documented maximum", async () => { + const client = createBatchImageMock(); + mockRuntime(client); + const { POST } = await import("../src/app/api/images/signed-urls/route"); + + const tooMany = Array.from( + { length: 101 }, + (_, index) => `55555555-5555-4555-8555-${String(index).padStart(12, "0")}`, + ); + const response = await POST(signedUrlsRequest(tooMany)); + + expect(response.status).toBe(400); + expect(client.storageMocks.createSignedUrls).not.toHaveBeenCalled(); + }); + it("rejects anonymous uploads without touching storage", async () => { const client = createSupabaseMock(); mockRuntime(client); diff --git a/vitest.config.mts b/vitest.config.mts index e7d5347a67..96d949689c 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -27,11 +27,18 @@ const config = { include: ["src/**/*.{ts,tsx}", "scripts/**/*.{ts,mjs,cjs}", "worker/**/*.ts", "supabase/functions/**/*.ts"], exclude: ["src/lib/supabase/database.types.ts"], thresholds: { + // Broad regression floor. Re-ratcheted 2026-07-29: the previous values + // (48/38/43/50) had drifted 14-17pp below measured coverage + // (63.99/55.29/57.6/66.19), so a change could delete a large amount of + // coverage and still pass. Each floor now sits ~2pp under measured — enough + // headroom for a PR that ships an uncovered surface, not enough to hide a + // regression. Re-measure with `npm run test:coverage` and raise these when + // the gap grows past ~5pp again; never lower them to make a red gate green. "src/{lib/**/*.ts,app/**/route.ts,components/**/*.{ts,tsx}}": { - statements: 48, - branches: 38, - functions: 43, - lines: 50, + statements: 62, + branches: 53, + functions: 55, + lines: 64, }, // Aggregate behavioral floors ratchet the full post-fixture group rather // than making individual large RAG modules brittle. Each value is the