From 87e690bdcbf10663908e1a982372c9553a033263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 10:31:38 +0200 Subject: [PATCH] fix(coach): keep the numeric verifier dormant when no tool returned figures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit spliced the DATA INVENTORY manifest into the tool-mode authoritative payload set unconditionally. That made the payload set non-empty on a turn where the model answered WITHOUT calling any tool (empty tool results), which flipped the verifier from dormant to active — grading the reply against an inventory that carries per-domain sample counts only, never the snapshot's pre-computed averages. A figure the model correctly narrated from the snapshot (a 30-day systolic average) was then stripped to "[unverified]" — the exact false positive this work exists to prevent, and a regression from main's "no tool figures this turn → skip, prompt-level grounding is the backstop" contract. The inventory must only WIDEN an already-active authoritative set, never ACTIVATE it on its own. Gate activation on a real present figure block (a pinned workout-evidence block or a present tool result) exactly as main did; when active, the inventory counts still join the set so a plain count restatement stays grounded. Adds a route-level regression test (fast suite) proving the verifier stays dormant when no tool returned figures, plus companion unit tests documenting that a real tool section grounds the average while a counts-only inventory does not. --- .../chat/__tests__/route-tool-mode.test.ts | 59 +++++++++++++++++++ src/app/api/insights/chat/route.ts | 30 ++++++---- .../__tests__/coach-prose-grounding.test.ts | 38 ++++++++++++ 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/src/app/api/insights/chat/__tests__/route-tool-mode.test.ts b/src/app/api/insights/chat/__tests__/route-tool-mode.test.ts index dde8d8ec4..3e99f2cf6 100644 --- a/src/app/api/insights/chat/__tests__/route-tool-mode.test.ts +++ b/src/app/api/insights/chat/__tests__/route-tool-mode.test.ts @@ -479,6 +479,65 @@ describe("coach chat — tool-mode routing (F1)", () => { expect(content).toContain("320"); }); + it("keeps the verifier dormant when NO tool returned figures this turn, even though the inventory rides the prompt (v1.32.1 regression)", async () => { + // Regression guard for the integration failure: in tool mode the model can + // answer a quick turn WITHOUT calling any tool (empty toolResults). The + // always-sent DATA INVENTORY carries counts only, never the snapshot's + // pre-computed averages. The verifier must NOT activate off the inventory + // alone — otherwise a figure the model narrated from the snapshot (a + // 30-day systolic average) is wrongly stripped. Matches `main`'s "no tool + // figures → skip, prompt-level grounding is the backstop" contract. + resolveProviderChain.mockResolvedValue([ + { providerType: "anthropic", instance: {} }, + ]); + (buildCoachDataInventory as ReturnType).mockResolvedValueOnce( + { + entries: [ + { + tool: "get_metric_series", + metric: "bp", + domain: "blood pressure", + present: true, + count: 14, + }, + ], + restMode: false, + cycleEnabled: false, + window: "last30days", + probeScope: { sources: ["bp"], window: "last30days" }, + }, + ); + const reply = "Your 30-day systolic average is 130 mmHg, trending up."; + (parseKeyValuesSentinel as ReturnType).mockReturnValue({ + prose: reply, + keyValues: [], + malformed: false, + malformedEntries: [], + }); + (parseSuggestReminder as ReturnType).mockReturnValue({ + prose: reply, + }); + (runCoachToolLoop as ReturnType).mockImplementation( + async () => ({ + result: { content: reply, tokensUsed: 60, model: "m" }, + workingProviderType: "anthropic", + totalTokens: 60, + rounds: 1, + toolTrace: [], + // No tool returned figures this turn — the model answered directly. + toolResults: [], + }), + ); + await postAndDrain({ message: "How is my blood pressure this month?" }); + const calls = (appendMessage as ReturnType).mock.calls; + const assistantCall = calls.find( + (c) => (c[0] as { role: string }).role === "assistant", + ); + const content = (assistantCall?.[0] as { content: string }).content; + expect(content).not.toContain("[unverified]"); + expect(content).toContain("130"); + }); + it("falls back to the snapshot-stuffing path when a provider lacks tools", async () => { resolveProviderChain.mockResolvedValue([ { providerType: "anthropic", instance: {} }, diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index fa05230b3..718a0a130 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -651,20 +651,28 @@ async function handleChatRequest(request: NextRequest): Promise { result = loop.result; workingProviderType = loop.workingProviderType; toolTrace = loop.toolTrace; - toolResultPayloads = [ + // v1.32.1 — the numeric verifier ACTIVATES only when this turn actually + // delivered figures the model was told to ground against: a pinned + // workout-evidence block or a present tool result. The DATA INVENTORY + // manifest (sample counts per domain) is NOT an activator — it rides + // every tool-mode prompt even on a turn where the model answered + // without calling a tool, and on that turn the base prompt deliberately + // carries no pre-computed figures (the model must fetch them), so the + // verifier must stay dormant and leave the prompt-level grounding rule + // as the backstop, exactly as on `main`. Activating it off the + // counts-only inventory would flag a snapshot figure the model cited + // without a fresh tool call as ungrounded (a real regression caught by + // the integration suite). When the turn IS active, the inventory counts + // still WIDEN the authoritative set so a plain count restatement + // ("you've logged 42 BP readings") stays grounded. + const presentToolPayloads = [ ...(workoutEvidence === null ? [] : [workoutEvidence]), - // v1.32.1 (false-positive closure) — the DATA INVENTORY manifest - // (sample counts per domain) rides EVERY tool-mode turn's prompt - // regardless of which tools actually ran this turn - // (`renderDataInventory`) — it is server-computed and handed to the - // model exactly like `workoutEvidence` above, so a plain restatement - // of a domain's sample count ("you've logged 42 BP readings") is - // grounded even when the model did not also call a retrieval tool - // for that domain this turn. Splicing it in here closes that gap - // the same way `workoutEvidence` already does. - inventory.entries, ...(loop.toolResults ?? []).map((r) => r.data), ]; + toolResultPayloads = + presentToolPayloads.length > 0 + ? [...presentToolPayloads, inventory.entries] + : []; totalTokensSpent = loop.totalTokens; cachedTokensSpent = loop.cachedTokens; } else { diff --git a/src/lib/ai/coach/__tests__/coach-prose-grounding.test.ts b/src/lib/ai/coach/__tests__/coach-prose-grounding.test.ts index 126de206b..d9150d489 100644 --- a/src/lib/ai/coach/__tests__/coach-prose-grounding.test.ts +++ b/src/lib/ai/coach/__tests__/coach-prose-grounding.test.ts @@ -58,6 +58,44 @@ describe("findUnverifiedCoachNumbers", () => { expect(findUnverifiedCoachNumbers(prose, payloads)).toEqual([]); }); + it("grounds a 30-day systolic average against the real tool section that carries it", () => { + // Mirrors the integration fixture: the model narrates the pre-computed + // 30-day systolic average (130) that the bp tool section actually returned. + // It must NOT be flagged. (Regression companion: the route must feed this + // real section, not a counts-only inventory — see route-tool-mode tests.) + const bpSection = [ + { metric: "bp", section: { aggregate: { avgSys30: 130 } } }, + ]; + const prose = "Your 30-day systolic average is 130 mmHg, trending up."; + expect(findUnverifiedCoachNumbers(prose, bpSection)).toEqual([]); + }); + + it("does NOT ground an average against a counts-only inventory payload (documents why the route must not activate on the inventory alone)", () => { + // The DATA INVENTORY carries per-domain sample COUNTS, never the snapshot's + // averages. If the route were to feed inventory-entries alone as the + // authoritative set, a cited average (130) would be graded against {14} + // and wrongly flagged. This asserts that shape so the route's activation + // gate (only run when a real tool result / workout block is present) is + // load-bearing, not incidental. + const inventoryOnly = [ + [ + { + tool: "get_metric_series", + metric: "bp", + domain: "blood pressure", + present: true, + count: 14, + }, + ], + ]; + const findings = findUnverifiedCoachNumbers( + "Your 30-day systolic average is 130 mmHg.", + inventoryOnly, + ); + expect(findings).toHaveLength(1); + expect(findings[0].value).toBe(130); + }); + it("exempts structural window + small-ordinal integers", () => { const prose = "Over the last 30 days, 2 readings stood out of 7 total."; // 30 + 7 are window integers; 2 is a small ordinal — none graded.