From 73ce721ff26f1a0c5132e46b1d4d77efc7090649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 5 Oct 2026 21:52:48 +0200 Subject: [PATCH] fix(mcp): keep the metric series read as it was The Coach's figure check ran before the MCP branch, so an MCP client was told retrieval_failed for a block it used to receive, and the plain-text renderer then said no data was on file. Any single-source build also condensed its own block, which reached MCP through the source snapshot's fallback to the full builder. The figure check now runs after the MCP branch, and condensing is an explicit option of the snapshot build that only the Coach tools set (it is part of the cache key). MCP output is what it was before. --- CHANGELOG.md | 9 +-- .../__tests__/series-long-window.test.ts | 70 ++++++++++++++++++- src/lib/ai/coach/snapshot.ts | 22 +++++- src/lib/ai/coach/tools/executor.ts | 27 ++++--- 4 files changed, 108 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70a6f3d3a..c87841dab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,11 @@ dashboard card fits a phone; text fields lose the purple focus ring. shared per-turn snapshot capped at 24,000 characters; on accounts with many series whole blocks became a placeholder and the tool still answered `present: true`. A trimmed block is now re-read for the tool's own source, - a single-source read is condensed (summary, newest days, weekly and monthly - means) instead of emptied, a block without numbers answers - `retrieval_failed`, and the Coach calls the table tool itself for long - histories. The step list shows the reading count; no stray comma on copy. + a Coach tool's read of one source is condensed (summary, newest days, + weekly and monthly means) instead of emptied, a block without numbers + answers `retrieval_failed` to the Coach, and the Coach calls the table tool + itself for long histories. MCP reads are unchanged. The step list shows + the reading count; no stray comma on copy. - **"Today" from yesterday's readings.** Signals of the day, the vitals fact, the lead line, the health status and the morning push only speak about today from readings taken today in the profile timezone; a stored briefing diff --git a/src/lib/ai/coach/__tests__/series-long-window.test.ts b/src/lib/ai/coach/__tests__/series-long-window.test.ts index 7bc3bd4a6..f05151f4d 100644 --- a/src/lib/ai/coach/__tests__/series-long-window.test.ts +++ b/src/lib/ai/coach/__tests__/series-long-window.test.ts @@ -21,11 +21,21 @@ import { } from "@/lib/ai/coach/series-condense"; const buildCoachSnapshot = - vi.fn<(userId: string, scope?: unknown) => Promise>(); + vi.fn< + ( + userId: string, + scope?: unknown, + options?: unknown, + ) => Promise + >(); vi.mock("@/lib/ai/coach/snapshot", async (importOriginal) => ({ ...(await importOriginal()), - buildCoachSnapshot: (userId: string, scope?: unknown) => - buildCoachSnapshot(userId, scope), + buildCoachSnapshot: (userId: string, scope?: unknown, options?: unknown) => + buildCoachSnapshot(userId, scope, options), +})); +const buildCoachSourceSnapshot = vi.fn(); +vi.mock("@/lib/ai/coach/source-snapshot", () => ({ + buildCoachSourceSnapshot: (...a: unknown[]) => buildCoachSourceSnapshot(...a), })); vi.mock("@/lib/db", () => ({ prisma: { @@ -308,3 +318,57 @@ describe("the prompt-budget pass on a single-source read", () => { expect(degraded.map((d) => d.key).at(-1)).toBe("fatMass"); }); }); + +describe("the MCP read stays as it was", () => { + beforeEach(() => { + buildCoachSnapshot.mockReset(); + buildCoachSourceSnapshot.mockReset(); + }); + + it("hands on the section as built, without the Coach's figure check", async () => { + buildCoachSourceSnapshot.mockResolvedValue({ + sections: { fatMass: { unit: "kg" } }, + referenceGrounding: null, + }); + const result = await executeCoachTool({ + userId: "u1", + name: "get_metric_series", + rawArguments: JSON.stringify({ metric: "fat_mass", window: "allTime" }), + sourceSnapshot: true, + }); + expect(JSON.stringify(result)).toBe( + JSON.stringify({ + present: true, + data: { metric: "fat_mass", section: { unit: "kg" } }, + }), + ); + expect(buildCoachSnapshot).not.toHaveBeenCalled(); + }); + + it("condenses only on a Coach read of one source", async () => { + buildCoachSnapshot.mockImplementation(async (_u, scope) => + scope === SHARED + ? snapshot( + { fatMass: { omitted: "trimmed for prompt budget" } }, + { degradedBlocks: ["fatMass"] }, + ) + : snapshot({ fatMass: FAT_MASS }), + ); + // Shared build first (plain), then the re-read of the one source. + await series({ metric: "fat_mass", window: "allTime" }); + expect(buildCoachSnapshot.mock.calls[0][2]).toEqual({ + reach: UNBOUNDED_REACH, + }); + expect(buildCoachSnapshot.mock.calls[1][2]).toEqual({ + reach: UNBOUNDED_REACH, + condenseRequested: true, + }); + // A window other than the turn's is its own single-source read. + buildCoachSnapshot.mockClear(); + await series({ metric: "fat_mass", window: "last90days" }); + expect(buildCoachSnapshot.mock.calls[0][2]).toEqual({ + reach: UNBOUNDED_REACH, + condenseRequested: true, + }); + }); +}); diff --git a/src/lib/ai/coach/snapshot.ts b/src/lib/ai/coach/snapshot.ts index 4c7bae0dd..a104ca00d 100644 --- a/src/lib/ai/coach/snapshot.ts +++ b/src/lib/ai/coach/snapshot.ts @@ -199,13 +199,26 @@ export async function buildCoachSnapshot( * one, as it always did. */ reach?: CoachHistoryReach; + /** + * A Coach tool reading one source: condense that source's blocks to the + * prompt budget instead of shedding them (`series-condense.ts`). Every + * other caller, MCP among them, keeps the plain budget pass, so its + * output is what it always was. + */ + condenseRequested?: boolean; }, ): Promise { const reach = options?.reach ?? UNBOUNDED_REACH; - const key = snapshotCacheKey(userId, scope, reach); + const condenseRequested = options?.condenseRequested === true; + const key = `${snapshotCacheKey(userId, scope, reach)}${condenseRequested ? "|condense" : ""}`; const cached = readSnapshotCache(key); if (cached) return cached; - const result = await buildCoachSnapshotImpl(userId, scope, reach); + const result = await buildCoachSnapshotImpl( + userId, + scope, + reach, + condenseRequested, + ); writeSnapshotCache(key, result); return result; } @@ -237,6 +250,7 @@ async function buildCoachSnapshotImpl( userId: string, scope: CoachScope | undefined, reach: CoachHistoryReach, + condenseRequested: boolean, ): Promise { // Prefs, module gates and the admitted sources — see `snapshot-prelude.ts`. // The lookback limit caps the narration window there, whatever the caller @@ -1062,7 +1076,9 @@ async function buildCoachSnapshotImpl( // a metric page) asked for that source's blocks: those are condensed, never // emptied, and only after everything else has been shed. const requestedSource = - Array.isArray(scope?.sources) && scope.sources.length === 1 + condenseRequested && + Array.isArray(scope?.sources) && + scope.sources.length === 1 ? scope.sources[0] : null; const requestedBlocks = new Set( diff --git a/src/lib/ai/coach/tools/executor.ts b/src/lib/ai/coach/tools/executor.ts index a431530e5..ea65a685c 100644 --- a/src/lib/ai/coach/tools/executor.ts +++ b/src/lib/ai/coach/tools/executor.ts @@ -276,8 +276,13 @@ async function readToolSnapshot( reach: CoachHistoryReach, ) { const scope = scopeFor(sources, window, fallbackWindow, sharedScope, reach); + if (scope !== sharedScope) { + return buildCoachSnapshot(userId, scope, { + reach, + condenseRequested: true, + }); + } const snapshot = await buildCoachSnapshot(userId, scope, { reach }); - if (scope !== sharedScope) return snapshot; const cut = sectionKeys.filter((key) => snapshot.degradedBlocks?.includes(key), ); @@ -289,7 +294,7 @@ async function readToolSnapshot( return buildCoachSnapshot( userId, { sources, window: scope.window }, - { reach }, + { reach, condenseRequested: true }, ); } @@ -567,6 +572,16 @@ async function getMetricSeries( reach, ); } + if (sourceSnapshot) { + // MCP: the section exactly as it always was, without the figure check + // below: an MCP client reads `present` by its own contract (absence is + // "not recorded"), and its output stays byte for byte what it was. + return { + present: true, + data: { metric, section }, + grounding: snapshot.referenceGrounding ?? undefined, + }; + } // A block without a single figure (a bare unit, an `omitted` marker) is // not an answer: "present" with nothing in it is what the model then // repeats to the person. Say the read failed instead. @@ -577,14 +592,6 @@ async function getMetricSeries( }); return { present: false, reason: "retrieval_failed" }; } - if (sourceSnapshot) { - // MCP: the section exactly as it always was. - return { - present: true, - data: { metric, section }, - grounding: snapshot.referenceGrounding ?? undefined, - }; - } // Outside MCP the read is a Coach snapshot, which counts what it read. const counts = (snapshot as Partial).provenance ?.counts as Record | undefined;