diff --git a/src/app/api/insights/chat/__tests__/dialog-harness.ts b/src/app/api/insights/chat/__tests__/dialog-harness.ts index c9b2dc079..b1f2cc60c 100644 --- a/src/app/api/insights/chat/__tests__/dialog-harness.ts +++ b/src/app/api/insights/chat/__tests__/dialog-harness.ts @@ -68,6 +68,18 @@ export interface ProviderCall { } export const providerCalls: ProviderCall[] = []; +/** + * Everything one provider call carried on the user side. Since #1217 a turn's + * own lines (a resolved follow-up, an answered clarification, a continuation) + * travel here, never in the system prompt, so the cached prefix holds. + */ +export function userSide(call: ProviderCall): string { + return call.messages + .filter((msg) => msg.role === "user") + .map((msg) => msg.content) + .join("\n\n"); +} + /** The last assistant message of the stored conversation. */ export const LAST_ASSISTANT_ID = "m-a1"; export const CONVERSATION_ID = "c-dialog"; diff --git a/src/app/api/insights/chat/__tests__/golden-transcripts.test.ts b/src/app/api/insights/chat/__tests__/golden-transcripts.test.ts index 1703970b6..821db4dee 100644 --- a/src/app/api/insights/chat/__tests__/golden-transcripts.test.ts +++ b/src/app/api/insights/chat/__tests__/golden-transcripts.test.ts @@ -113,6 +113,7 @@ vi.mock("@/lib/ai/coach/budget", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: h.m.getCoachSystemPrompt, + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: h.m.getSelfContextTextForUser, @@ -652,21 +653,34 @@ describe("coach chat golden transcripts", () => { expect(transcript.thrown).toBeNull(); expect(m.runStreamingRawCompletionWithFallback).toHaveBeenCalledTimes(1); const { params } = m.runStreamingRawCompletionWithFallback.mock - .calls[0][0] as { params: { system: string } }; - expect(params.system).toContain(CLARIFIED); + .calls[0][0] as { + params: { system: string; messages: Array<{ content: string }> }; + }; + // #1217 — on the user side, never in the cached system prompt. + const user = params.messages.map((msg) => msg.content).join("\n"); + expect(user).toContain(CLARIFIED); + expect(params.system).not.toContain(CLARIFIED); // Server-authored: the request's choice id picks the stored value, // the label the model once wrote never rides along. - expect(params.system).not.toContain("Resting pulse"); + const line = user + .split("\n") + .find((text) => text.startsWith("CLARIFIED:")); + expect(line).toContain(CLARIFIED); + expect(line).not.toContain("Resting pulse"); }); it("the tool-mode prompt carries the same line", async () => { reply("Your resting pulse averaged 62 bpm."); const transcript = await runTurn(post, body, {}); expect(transcript.thrown).toBeNull(); - const { system } = m.runCoachToolLoop.mock.calls[0][0] as { + const { system, messages } = m.runCoachToolLoop.mock.calls[0][0] as { system: string; + messages: Array<{ content: string }>; }; - expect(system).toContain(CLARIFIED); + expect(messages.map((msg) => msg.content).join("\n")).toContain( + CLARIFIED, + ); + expect(system).not.toContain(CLARIFIED); }); it("an unanswered turn sends neither prompt the line", async () => { @@ -680,8 +694,13 @@ describe("coach chat golden transcripts", () => { {}, ); const { params } = m.runStreamingRawCompletionWithFallback.mock - .calls[0][0] as { params: { system: string } }; + .calls[0][0] as { + params: { system: string; messages: Array<{ content: string }> }; + }; expect(params.system).not.toContain("CLARIFIED:"); + expect( + params.messages.map((msg) => msg.content).join("\n"), + ).not.toContain("CLARIFIED:"); }); }); }); diff --git a/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts b/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts index e76d8a5e2..a945487be 100644 --- a/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts +++ b/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts @@ -118,6 +118,7 @@ vi.mock("@/lib/ai/coach/refusal", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-capability-gate.test.ts b/src/app/api/insights/chat/__tests__/route-capability-gate.test.ts index ea52a5815..18ca5598c 100644 --- a/src/app/api/insights/chat/__tests__/route-capability-gate.test.ts +++ b/src/app/api/insights/chat/__tests__/route-capability-gate.test.ts @@ -116,6 +116,7 @@ vi.mock("@/lib/ai/coach/budget", () => ({ vi.mock("@/lib/ai/coach/refusal", () => ({ detectRefusal: vi.fn() })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(), diff --git a/src/app/api/insights/chat/__tests__/route-fence-guard.test.ts b/src/app/api/insights/chat/__tests__/route-fence-guard.test.ts index 203fde324..f966c86c3 100644 --- a/src/app/api/insights/chat/__tests__/route-fence-guard.test.ts +++ b/src/app/api/insights/chat/__tests__/route-fence-guard.test.ts @@ -103,6 +103,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-followups.test.ts b/src/app/api/insights/chat/__tests__/route-followups.test.ts index fbfc92f84..b4490a3d5 100644 --- a/src/app/api/insights/chat/__tests__/route-followups.test.ts +++ b/src/app/api/insights/chat/__tests__/route-followups.test.ts @@ -265,7 +265,7 @@ describe("a reuse chip", () => { const run = await tap("f3"); expect(run.status).toBe(200); expect(annotated("coach.followUp.reuse_unavailable")).toHaveLength(1); - expect(providerCalls[0].system).toContain( + expect(h.userSide(providerCalls[0])).toContain( "Call show_result with ref m1.r1 and view chart", ); }); @@ -282,15 +282,16 @@ describe("a model chip", () => { anchor: { domain: "glucose", window: "allTime" }, }); expect(run.status).toBe(200); - const system = providerCalls[0].system; - expect(system).toContain( + const user = h.userSide(providerCalls[0]); + expect(user).toContain( "FOLLOW-UP: the person tapped the previous_period chip", ); - expect(system).toContain( + expect(user).toContain( "get_metric_table metric=bp window=last30days period=previous granularity=week", ); - expect(system).toContain("m1.r1"); - expect(system).not.toContain("glucose window=allTime"); + expect(user).toContain("m1.r1"); + expect(user).not.toContain("glucose window=allTime"); + expect(providerCalls[0].system).not.toContain("FOLLOW-UP:"); }); it("degrades a chip that is not on the latest reply to a plain message", async () => { @@ -303,7 +304,7 @@ describe("a model chip", () => { }); expect(run.status).toBe(200); expect(providerCalls).toHaveLength(1); - expect(providerCalls[0].system).not.toContain("FOLLOW-UP:"); + expect(h.userSide(providerCalls[0])).not.toContain("FOLLOW-UP:"); expect(annotated("coach.followUp.stale")).toEqual([ expect.objectContaining({ meta: { reason: "not_latest" } }), ]); @@ -313,7 +314,7 @@ describe("a model chip", () => { conversationWithChips(); world.script = [{ text: "Here is what I see." }]; await tap("f9"); - expect(providerCalls[0].system).not.toContain("FOLLOW-UP:"); + expect(h.userSide(providerCalls[0])).not.toContain("FOLLOW-UP:"); expect(annotated("coach.followUp.stale")).toEqual([ expect.objectContaining({ meta: { reason: "unknown_chip" } }), ]); @@ -343,16 +344,17 @@ describe("a model chip", () => { { text: "Having looked further, the readings stay level." }, ]; await tap("f1"); - const system = providerCalls[0].system; - expect(system).toContain( + const user = h.userSide(providerCalls[0]); + expect(user).toContain( "CONTINUE: the person asked you to keep looking. The unfinished question is their message before that request, in CONVERSATION.", ); - // The question stays the person's own turn; it never reaches the - // system role. - expect(system).not.toContain("How was my blood pressure last month?"); - expect(system).toContain( + expect(user).toContain( "Already fetched: get_metric_table(bp, last30days, week) → m1.r1", ); + // The system role carries neither the line nor the question (#1217). + const { system } = providerCalls[0]; + expect(system).not.toContain("CONTINUE:"); + expect(system).not.toContain("How was my blood pressure last month?"); }); }); @@ -373,7 +375,7 @@ describe("an answered question", () => { clarification: { messageId: h.LAST_ASSISTANT_ID, choiceId: "c2" }, }); expect(run.status).toBe(200); - expect(providerCalls[0].system).toContain( + expect(h.userSide(providerCalls[0])).toContain( "CLARIFIED: the person answered your clarifying question by choosing metric=resting_hr.", ); }); @@ -385,7 +387,7 @@ describe("an answered question", () => { message: "The one from my watch", clarification: { messageId: h.LAST_ASSISTANT_ID }, }); - expect(providerCalls[0].system).toContain( + expect(h.userSide(providerCalls[0])).toContain( "CLARIFIED: the person answered your clarifying question in their own words", ); }); @@ -397,7 +399,7 @@ describe("an answered question", () => { message: "Resting heart rate", clarification: { messageId: "m-older", choiceId: "c2" }, }); - expect(providerCalls[0].system).not.toContain("CLARIFIED:"); + expect(h.userSide(providerCalls[0])).not.toContain("CLARIFIED:"); expect(annotated("coach.clarification.stale")).toHaveLength(1); }); diff --git a/src/app/api/insights/chat/__tests__/route-guard-activation.test.ts b/src/app/api/insights/chat/__tests__/route-guard-activation.test.ts index d63bc8a7f..182861065 100644 --- a/src/app/api/insights/chat/__tests__/route-guard-activation.test.ts +++ b/src/app/api/insights/chat/__tests__/route-guard-activation.test.ts @@ -154,6 +154,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-guard-ledger.test.ts b/src/app/api/insights/chat/__tests__/route-guard-ledger.test.ts index 4f455a742..c692f5205 100644 --- a/src/app/api/insights/chat/__tests__/route-guard-ledger.test.ts +++ b/src/app/api/insights/chat/__tests__/route-guard-ledger.test.ts @@ -147,6 +147,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-guard.test.ts b/src/app/api/insights/chat/__tests__/route-guard.test.ts index b47f19163..97d5c793f 100644 --- a/src/app/api/insights/chat/__tests__/route-guard.test.ts +++ b/src/app/api/insights/chat/__tests__/route-guard.test.ts @@ -127,6 +127,7 @@ vi.mock("@/lib/ai/coach/refusal", () => ({ detectRefusal })); // NB: the outbound guard + numeric verifier are REAL — that is the point. vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-list-query.test.ts b/src/app/api/insights/chat/__tests__/route-list-query.test.ts index 59beef5cf..7a221034f 100644 --- a/src/app/api/insights/chat/__tests__/route-list-query.test.ts +++ b/src/app/api/insights/chat/__tests__/route-list-query.test.ts @@ -109,6 +109,7 @@ vi.mock("@/lib/ai/coach/budget", () => ({ vi.mock("@/lib/ai/coach/refusal", () => ({ detectRefusal: vi.fn() })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(), diff --git a/src/app/api/insights/chat/__tests__/route-snapshot-once.test.ts b/src/app/api/insights/chat/__tests__/route-snapshot-once.test.ts index afe28acfc..713dfcc5c 100644 --- a/src/app/api/insights/chat/__tests__/route-snapshot-once.test.ts +++ b/src/app/api/insights/chat/__tests__/route-snapshot-once.test.ts @@ -120,6 +120,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), 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 19749f312..fcb3028b6 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 @@ -153,6 +153,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/app/api/insights/chat/__tests__/route-workout-scope.test.ts b/src/app/api/insights/chat/__tests__/route-workout-scope.test.ts index 10081bc48..28a98783e 100644 --- a/src/app/api/insights/chat/__tests__/route-workout-scope.test.ts +++ b/src/app/api/insights/chat/__tests__/route-workout-scope.test.ts @@ -193,6 +193,7 @@ vi.mock("@/lib/ai/coach/outbound-guard", () => ({ })); vi.mock("@/lib/ai/coach/system-prompt", () => ({ getCoachSystemPrompt: vi.fn(() => "SYSTEM"), + buildCoachTurnNotes: vi.fn(() => ""), })); vi.mock("@/lib/ai/coach/about-me", () => ({ getSelfContextTextForUser: vi.fn(async () => null), diff --git a/src/lib/ai/coach/__tests__/stable-prefix.test.ts b/src/lib/ai/coach/__tests__/stable-prefix.test.ts new file mode 100644 index 000000000..dcc7a7eb3 --- /dev/null +++ b/src/lib/ai/coach/__tests__/stable-prefix.test.ts @@ -0,0 +1,108 @@ +/** + * #1217 — the Coach's cached prefix (tools plus system prompt) is the same + * string on every turn of a conversation. What changes per turn (the opener + * hint, the name clause, a resolved follow-up, the table rules once a table + * exists) travels in the user message, so a provider's prompt cache holds + * from one turn to the next instead of being rewritten every time. + */ +import { createHash } from "node:crypto"; + +import { describe, expect, it } from "vitest"; + +import { buildCoachToolRequest } from "@/lib/ai/coach/chat-request-builder"; +import { + buildCoachTurnNotes, + getCoachSystemPrompt, +} from "@/lib/ai/coach/system-prompt"; +import { buildToolModeAddendum } from "@/lib/ai/coach/tools/system-addendum"; +import { + buildDialogAddenda, + buildTableRulesAddendum, +} from "@/lib/ai/coach/turn/addenda"; +import { + openerArchetypeHint, + shouldUseNameForTurn, +} from "@/lib/ai/prompts/opener-archetype"; +import type { Locale } from "@/lib/i18n/config"; + +const sha = (text: string) => createHash("sha256").update(text).digest("hex"); + +/** One turn's request, assembled the way the turn model assembles it. */ +function turnRequest(locale: Locale, turnIndex: number, hints: string[]) { + const systemPrompt = getCoachSystemPrompt(locale, undefined, "I run."); + const turnNotes = buildCoachTurnNotes(locale, { + firstName: "Alex", + mayUseName: shouldUseNameForTurn(`u1:${turnIndex}`), + openerHint: openerArchetypeHint(`u1:${turnIndex}`, locale), + }); + const tables = turnIndex > 1 ? [buildTableRulesAddendum(locale)] : []; + return buildCoachToolRequest({ + systemPrompt, + toolModeAddendum: `${buildToolModeAddendum(locale)}\n\n${buildDialogAddenda( + locale, + { tableRules: false }, + )}`, + focusHint: "", + workoutEvidence: null, + dataInventory: "DATA INVENTORY\n- pulse", + guidedBlock: "", + transcript: `user: question ${turnIndex}`, + languageName: "English", + turnNotes: [...tables, turnNotes, ...hints] + .filter((block) => block.length > 0) + .join("\n\n"), + }); +} + +describe("stable Coach prefix (#1217)", () => { + it.each(["en", "de", "fr", "ko"] as Locale[])( + "hashes the system prompt identically on every turn (%s)", + (locale) => { + const hashes = new Set(); + for (let turn = 0; turn < 12; turn++) { + const request = turnRequest( + locale, + turn, + turn % 3 === 0 ? [`FOLLOW-UP: earlier answer ${turn}`] : [], + ); + hashes.add(sha(request.system)); + } + expect(hashes.size).toBe(1); + }, + ); + + it("carries the per-turn lines in the user message instead", () => { + const opener = openerArchetypeHint("u1:4", "en"); + const request = turnRequest("en", 4, ["FOLLOW-UP: the pulse table"]); + const user = String(request.messages[0]!.content); + expect(user).toContain(opener); + expect(user).toContain("FOLLOW-UP: the pulse table"); + expect(user).toContain(buildTableRulesAddendum("en")); + expect(request.system).not.toContain(opener); + expect(request.system).not.toContain("FOLLOW-UP"); + expect(request.system).not.toContain(buildTableRulesAddendum("en")); + }); + + it("keeps the opener hint and the name out of the system prompt", () => { + for (const locale of ["en", "de"] as Locale[]) { + const system = getCoachSystemPrompt(locale, undefined, null); + expect(system).not.toMatch( + /OPENER HINT for this turn|OPENER-HINWEIS für diesen Turn/, + ); + expect(system).not.toContain("Alex"); + } + }); + + it("names the turn's opener and, on a gated turn, the first name", () => { + const notes = buildCoachTurnNotes("en", { + firstName: "Alex", + mayUseName: true, + openerHint: "Lead with the finding.", + }); + expect(notes).toContain( + "OPENER HINT for this turn: Lead with the finding.", + ); + expect(notes).toContain('the user\'s first name is "Alex"'); + expect(buildCoachTurnNotes("en", {})).toBe(""); + }); +}); diff --git a/src/lib/ai/coach/__tests__/tools-loop-budget.test.ts b/src/lib/ai/coach/__tests__/tools-loop-budget.test.ts index d208abff1..c39c6f4b5 100644 --- a/src/lib/ai/coach/__tests__/tools-loop-budget.test.ts +++ b/src/lib/ai/coach/__tests__/tools-loop-budget.test.ts @@ -105,11 +105,24 @@ function lastParams() { const calls = runRawCompletionWithFallback.mock.calls; return calls[calls.length - 1][0].params as { system: string; + messages: Array<{ role: string; content: unknown }>; toolChoice?: string; tools?: unknown[]; }; } +/** + * The final round's reason, as the model reads it: the last user-side line. + * #1217 — it no longer rides the system prompt, which stays the cached + * prefix every round of the turn shares. + */ +function finalRoundLine(): string { + const last = lastParams().messages.at(-1); + expect(last?.role).toBe("user"); + expect(lastParams().system).toBe(baseArgs.system); + return String(last?.content); +} + beforeEach(() => { emitSignal.mockReset(); executeCoachTool.mockReset(); @@ -138,9 +151,7 @@ describe("stop reasons", () => { // The tools stay on the final round: the history holds the calls, and // Anthropic refuses tool_use/tool_result blocks without `tools` (400). expect(lastParams().tools).toEqual(COACH_TOOL_DEFS); - expect(lastParams().system).toMatch( - /FINAL ROUND: this turn's token budget/, - ); + expect(finalRoundLine()).toMatch(/FINAL ROUND: this turn's token budget/); expect(out.result.content).toBe("Answer from what I have."); }); @@ -209,7 +220,7 @@ describe("stop reasons", () => { expect(out.stop).toEqual({ reason: "no_progress", rounds: 3 }); expect(out.forcedFinal).toBe(false); expectFinalRoundKeepsTools(); - expect(lastParams().system).toMatch(/brought nothing new/); + expect(finalRoundLine()).toMatch(/brought nothing new/); }); }); diff --git a/src/lib/ai/coach/chat-request-builder.ts b/src/lib/ai/coach/chat-request-builder.ts index 1a892b46e..91ea7e03a 100644 --- a/src/lib/ai/coach/chat-request-builder.ts +++ b/src/lib/ai/coach/chat-request-builder.ts @@ -148,6 +148,13 @@ export function buildCoachToolRequest(args: { guidedBlock: string; transcript: string; languageName: string; + /** + * v1.43 (#1217) — what changes from one turn to the next: the opener hint, + * the name clause, a resolved follow-up, the table rules once a table + * exists. Carried here, on the user side, so `system` is the same string + * on every turn and every round. Empty or absent: nothing is added. + */ + turnNotes?: string; }): { system: string; messages: AiMessage[] } { const focusBlock = args.focusHint ? `${args.focusHint}\n\n` : ""; const workoutDataBlock = @@ -160,7 +167,7 @@ ${fenceHealthData(JSON.stringify({ thisWorkout: args.workoutEvidence }))} const messages: AiMessage[] = [ { role: "user", - content: `${focusBlock}${workoutDataBlock}${args.memoryBlock ? `${args.memoryBlock}\n\n` : ""}${args.dataInventory}${args.priorResults ? `\n\n${args.priorResults}` : ""}${args.guidedBlock} + content: `${focusBlock}${workoutDataBlock}${args.memoryBlock ? `${args.memoryBlock}\n\n` : ""}${args.dataInventory}${args.priorResults ? `\n\n${args.priorResults}` : ""}${args.guidedBlock}${args.turnNotes ? `\n\n${args.turnNotes}` : ""} CONVERSATION ${args.transcript} diff --git a/src/lib/ai/coach/results/__tests__/loop-result-tables.test.ts b/src/lib/ai/coach/results/__tests__/loop-result-tables.test.ts index 780b1281f..1e392f059 100644 --- a/src/lib/ai/coach/results/__tests__/loop-result-tables.test.ts +++ b/src/lib/ai/coach/results/__tests__/loop-result-tables.test.ts @@ -99,7 +99,7 @@ describe("runCoachToolLoop — result tables", () => { expect(JSON.stringify(out.toolResults)).not.toContain("987654"); }); - it("adds the table rules to the rounds after the first table, not before", async () => { + it("adds the table rules to the rounds after the first table, not before, without touching the system prompt", async () => { executeCoachTool .mockResolvedValueOnce({ present: true, data: { n: 1 } }) .mockResolvedValueOnce({ @@ -113,16 +113,27 @@ describe("runCoachToolLoop — result tables", () => { name, arguments: '{"metric":"pulse"}', }); - runRawCompletionWithFallback - .mockResolvedValueOnce(round("", [call("a", "get_metric_series")])) - .mockResolvedValueOnce(round("", [call("b", "get_metric_table")])) - .mockResolvedValueOnce(round("answer")); + // The loop hands every round the same growing message array, so record + // what each round saw at the moment it was called. + const seen: Array<{ system: string; messages: unknown[] }> = []; + const replies = [ + round("", [call("a", "get_metric_series")]), + round("", [call("b", "get_metric_table")]), + round("answer"), + ]; + runRawCompletionWithFallback.mockImplementation(async (arg: unknown) => { + const { params } = arg as { + params: { system: string; messages: unknown[] }; + }; + seen.push({ system: params.system, messages: [...params.messages] }); + return replies[seen.length - 1]; + }); await runCoachToolLoop({ userId: "u1", providers: [], system: "sys", - systemOnceTableShown: "sys+tables", + tableRulesOnceShown: "TABLE RULES", messages: [{ role: "user", content: "q" }], tools: COACH_TOOL_DEFS, reach: UNBOUNDED_REACH, @@ -133,9 +144,26 @@ describe("runCoachToolLoop — result tables", () => { refs: createResultRefAllocator(), }, }); - const systems = runRawCompletionWithFallback.mock.calls.map( - ([arg]) => (arg as { params: { system: string } }).params.system, + // #1217 — one system prompt for every round, so the cached prefix holds. + expect(seen.map((round) => round.system)).toEqual(["sys", "sys", "sys"]); + const rulesIn = seen.map((round) => + round.messages.some( + (m) => + (m as { role: string; content: unknown }).role === "user" && + (m as { content: unknown }).content === "TABLE RULES", + ), + ); + expect(rulesIn).toEqual([false, false, true]); + // The rules follow the tool results that produced the table. + const last = seen[2]!.messages as Array<{ role: string }>; + expect(last.at(-1)).toEqual({ role: "user", content: "TABLE RULES" }); + expect(last.at(-2)?.role).toBe("tool"); + // Each round's messages extend the previous round's: a stable prefix. + expect(seen[1]!.messages.slice(0, seen[0]!.messages.length)).toEqual( + seen[0]!.messages, + ); + expect(seen[2]!.messages.slice(0, seen[1]!.messages.length)).toEqual( + seen[1]!.messages, ); - expect(systems).toEqual(["sys", "sys", "sys+tables"]); }); }); diff --git a/src/lib/ai/coach/system-prompt.ts b/src/lib/ai/coach/system-prompt.ts index 5ee5600fe..ee5a21713 100644 --- a/src/lib/ai/coach/system-prompt.ts +++ b/src/lib/ai/coach/system-prompt.ts @@ -1249,8 +1249,12 @@ const LOCALE_REPLY_FOOTER_FALLBACK: Record< /** * v1.22 (W6) — per-turn personalization passed by the chat route. All fields - * are optional and the addendum is omitted when none apply, so the prompt is - * byte-identical to the pre-feature one for the coverage test + unnamed users. + * are optional and the notes are empty when none apply. + * + * v1.43 (#1217) — these lines change every turn, so they no longer ride the + * system prompt: a system prompt that changed per turn rewrote the provider's + * cached prefix (tools plus system, 14 to 19 K tokens) on every turn. They + * are composed by {@link buildCoachTurnNotes} and travel in the user message. */ export interface CoachPersonalization { /** The user's first name; null when no display name is set. */ @@ -1261,11 +1265,17 @@ export interface CoachPersonalization { openerHint?: string | null; } +/** + * The Coach system prompt. Stable for one person across every turn and every + * round of a turn: it depends on the locale, the Coach preferences and the + * about-me text only, so the provider's prompt cache holds. Anything that + * changes per turn (the opener hint, the name clause, a resolved follow-up) + * goes into the user message instead. + */ export function getCoachSystemPrompt( locale: Locale, prefs: CoachPrefs = DEFAULT_COACH_PREFS, aboutMe: string | null = null, - personalization: CoachPersonalization = {}, ): string { let base: string; if (locale === "de") { @@ -1313,11 +1323,11 @@ export function getCoachSystemPrompt( // may only link a slug it lists, so a fabricated /learn URL is impossible. const withLearn = `${withPrefix}\n\n${learnCatalogPromptBlock()}`; // v1.22 (W6) — the narrative-quality addendum: the context-layer rule, the - // single-chart-token exception, the BP usual-range verbatim clause, plus the - // per-turn opener hint and sparse first-name personalization. Composed after - // the shared contracts so the chart-token exception can carve out the one - // place a `metric:` token is permitted. - const withV122 = `${withLearn}\n\n${buildCoachV122Addendum(locale, personalization)}`; + // single-chart-token exception, the BP usual-range verbatim clause. Composed + // after the shared contracts so the chart-token exception can carve out the + // one place a `metric:` token is permitted. The per-turn opener hint + // and name clause are in `buildCoachTurnNotes`. + const withV122 = `${withLearn}\n\n${buildCoachV122Addendum(locale)}`; // v1.15.20 — the user-authored "about me" self-description rides the // system prompt as a delimited, user-provided block (Settings → AI). const suffix = aboutMe ? buildAboutMeBlock(locale, aboutMe) : ""; @@ -1338,14 +1348,11 @@ export function getCoachSystemPrompt( * - BP USUAL RANGE — when the snapshot's blood-pressure block carries a * `usualRange`, quote THOSE bounds verbatim; never invent a band from window * means (kills the fabricated "153–150" range). - * - OPENER HINT — vary the turn's opening so a multi-turn session does not open - * identically every time. - * - NAME — sparse, anti-formulaic first-name address on the gated turns only. + * + * The per-turn opener hint and first-name clause that used to close this + * addendum are {@link buildCoachTurnNotes} now. */ -function buildCoachV122Addendum( - locale: Locale, - personalization: CoachPersonalization, -): string { +function buildCoachV122Addendum(locale: Locale): string { const isDe = instructionLocale(locale) === "de"; const parts: string[] = []; parts.push(isDe ? "COACH-ZUSATZ (v1.22)" : "COACH ADDENDUM (v1.22)"); @@ -1392,6 +1399,21 @@ function buildCoachV122Addendum( : `EXPERIMENT REVIEW — when the snapshot carries an "experimentOutcomes" block, read the result back in the user's own numbers, association-only ("looks worth keeping" / "associated with", NEVER "proven" / "it worked" / "caused"). Report a null result honestly ("no measurable change — your call"). If a vital WORSENED during the experiment, do NOT cheerlead it — raise it plainly and route to the doctor / the acute-redflag path. Only ever propose BEHAVIORAL experiments (sleep timing, steps, screen time, meal timing); never a medication / dose / supplement trial — refuse those and defer to the prescriber.`, ); + return parts.join("\n\n"); +} + +/** + * v1.43 (#1217) — the lines that change every Coach turn: the hash-seeded + * opener hint and, on the gated turns, the first-name clause. They travel in + * the user message so the system prompt stays byte-stable across turns and + * the provider's prompt cache holds. Empty when neither applies. + */ +export function buildCoachTurnNotes( + locale: Locale, + personalization: CoachPersonalization, +): string { + const isDe = instructionLocale(locale) === "de"; + const parts: string[] = []; if (personalization.openerHint) { parts.push( isDe @@ -1409,7 +1431,8 @@ function buildCoachV122Addendum( ); } - return parts.join("\n\n"); + if (parts.length === 0) return ""; + return [isDe ? "FÜR DIESEN TURN" : "FOR THIS TURN", ...parts].join("\n\n"); } /** diff --git a/src/lib/ai/coach/tools/loop.ts b/src/lib/ai/coach/tools/loop.ts index 2a0c40a82..5fe032389 100644 --- a/src/lib/ai/coach/tools/loop.ts +++ b/src/lib/ai/coach/tools/loop.ts @@ -194,10 +194,13 @@ export async function runCoachToolLoop(args: { providers: ProviderChainResolved[]; system: string; /** - * v1.39.4 — the system prompt for the rounds after a call produced a - * table, when it differs (the table rules ride it). + * v1.39.4 — the table rules, for the rounds after a call produced a table. + * v1.43 (#1217) — they join the conversation as a user-side note after + * that round's tool results instead of switching the system prompt, so the + * cached prefix (tools plus system) survives the switch. Absent when the + * turn already carries them. */ - systemOnceTableShown?: string; + tableRulesOnceShown?: string; /** The conversation messages (history + the new user turn). */ messages: AiMessage[]; tools: AiToolDef[]; @@ -284,7 +287,12 @@ export async function runCoachToolLoop(args: { const cacheKey = coachPromptCacheKey(turn?.conversationId); const messages: AiMessage[] = [...args.messages]; - let system = args.system; + // v1.43 (#1217) — the system prompt never changes inside a turn: what used + // to be swapped or appended into it (the table rules, the final-round line) + // goes on the user side, so every round reads the same cached prefix. + const system = args.system; + let tableRulesPending = args.tableRulesOnceShown; + let tableShownThisRound = false; let totalTokens = 0; let cachedTokens = 0; let rounds = 0; @@ -352,10 +360,15 @@ export async function runCoachToolLoop(args: { surface: "coach", ledger, params: { - system: stopReason - ? `${system}\n\n${FINAL_ROUND_LINE[stopReason]}` - : system, - messages, + system, + // The final round says why it is final as the last user-side line, + // after everything the earlier rounds cached. + messages: stopReason + ? [ + ...messages, + { role: "user", content: FINAL_ROUND_LINE[stopReason] }, + ] + : messages, temperature, maxTokens, signal, @@ -522,9 +535,7 @@ export async function runCoachToolLoop(args: { // prose against that same summary. notify(() => onCallSettled?.(call, toolResult, index, round)); const { table, ...forModel } = toolResult; - if (table && args.systemOnceTableShown) { - system = args.systemOnceTableShown; - } + if (table) tableShownThisRound = true; const validArgs = call.name === COMPARE_SERIES_TOOL_NAME ? compareArgs(call.arguments) @@ -551,6 +562,14 @@ export async function runCoachToolLoop(args: { budget.addInput(textTokens(serialised)); messages.push({ role: "tool", toolCallId: call.id, content: serialised }); } + // The first table of the turn brings the table rules along, once, right + // after the results that produced it. + if (tableShownThisRound && tableRulesPending) { + messages.push({ role: "user", content: tableRulesPending }); + budget.addInput(textTokens(tableRulesPending)); + tableRulesPending = undefined; + } + tableShownThisRound = false; budget.addInput(textTokens(result.content ?? "")); notify(() => onRoundSettled?.(round)); diff --git a/src/lib/ai/coach/turn/__tests__/model-stable-prefix.test.ts b/src/lib/ai/coach/turn/__tests__/model-stable-prefix.test.ts new file mode 100644 index 000000000..1a90f4b14 --- /dev/null +++ b/src/lib/ai/coach/turn/__tests__/model-stable-prefix.test.ts @@ -0,0 +1,136 @@ +/** + * #1217 — the turn model sends the same system prompt on every turn. The + * per-turn lines (opener hint, name clause, a resolved follow-up) reach the + * model on the user side, on the tool path and on the streaming no-tools + * path alike, so the provider's cached prefix survives from turn to turn. + */ +import { createHash } from "node:crypto"; + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const m = vi.hoisted(() => ({ + loop: vi.fn(), + stream: vi.fn(), +})); + +vi.mock("@/lib/db", () => ({ prisma: {} })); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/ai/coach/persistence", () => ({ appendMessage: vi.fn() })); +vi.mock("@/lib/ai/coach/memory/contract", () => ({ + buildMemoryContextBlock: vi.fn(async () => null), +})); +vi.mock("@/lib/ai/coach/tools/executor", async (importOriginal) => ({ + ...(await importOriginal()), + admittedPriorResults: vi.fn(async () => []), +})); +vi.mock("@/lib/ai/coach/tools", async (importOriginal) => ({ + ...(await importOriginal()), + buildCoachDataInventory: vi.fn(async () => ({ + entries: [], + probeScope: undefined, + })), + renderDataInventory: () => "DATA INVENTORY", + runCoachToolLoop: (args: unknown) => m.loop(args), +})); +vi.mock("@/lib/ai/provider-runner", async (importOriginal) => ({ + ...(await importOriginal()), + runStreamingRawCompletionWithFallback: (args: unknown) => m.stream(args), +})); + +import { runTurnModel } from "../model"; +import type { TurnContext } from "../context"; + +const sha = (text: string) => createHash("sha256").update(text).digest("hex"); + +function ctx(turnNotes: string): TurnContext { + return { + coachPrefs: {}, + reach: { kind: "unbounded" }, + aiResponseTimeoutMs: 60_000, + effectiveScope: undefined, + snapshot: { + snapshotJson: "{}", + sections: {}, + provenance: { windows: [], metrics: [], counts: {} }, + }, + aboutMe: null, + scheduleDoses: [], + medicationNames: [], + turnContext: { + allTurns: [], + window: [], + transcript: "user: hi", + guidedBlock: "", + historyElided: false, + isFirstTurn: true, + includeFullSnapshot: true, + }, + workoutEvidence: null, + systemPrompt: "STABLE SYSTEM", + userPrompt: "SNAPSHOT AND CONVERSATION", + turnNotes, + } as unknown as TurnContext; +} + +function run(turnNotes: string, turnHints: string[], toolMode: boolean) { + return runTurnModel({ + userId: "u1", + locale: "en", + signal: new AbortController().signal, + conversationId: "c1", + ctx: ctx(turnNotes), + chain: [] as never, + toolMode, + ledger: { + settleRound: vi.fn(async () => {}), + reserveRound: vi.fn(async () => true), + close: vi.fn(async () => {}), + } as never, + emitter: { emit: () => {}, aborted: () => false }, + turnHints, + message: "hi", + payer: "user", + conversationWindowSet: false, + }).catch(() => null); +} + +beforeEach(() => { + m.loop.mockReset().mockRejectedValue(new Error("stop after capture")); + m.stream.mockReset().mockRejectedValue(new Error("stop after capture")); +}); + +describe("runTurnModel keeps the system prompt stable (#1217)", () => { + it("on the tool path", async () => { + await run("OPENER HINT for this turn: A", [], true); + await run("OPENER HINT for this turn: B", ["FOLLOW-UP: the table"], true); + const [first, second] = m.loop.mock.calls.map( + ([args]) => + args as { system: string; messages: Array<{ content: string }> }, + ); + expect(sha(first!.system)).toBe(sha(second!.system)); + expect(first!.system).not.toContain("OPENER HINT"); + expect(second!.system).not.toContain("FOLLOW-UP"); + expect(first!.messages[0]!.content).toContain( + "OPENER HINT for this turn: A", + ); + expect(second!.messages[0]!.content).toContain("FOLLOW-UP: the table"); + }); + + it("on the streaming no-tools path", async () => { + await run("OPENER HINT for this turn: A", [], false); + await run("OPENER HINT for this turn: B", ["FOLLOW-UP: the table"], false); + const params = m.stream.mock.calls.map( + ([args]) => + ( + args as { + params: { system: string; messages: Array<{ content: string }> }; + } + ).params, + ); + expect(params[0]!.system).toBe("STABLE SYSTEM"); + expect(params[1]!.system).toBe("STABLE SYSTEM"); + const users = params.map((p) => JSON.stringify(p.messages)); + expect(users[0]).toContain("OPENER HINT for this turn: A"); + expect(users[1]).toContain("FOLLOW-UP: the table"); + }); +}); diff --git a/src/lib/ai/coach/turn/addenda/index.ts b/src/lib/ai/coach/turn/addenda/index.ts index 20f70cf04..6efb8ce3d 100644 --- a/src/lib/ai/coach/turn/addenda/index.ts +++ b/src/lib/ai/coach/turn/addenda/index.ts @@ -31,3 +31,16 @@ export function buildDialogAddenda( .filter((block) => block.length > 0) .join("\n\n"); } + +/** + * v1.43 (#1217) — the table rules alone (how a table was fetched, how to + * follow up on one). They apply once the conversation holds a table, which can + * change mid-turn, so they travel as a user-side note rather than switching the + * system prompt: a switched system prompt rewrites the provider's cached + * prefix on the costliest round of the turn. + */ +export function buildTableRulesAddendum(locale: Locale): string { + return [methodAddendum(locale), followUpsAddendum(locale)] + .filter((block) => block.length > 0) + .join("\n\n"); +} diff --git a/src/lib/ai/coach/turn/context.ts b/src/lib/ai/coach/turn/context.ts index b7e3ce1c0..d6dd8584e 100644 --- a/src/lib/ai/coach/turn/context.ts +++ b/src/lib/ai/coach/turn/context.ts @@ -13,7 +13,10 @@ import type { CoachScope } from "@/lib/ai/coach/types"; import { enqueueCoachMemoryRefresh } from "@/lib/ai/coach/coach-memory-shared"; import { instructionLocale } from "@/lib/ai/prompts/output-language"; import { storeDeterministicFacts } from "@/lib/ai/coach/facts"; -import { getCoachSystemPrompt } from "@/lib/ai/coach/system-prompt"; +import { + buildCoachTurnNotes, + getCoachSystemPrompt, +} from "@/lib/ai/coach/system-prompt"; import { openerArchetypeHint, shouldUseNameForTurn, @@ -69,6 +72,11 @@ export interface TurnContext { workoutEvidence: Record | null; systemPrompt: string; userPrompt: string; + /** + * v1.43 (#1217) — the lines that change every turn (opener hint, name + * clause), for the user message. Empty when none apply. + */ + turnNotes: string; } export async function assembleTurnContext(args: { @@ -150,12 +158,11 @@ export async function assembleTurnContext(args: { firstName != null && shouldUseNameForTurn(`${userId}:${turnIndex}`), openerHint: openerArchetypeHint(`${userId}:${turnIndex}`, locale), }; - const baseSystemPrompt = getCoachSystemPrompt( - locale, - coachPrefs, - aboutMe, - coachPersonalization, - ); + // v1.43 (#1217) — the system prompt is stable per person; the per-turn + // personalization travels in the user message (`turnNotes`), so the + // provider's cached prefix survives from one turn to the next. + const baseSystemPrompt = getCoachSystemPrompt(locale, coachPrefs, aboutMe); + const turnNotes = buildCoachTurnNotes(locale, coachPersonalization); const turnContext = buildCoachTurnContext({ priorTurns, priorSummary, @@ -240,5 +247,6 @@ export async function assembleTurnContext(args: { workoutEvidence, systemPrompt, userPrompt, + turnNotes, }; } diff --git a/src/lib/ai/coach/turn/model.ts b/src/lib/ai/coach/turn/model.ts index f8c7996f7..b39c3ffb1 100644 --- a/src/lib/ai/coach/turn/model.ts +++ b/src/lib/ai/coach/turn/model.ts @@ -80,7 +80,7 @@ import { type PriorResultTurn, } from "@/lib/ai/coach/results/refs"; -import { buildDialogAddenda } from "./addenda"; +import { buildDialogAddenda, buildTableRulesAddendum } from "./addenda"; import type { TurnLedger } from "./budget"; import type { TurnReasoning } from "./reasoning"; import type { TurnChain } from "./chain"; @@ -228,7 +228,7 @@ export function buildTurnResults( } } -/** v1.39.4 — server-authored lines appended to the system prompt. */ +/** v1.39.4 — server-authored lines appended to a prompt block. */ function appendBlocks(base: string, blocks: string[]): string { const extra = blocks.filter((block) => block.length > 0); return extra.length > 0 ? [base, ...extra].join("\n\n") : base; @@ -374,28 +374,35 @@ export async function runTurnModel(args: { reach: ctx.reach, prior: args.priorResults ?? [], }); - const requestWith = (tableRules: boolean) => - buildCoachToolRequest({ - systemPrompt: ctx.systemPrompt, - toolModeAddendum: appendBlocks(buildToolModeAddendum(locale), [ - buildDialogAddenda(locale, { tableRules }), - ...args.turnHints, - ]), - focusHint: renderFocusHint(effectiveScope?.sources), - workoutEvidence, - dataInventory: renderDataInventory(inventory), - priorResults: renderPriorResultRefs(priorResults), - guidedBlock: turnContext.guidedBlock, - transcript: turnContext.transcript, - languageName: LANGUAGE_NAMES[locale], - ...(memory ? { memoryBlock: memory.text } : {}), - }); - // The recheck and follow-up rules are about tables: they ride the - // prompt once the conversation holds one, or from the round after this - // turn produced its first. - const withTables = requestWith(true); - const toolRequest = - priorResults.length > 0 ? withTables : requestWith(false); + // The recheck and follow-up rules are about tables: they ride the turn + // once the conversation holds one, or from the round after this turn + // produced its first. v1.43 (#1217) — like every other line that can + // change from one turn or round to the next (the opener hint, the name + // clause, a resolved follow-up), they travel on the user side, so the + // system prompt and the tools stay one cached prefix for the whole + // conversation. + const tableRules = buildTableRulesAddendum(locale); + const toolRequest = buildCoachToolRequest({ + systemPrompt: ctx.systemPrompt, + toolModeAddendum: appendBlocks(buildToolModeAddendum(locale), [ + buildDialogAddenda(locale, { tableRules: false }), + ]), + focusHint: renderFocusHint(effectiveScope?.sources), + workoutEvidence, + dataInventory: renderDataInventory(inventory), + priorResults: renderPriorResultRefs(priorResults), + guidedBlock: turnContext.guidedBlock, + transcript: turnContext.transcript, + languageName: LANGUAGE_NAMES[locale], + turnNotes: [ + ...(priorResults.length > 0 ? [tableRules] : []), + ctx.turnNotes, + ...args.turnHints, + ] + .filter((block) => block.length > 0) + .join("\n\n"), + ...(memory ? { memoryBlock: memory.text } : {}), + }); // v1.39.4 — every call this turn settled, by its turn-wide index, for // the result tables. Only this turn's own calls ever reach them. const settled: SettledToolCall[] = []; @@ -420,7 +427,7 @@ export async function runTurnModel(args: { userId, providers: chain, system: toolRequest.system, - systemOnceTableShown: withTables.system, + ...(priorResults.length > 0 ? {} : { tableRulesOnceShown: tableRules }), messages: toolRequest.messages, tools: COACH_TURN_TOOL_DEFS, temperature: AI_BUDGETS.coach.temperature, @@ -631,10 +638,18 @@ export async function runTurnModel(args: { }); const summaries: string[] = []; // v1.41 — the memory block rides the user turn here too, ahead of the - // snapshot, fenced like the rest of the person's own context. - const user = memory - ? `${memory.text}\n\n${ctx.userPrompt}` - : ctx.userPrompt; + // snapshot, fenced like the rest of the person's own context. v1.43 + // (#1217) — so do this turn's own lines (opener hint, name clause, a + // resolved follow-up), which used to ride the system prompt and rewrote + // its cached prefix on every turn. + const user = [ + memory?.text, + ctx.turnNotes, + ...args.turnHints, + ctx.userPrompt, + ] + .filter((block): block is string => Boolean(block)) + .join("\n\n"); const fallback = await runStreamingRawCompletionWithFallback({ surface: "coach", userId, @@ -644,7 +659,7 @@ export async function runTurnModel(args: { }, params: { ...singleUserTurn({ - system: appendBlocks(ctx.systemPrompt, args.turnHints), + system: ctx.systemPrompt, user, temperature: AI_BUDGETS.coach.temperature, maxTokens: AI_BUDGETS.coach.maxTokens,