mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(coach): keep the numeric verifier dormant when no tool returned figures
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.
This commit is contained in:
1 parent
fbdeec9204
commit
87e690bdcb
3 files changed
+116
-11
No files matched your search
@@ -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<typeof vi.fn>).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<typeof vi.fn>).mockReturnValue({
|
||||
prose: reply,
|
||||
keyValues: [],
|
||||
malformed: false,
|
||||
malformedEntries: [],
|
||||
});
|
||||
(parseSuggestReminder as ReturnType<typeof vi.fn>).mockReturnValue({
|
||||
prose: reply,
|
||||
});
|
||||
(runCoachToolLoop as ReturnType<typeof vi.fn>).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<typeof vi.fn>).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: {} },
|
||||
|
||||
@@ -651,20 +651,28 @@ async function handleChatRequest(request: NextRequest): Promise<Response> {
|
||||
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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in new issue
Block a user