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.
This commit is contained in:
Marc-André Bombeck committed 2026-10-05 21:52:48 +02:00
1 parent af79f4fb5c
commit 73ce721ff2
4 files changed
+108 -20

No files matched your search

+5 -4
View File
@@ -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
@@ -21,11 +21,21 @@ import {
} from "@/lib/ai/coach/series-condense";
const buildCoachSnapshot =
vi.fn<(userId: string, scope?: unknown) => Promise<CoachSnapshotResult>>();
vi.fn<
(
userId: string,
scope?: unknown,
options?: unknown,
) => Promise<CoachSnapshotResult>
>();
vi.mock("@/lib/ai/coach/snapshot", async (importOriginal) => ({
...(await importOriginal<object>()),
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,
});
});
});
+19 -3
View File
@@ -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<CoachSnapshotResult> {
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<CoachSnapshotResult> {
// 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(
+17 -10
View File
@@ -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<CoachSnapshotResult>).provenance
?.counts as Record<string, number | undefined> | undefined;