From c402e23656166326f1b1aca1f7e4f1f61af4fb52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 14:06:43 +0200 Subject: [PATCH] fix(mcp): say when a tool's list was cut get_ecg_recordings stopped at 200, get_visits at 50, the booked appointments in get_preventive_care at 25, and search at its per-source scan, all without a word, so a model read the last row as the oldest there is. capped() reads through listPage with the same semantics as the REST list cap: one row past the limit, truncated true only when that row exists. The ECG and visit results carry truncated and limit, the preventive-care result carries appointmentsTruncated and appointmentsLimit, and search carries truncated with the sources that were cut, on every page. Every capped read orders by its id as a tiebreaker; list-page-guard now checks capped() reads too, and accepts a distinct read ordered by its own key. --- src/__tests__/list-page-guard.test.ts | 37 +++- src/lib/mcp/__tests__/search-fetch.test.ts | 32 ++++ src/lib/mcp/__tests__/tools.test.ts | 81 +++++++- src/lib/mcp/capped.ts | 49 +++++ src/lib/mcp/tools.ts | 203 +++++++++++++-------- 5 files changed, 325 insertions(+), 77 deletions(-) create mode 100644 src/lib/mcp/capped.ts diff --git a/src/__tests__/list-page-guard.test.ts b/src/__tests__/list-page-guard.test.ts index bdd57b8fb..1865e038e 100644 --- a/src/__tests__/list-page-guard.test.ts +++ b/src/__tests__/list-page-guard.test.ts @@ -30,7 +30,12 @@ import { walkSourceFiles } from "./helpers/source-files"; const SRC = join(process.cwd(), "src"); /** The modules allowed to cut a list by hand: the helper and its primitive. */ -const LIST_MODULES = new Set(["lib/api/list-cap.ts", "lib/http/list-query.ts"]); +const LIST_MODULES = new Set([ + "lib/api/list-cap.ts", + "lib/http/list-query.ts", + // The MCP tools' name for `listPage`; its reads are checked at its callers. + "lib/mcp/capped.ts", +]); function stripComments(text: string): string { return text.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, ""); @@ -93,6 +98,16 @@ function endsInId(order: string, file: string): boolean { return decl ? literal(decl[1]) : false; } +/** + * A `distinct` read returns one row per key, so ordering by that key alone is + * already total: `distinct: ["analyte"]` with `orderBy: { analyte: "asc" }`. + */ +function ordersByDistinctKey(body: string, order: string): boolean { + const distinct = /\bdistinct\s*:\s*\[\s*["'](\w+)["']\s*\]/.exec(body); + if (!distinct) return false; + return new RegExp(`^\\{\\s*${distinct[1]}\\s*:[^,}]+\\}$`).test(order.trim()); +} + describe("capped lists go through listPage", () => { const files = walkSourceFiles(SRC, { floor: 3000 }).filter( (file) => @@ -136,6 +151,12 @@ describe("capped lists go through listPage", () => { listPage(limit, (take) => prisma.x.findMany({ where, orderBy, take }))`; const [short] = callBodies(viaVariable, "listPage"); expect(endsInId(orderByValue(short)!, viaVariable)).toBe(true); + // A distinct read is ordered whole by its own key. + const distinctRead = `listPage(limit, (take) => + prisma.x.findMany({ distinct: ["analyte"], orderBy: { analyte: "asc" }, take }))`; + const [dist] = callBodies(distinctRead, "listPage"); + expect(ordersByDistinctKey(dist, orderByValue(dist)!)).toBe(true); + expect(ordersByDistinctKey(dist, '{ createdAt: "desc" }')).toBe(false); const viaBadVariable = `const orderBy = [primaryOrder]; listPage(limit, (take) => prisma.x.findMany({ where, orderBy, take }))`; const [bad] = callBodies(viaBadVariable, "listPage"); @@ -172,16 +193,24 @@ describe("capped lists go through listPage", () => { expect(guessed).toEqual([]); }); - it("every listPage read orders by something that ends in the row id", () => { + it("every listPage or capped read orders by something that ends in the row id", () => { const unordered: string[] = []; let reads = 0; for (const f of files) { if (LIST_MODULES.has(f)) continue; const code = read(f); - for (const body of callBodies(code, "listPage")) { + for (const body of [ + ...callBodies(code, "listPage"), + ...callBodies(code, "capped"), + ]) { reads += 1; const order = orderByValue(body); - if (!order || !endsInId(order, code)) unordered.push(f); + if ( + !order || + !(endsInId(order, code) || ordersByDistinctKey(body, order)) + ) { + unordered.push(f); + } } } expect(reads).toBeGreaterThanOrEqual(18); diff --git a/src/lib/mcp/__tests__/search-fetch.test.ts b/src/lib/mcp/__tests__/search-fetch.test.ts index 63f09c7c3..1e35cc35a 100644 --- a/src/lib/mcp/__tests__/search-fetch.test.ts +++ b/src/lib/mcp/__tests__/search-fetch.test.ts @@ -53,6 +53,7 @@ import { MCP_TOOLS } from "../tools"; import { buildCoachDataInventory } from "@/lib/ai/coach/tools/inventory"; import { executeCoachTool } from "@/lib/ai/coach/tools/executor"; import { prisma } from "@/lib/db"; +import { loadRecordCandidates } from "@/lib/mcp/record-search"; import { USER_TEXT_FENCE_START, USER_TEXT_FENCE_END, @@ -134,6 +135,37 @@ describe("search", () => { expect(result.results.map((r) => r.id)).toContain("lab:LDL"); expect(result.results.some((r) => r.id.startsWith("med:"))).toBe(false); }); + it("says the search was assembled from a cut read, on every page", async () => { + vi.mocked(buildCoachDataInventory).mockResolvedValue({ + entries: [], + window: "90d", + restMode: false, + cycleEnabled: false, + } as never); + vi.mocked(prisma.medication.findMany).mockResolvedValue([]); + vi.mocked(prisma.labResult.findMany).mockResolvedValue([]); + vi.mocked(loadRecordCandidates).mockImplementationOnce( + async (_userId, _query, _origin, options) => { + options?.onTruncated?.("visit"); + return []; + }, + ); + + const cut = (await tool("search").run(CTX, { query: "cardio" })) as { + truncated: boolean; + truncatedSources?: string[]; + }; + expect(cut.truncated).toBe(true); + expect(cut.truncatedSources).toEqual(["visit"]); + + const whole = (await tool("search").run(CTX, { query: "cardio" })) as { + truncated: boolean; + truncatedSources?: string[]; + }; + expect(whole.truncated).toBe(false); + expect(whole).not.toHaveProperty("truncatedSources"); + }); + it("keeps distinct lab analytes stable across cursor pages", async () => { vi.mocked(buildCoachDataInventory).mockResolvedValue({ entries: [], diff --git a/src/lib/mcp/__tests__/tools.test.ts b/src/lib/mcp/__tests__/tools.test.ts index f736e0d92..5b9d9a8c1 100644 --- a/src/lib/mcp/__tests__/tools.test.ts +++ b/src/lib/mcp/__tests__/tools.test.ts @@ -1110,12 +1110,48 @@ describe("get_visits", () => { }; expect(arg.where.userId).toBe("user-1"); expect(arg.where.deletedAt).toBeNull(); - expect(arg.take).toBe(50); + // One past the cap of 50: the extra row only says whether more exist. + expect(arg.take).toBe(51); // The window arg narrows the range; the injected userId body field is // ignored entirely. expect(arg.where.occurredAt).toBeDefined(); }); + it("says when the visit list was cut at its cap, and only then", async () => { + const visit = (n: number) => ({ + id: `enc-${n}`, + occurredAt: new Date(Date.UTC(2026, 0, 1) - n * 86_400_000), + status: "DONE", + kind: "ROUTINE", + reasonEncrypted: null, + outcomeEncrypted: null, + bodySiteEncrypted: null, + laterality: null, + practitioner: null, + }); + vi.mocked(prisma.encounter.findMany).mockResolvedValueOnce( + Array.from({ length: 51 }, (_, n) => visit(n)) as never, + ); + const cut = (await tool("get_visits").run(CTX, {})) as { + visits: unknown[]; + truncated: boolean; + limit: number; + }; + expect(cut.visits).toHaveLength(50); + expect(cut).toMatchObject({ truncated: true, limit: 50 }); + + // Exactly full is the whole list. + vi.mocked(prisma.encounter.findMany).mockResolvedValueOnce( + Array.from({ length: 50 }, (_, n) => visit(n)) as never, + ); + const full = (await tool("get_visits").run(CTX, {})) as { + visits: unknown[]; + truncated: boolean; + }; + expect(full.visits).toHaveLength(50); + expect(full.truncated).toBe(false); + }); + it("narrows to one practitioner by a case-insensitive name substring", async () => { vi.mocked(prisma.encounter.findMany).mockResolvedValue([] as never); await tool("get_visits").run(CTX, { practitioner: "herz" }); @@ -1535,6 +1571,49 @@ describe("get_intraday_pulse — v1.30 coverage review (G2)", () => { }); }); +describe("get_ecg_recordings — the list says when it was cut", () => { + const row = (n: number) => ({ + id: `ecg-${n}`, + recordedAt: new Date(Date.UTC(2026, 6, 1) - n * 3_600_000), + durationSeconds: 30, + samplingFrequency: 512, + sampleCount: 15360, + averageHeartRate: 70, + lead: "LEAD_I", + rhythmClassification: "NOT_DETECTED", + source: "APPLE_HEALTH", + }); + + it("reads one past the cap and reports the cut exactly", async () => { + vi.mocked(prisma.ecgRecording.findMany).mockResolvedValueOnce( + Array.from({ length: 201 }, (_, n) => row(n)) as never, + ); + const cut = (await tool("get_ecg_recordings").run(CTX, {})) as { + recordings: unknown[]; + truncated: boolean; + limit: number; + }; + const call = vi + .mocked(prisma.ecgRecording.findMany) + .mock.calls.at(-1)![0] as { + take: number; + orderBy: unknown; + }; + expect(call.take).toBe(201); + expect(call.orderBy).toEqual([{ recordedAt: "desc" }, { id: "desc" }]); + expect(cut.recordings).toHaveLength(200); + expect(cut).toMatchObject({ truncated: true, limit: 200 }); + + vi.mocked(prisma.ecgRecording.findMany).mockResolvedValueOnce( + Array.from({ length: 200 }, (_, n) => row(n)) as never, + ); + const full = (await tool("get_ecg_recordings").run(CTX, {})) as { + truncated: boolean; + }; + expect(full.truncated).toBe(false); + }); +}); + describe("get_ecg_recordings — v1.30 coverage review (G3)", () => { it("returns metadata-only recordings with the device classification verbatim", async () => { vi.mocked(prisma.ecgRecording.findMany).mockResolvedValue([ diff --git a/src/lib/mcp/capped.ts b/src/lib/mcp/capped.ts new file mode 100644 index 000000000..7c5097e5c --- /dev/null +++ b/src/lib/mcp/capped.ts @@ -0,0 +1,49 @@ +/** + * A capped MCP read, and the fields that tell the model it was cut. + * + * Several tools stopped at their own ceiling and said nothing: the ECG list at + * 200, the visit history at 50, the booked appointments at 25, the record + * search at a per-kind scan. A model handed the 50 newest visits reads the + * fiftieth as the oldest there is and answers "your first cardiology visit was + * in 2024" from a list that simply ended there. + * + * Same semantics as the REST list cap (`src/lib/api/list-cap.ts`), through the + * same helper: the read asks for one row past the limit, and `truncated` is + * true only when that row exists. A tool spreads {@link capFields} into its + * result and {@link cappedOutput} into its output shape. + */ +import { z } from "zod/v4"; + +import { listPage } from "@/lib/http/list-query"; +import type { ListCap } from "@/lib/api/list-cap-shape"; + +/** Run a capped read: the rows within `limit`, and whether more exist. */ +export function capped( + limit: number, + read: (take: number) => Promise, +): Promise<{ rows: T[]; cap: ListCap }> { + return listPage(limit, read); +} + +/** The cap as tool-result fields. */ +export function capFields(cap: ListCap): { truncated: boolean; limit: number } { + return { truncated: cap.truncated, limit: cap.limit }; +} + +/** + * The output-shape fields for a capped list. Optional, because a grounded + * `{ present: false }` miss carries no list to have cut. + */ +export const cappedOutput = { + truncated: z + .boolean() + .optional() + .describe( + "True when more records exist than this result lists: the list was cut at `limit`. Do not treat the last record listed as the oldest or the only one; say the list is partial.", + ), + limit: z + .number() + .int() + .optional() + .describe("The most records this result could list."), +}; diff --git a/src/lib/mcp/tools.ts b/src/lib/mcp/tools.ts index ee6e575f0..476d8a3ed 100644 --- a/src/lib/mcp/tools.ts +++ b/src/lib/mcp/tools.ts @@ -70,6 +70,7 @@ import { import { loadIntradayPulse } from "@/lib/analytics/intraday-pulse-io"; import { DEFAULT_TIMEZONE, userDayKey } from "@/lib/tz/format"; import { encodeOffsetCursor, decodeOffsetCursor } from "@/lib/mcp/pagination"; +import { capFields, capped, cappedOutput } from "@/lib/mcp/capped"; import { loadAutoTakeRollingAnchors } from "@/lib/medications/auto-take"; import { computeDisplayDue, @@ -477,6 +478,15 @@ function searchAndFetchTools(): McpToolDefinition[] { }), ), nextCursor: z.string().optional(), + truncated: cappedOutput.truncated.describe( + "True when a source this search reads stopped at its scan ceiling, so older records of that kind were not searched and no cursor reaches them. An empty or short result is then not proof that nothing matches; narrow the query or use the kind's own tool.", + ), + truncatedSources: z + .array(z.string()) + .optional() + .describe( + "Which sources were cut: medication, lab, visit, condition, document, vaccination.", + ), }, async run(ctx, args) { const rawQuery = @@ -526,12 +536,21 @@ function searchAndFetchTools(): McpToolDefinition[] { isModuleEnabled(ctx.userId, "labs"), ]); + // Sources whose read stopped at its ceiling, so the result can say it + // was assembled from part of the record instead of reading as all of + // it. + const truncatedSources: string[] = []; const meds = medicationsOn - ? await prisma.medication.findMany({ - where: { userId: ctx.userId }, - select: { id: true, name: true, dose: true }, - orderBy: { createdAt: "desc" }, - take: SEARCH_RESULT_SCAN_CAP, + ? await capped(SEARCH_RESULT_SCAN_CAP, (take) => + prisma.medication.findMany({ + where: { userId: ctx.userId }, + select: { id: true, name: true, dose: true }, + orderBy: [{ createdAt: "desc" }, { id: "desc" }], + take, + }), + ).then(({ rows, cap }) => { + if (cap.truncated) truncatedSources.push("medication"); + return rows; }) : []; for (const med of meds) { @@ -549,12 +568,18 @@ function searchAndFetchTools(): McpToolDefinition[] { } const labs = labsOn - ? await prisma.labResult.findMany({ - where: { userId: ctx.userId, deletedAt: null }, - select: { analyte: true }, - distinct: ["analyte"], - orderBy: { analyte: "asc" }, - take: SEARCH_RESULT_SCAN_CAP, + ? await capped(SEARCH_RESULT_SCAN_CAP, (take) => + prisma.labResult.findMany({ + where: { userId: ctx.userId, deletedAt: null }, + select: { analyte: true }, + distinct: ["analyte"], + // One row per analyte, so the analyte alone is a total order. + orderBy: { analyte: "asc" }, + take, + }), + ).then(({ rows, cap }) => { + if (cap.truncated) truncatedSources.push("lab"); + return rows; }) : []; for (const lab of labs) { @@ -636,7 +661,9 @@ function searchAndFetchTools(): McpToolDefinition[] { // v1.39.3 — visits (procedures included), conditions, documents and // vaccinations, each behind its own module switch. candidates.push( - ...(await loadRecordCandidates(ctx.userId, rawQuery, origin)), + ...(await loadRecordCandidates(ctx.userId, rawQuery, origin, { + onTruncated: (kind) => truncatedSources.push(kind), + })), ); const results = rankCandidates(candidates, rawQuery).map( @@ -658,6 +685,12 @@ function searchAndFetchTools(): McpToolDefinition[] { ...(hasMore ? { nextCursor: encodeOffsetCursor(offset + SEARCH_PAGE_SIZE) } : {}), + // Every page says so, not only the last: the cut is in what was + // searched, which no cursor reaches past. + truncated: truncatedSources.length > 0, + ...(truncatedSources.length > 0 + ? { truncatedSources: [...new Set(truncatedSources)].sort() } + : {}), }; }, }, @@ -1356,6 +1389,12 @@ const getPreventiveCareOutput: z.ZodRawShape = { }), ) .optional(), + appointmentsTruncated: cappedOutput.truncated.describe( + "True when more booked appointments exist than `appointments` lists: the list was cut at `appointmentsLimit`, soonest first. Say the list is partial.", + ), + appointmentsLimit: cappedOutput.limit.describe( + "The most appointments this result could list.", + ), }; /** @@ -1392,6 +1431,7 @@ const getVisitsOutput: z.ZodRawShape = { // "module_disabled" when the illness module is off and the condition // labels are withheld rather than absent (v1.39.3). conditionsReason: z.string().optional(), + ...cappedOutput, }; /** Default trailing window for `get_visits` when the caller names none. */ @@ -1529,6 +1569,7 @@ const getEcgRecordingsOutput: z.ZodRawShape = { }), ) .optional(), + ...cappedOutput, }; /** `get_labs` behind its module gate: latest-per-biomarker or a history page. */ @@ -2182,21 +2223,25 @@ export const MCP_TOOLS: McpToolDefinition[] = [ // Upcoming appointments ride alongside the Vorsorge due-list, read from // `Encounter` (a booked visit in the future). They are a NAMED field on // this result, not a second tool, so "what is coming up" is one answer. - const appointmentRows = await prisma.encounter.findMany({ - where: { - userId: ctx.userId, - deletedAt: null, - status: "PLANNED", - occurredAt: { gt: now }, - }, - orderBy: { occurredAt: "asc" }, - take: MAX_APPOINTMENTS, - include: { - practitioner: { - select: { name: true, specialty: true, deletedAt: true }, - }, - }, - }); + const { rows: appointmentRows, cap: appointmentCap } = await capped( + MAX_APPOINTMENTS, + (take) => + prisma.encounter.findMany({ + where: { + userId: ctx.userId, + deletedAt: null, + status: "PLANNED", + occurredAt: { gt: now }, + }, + orderBy: [{ occurredAt: "asc" }, { id: "asc" }], + take, + include: { + practitioner: { + select: { name: true, specialty: true, deletedAt: true }, + }, + }, + }), + ); const appointments = appointmentRows.map((r) => { const reason = decryptVisitText(r.reasonEncrypted); // A practitioner the person deleted is no longer named. @@ -2288,7 +2333,15 @@ export const MCP_TOOLS: McpToolDefinition[] = [ return { present: true, ...(checkups.length > 0 ? { checkups } : {}), - ...(appointments.length > 0 ? { appointments } : {}), + ...(appointments.length > 0 + ? { + appointments, + // The cap describes the appointments only; the checkups are + // read whole. + appointmentsTruncated: appointmentCap.truncated, + appointmentsLimit: appointmentCap.limit, + } + : {}), }; }, }, @@ -2394,25 +2447,27 @@ export const MCP_TOOLS: McpToolDefinition[] = [ async run(ctx) { // Device data, served whatever the AI switches or the AI analysis // opt-out say, like `GET /api/insights/ecg`. - const rows = await prisma.ecgRecording.findMany({ - where: { userId: ctx.userId }, - // Everything EXCEPT `waveformEncrypted` — mirrors `GET - // /api/insights/ecg`'s own select exactly; no decrypt ever happens - // on this path. - select: { - id: true, - recordedAt: true, - durationSeconds: true, - samplingFrequency: true, - sampleCount: true, - averageHeartRate: true, - lead: true, - rhythmClassification: true, - source: true, - }, - orderBy: { recordedAt: "desc" }, - take: MAX_ECG_RECORDINGS, - }); + const { rows, cap } = await capped(MAX_ECG_RECORDINGS, (take) => + prisma.ecgRecording.findMany({ + where: { userId: ctx.userId }, + // Everything EXCEPT `waveformEncrypted` — mirrors `GET + // /api/insights/ecg`'s own select exactly; no decrypt ever happens + // on this path. + select: { + id: true, + recordedAt: true, + durationSeconds: true, + samplingFrequency: true, + sampleCount: true, + averageHeartRate: true, + lead: true, + rhythmClassification: true, + source: true, + }, + orderBy: [{ recordedAt: "desc" }, { id: "desc" }], + take, + }), + ); if (rows.length === 0) { annotate({ @@ -2444,6 +2499,7 @@ export const MCP_TOOLS: McpToolDefinition[] = [ present: true, classificationSource: "device" as const, recordings, + ...capFields(cap), }; }, }, @@ -2498,33 +2554,35 @@ export const MCP_TOOLS: McpToolDefinition[] = [ const cutoff = months === null ? null : new Date(now); cutoff?.setMonth(cutoff.getMonth() - (months ?? 0)); - const rows = await prisma.encounter.findMany({ - // `userId` is the resolved session's, never an argument — a visit - // belonging to another account is unreachable by construction. - where: { - userId: ctx.userId, - deletedAt: null, - occurredAt: { ...(cutoff ? { gte: cutoff } : {}), lte: now }, - ...(kind ? { kind } : {}), - ...(practitioner - ? { - practitioner: { - is: { - name: { contains: practitioner, mode: "insensitive" }, - deletedAt: null, + const { rows, cap } = await capped(MAX_VISITS, (take) => + prisma.encounter.findMany({ + // `userId` is the resolved session's, never an argument — a visit + // belonging to another account is unreachable by construction. + where: { + userId: ctx.userId, + deletedAt: null, + occurredAt: { ...(cutoff ? { gte: cutoff } : {}), lte: now }, + ...(kind ? { kind } : {}), + ...(practitioner + ? { + practitioner: { + is: { + name: { contains: practitioner, mode: "insensitive" }, + deletedAt: null, + }, }, - }, - } - : {}), - }, - orderBy: { occurredAt: "desc" }, - take: MAX_VISITS, - include: { - practitioner: { - select: { name: true, specialty: true, deletedAt: true }, + } + : {}), }, - }, - }); + orderBy: [{ occurredAt: "desc" }, { id: "desc" }], + take, + include: { + practitioner: { + select: { name: true, specialty: true, deletedAt: true }, + }, + }, + }), + ); // Absence is explicit. An empty list is { present: false } — "you have // never recorded a visit" — NEVER { present: true, visits: [] }, which @@ -2590,6 +2648,7 @@ export const MCP_TOOLS: McpToolDefinition[] = [ present: true, windowMonths: months, visits, + ...capFields(cap), ...(illnessOn ? {} : { conditionsReason: "module_disabled" }), }; },