From 7530324fd1a044db3e7a22d5ea5173c307c8906b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 20:57:37 +0200 Subject: [PATCH 1/9] refactor(ai): centralise per-surface generation budgets Replace the scattered `?? 0.3` / `?? 1000` temperature and maxTokens magic numbers across the status provider, Coach route, comprehensive generator and period-narrative generator with a single typed `ai-budgets.ts` map. Each entry documents the output contract that fixes its token ceiling. Right-size the per-metric status output from 1000 to 250 tokens: the card renders one `{ "summary": "..." }` of 2-4 sentences (30-60 words), so 1000 was an order of magnitude over the real ceiling. --- src/app/api/insights/chat/route.ts | 5 +- src/lib/ai/__tests__/ai-budgets.test.ts | 39 +++++++++ src/lib/ai/ai-budgets.ts | 85 +++++++++++++++++++ src/lib/insights/comprehensive-generate.ts | 5 +- .../narrative/period-narrative-generate.ts | 5 +- src/lib/insights/status-provider.ts | 5 +- 6 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 src/lib/ai/__tests__/ai-budgets.test.ts create mode 100644 src/lib/ai/ai-budgets.ts diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index 93311440c..09ecf7d5d 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -46,6 +46,7 @@ import { resolveProviderChain, resolveProvider } from "@/lib/ai/provider"; import { assertConsentForChain } from "@/lib/ai/consent-guard"; import type { CompletionResult } from "@/lib/ai/types"; import { PROMPT_VERSION } from "@/lib/ai/prompts/insight-generator"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; import { coachChatRequestSchema, @@ -439,8 +440,8 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`; params: { systemPrompt, userPrompt, - temperature: 0.4, - maxTokens: 600, + temperature: AI_BUDGETS.coach.temperature, + maxTokens: AI_BUDGETS.coach.maxTokens, }, }); result = fallback.result; diff --git a/src/lib/ai/__tests__/ai-budgets.test.ts b/src/lib/ai/__tests__/ai-budgets.test.ts new file mode 100644 index 000000000..7741da12f --- /dev/null +++ b/src/lib/ai/__tests__/ai-budgets.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; + +import { AI_BUDGETS, REFERENCE_AI_SEED } from "../ai-budgets"; + +describe("AI_BUDGETS", () => { + it("right-sizes the per-metric status output to its 30-60 word contract", () => { + // The status card renders exactly one `{ "summary": "..." }` of 2-4 + // sentences; 1000 was over-generous. 250 caps the ceiling without + // clipping the contract. + expect(AI_BUDGETS.status.maxTokens).toBe(250); + expect(AI_BUDGETS.status.temperature).toBe(0.3); + }); + + it("keeps the comprehensive briefing budget at 1500 / 0.3", () => { + expect(AI_BUDGETS.comprehensive.maxTokens).toBe(1500); + expect(AI_BUDGETS.comprehensive.temperature).toBe(0.3); + }); + + it("keeps the narrative budget at 400 / 0.3", () => { + expect(AI_BUDGETS.narrative.maxTokens).toBe(400); + expect(AI_BUDGETS.narrative.temperature).toBe(0.3); + }); + + it("keeps the coach budget at 600 / 0.4", () => { + expect(AI_BUDGETS.coach.maxTokens).toBe(600); + expect(AI_BUDGETS.coach.temperature).toBe(0.4); + }); + + it("keeps the worker budgets (summary / facts / self-context)", () => { + expect(AI_BUDGETS.summary).toEqual({ temperature: 0.3, maxTokens: 200 }); + expect(AI_BUDGETS.facts).toEqual({ temperature: 0.2, maxTokens: 300 }); + expect(AI_BUDGETS.selfContext.temperature).toBe(0.4); + }); + + it("exposes a stable reference seed constant", () => { + expect(typeof REFERENCE_AI_SEED).toBe("number"); + expect(Number.isInteger(REFERENCE_AI_SEED)).toBe(true); + }); +}); diff --git a/src/lib/ai/ai-budgets.ts b/src/lib/ai/ai-budgets.ts new file mode 100644 index 000000000..ce6c49434 --- /dev/null +++ b/src/lib/ai/ai-budgets.ts @@ -0,0 +1,85 @@ +/** + * Per-surface AI generation budgets — the single source of truth for the + * `temperature` and `maxTokens` each AI surface sends to the provider. + * + * Before this module the values were scattered as `?? 0.3` / `?? 1000` + * magic numbers across the status provider, the Coach route, the + * comprehensive generator and the period-narrative generator. Centralising + * them keeps the cost ceiling auditable in one place and ties every number + * to the output contract it serves (commented per entry below). + * + * `seed` (where present) is a deterministic constant pinned for the + * reference/reproducible surfaces (per the v1.18.7 AI audit MEDIUM-4): + * the status cards and the period narrative are reference assessments, so a + * fixed seed makes a prompt change diff-able against a stable baseline. The + * daily-briefing paragraph is deliberately NOT seeded (it re-rolls for + * phrasing variety — see comprehensive-generate.ts). + */ + +export interface AiBudget { + /** Sampling temperature. Low for reference surfaces, slightly higher for chat. */ + temperature: number; + /** Output-token ceiling. Sized to the surface's documented output contract. */ + maxTokens: number; +} + +/** + * Deterministic seed pinned for the reproducible reference surfaces. A stable + * constant (not a per-request value) so two runs of the same prompt+snapshot + * on a seed-aware provider produce byte-identical text — the QA-attribution + * baseline the audit's MEDIUM-4 calls for. + */ +export const REFERENCE_AI_SEED = 1_618_033 as const; + +/** + * Surface budgets. Each entry's comment states the output contract that + * fixes its `maxTokens`. + */ +export const AI_BUDGETS = { + /** + * Comprehensive insight + dailyBriefing — the largest structured payload + * (summary + recommendations + dailyBriefing 80-200 words + signals + + * keyFindings + trendAnnotations + storyboardAnnotations). 1500 tokens. + */ + comprehensive: { temperature: 0.3, maxTokens: 1500 }, + + /** + * Per-metric status assessment cards — output is a single + * `{ "summary": "..." }` of 2-4 sentences / 30-60 words + * (base-system.ts `length` section). ~80 tokens of content + JSON + * envelope; 250 is a comfortable ceiling (was an over-generous 1000). + */ + status: { temperature: 0.3, maxTokens: 250 }, + + /** + * Period narrative (week/month) — 2-4 short plain-text sentences + * (period-narrative-generate.ts SYSTEM_PROMPT). 400 tokens. + */ + narrative: { temperature: 0.3, maxTokens: 400 }, + + /** + * Coach SSE chat — a 60-180 word prose reply plus an optional evidence + * sentinel block (system-prompt.ts rule 1). 600 tokens. Slightly higher + * temperature than the reference surfaces for conversational variety. + */ + coach: { temperature: 0.4, maxTokens: 600 }, + + /** + * Coach rolling-conversation summary worker — a compact summary of the + * elided older turns. 200 tokens. + */ + summary: { temperature: 0.3, maxTokens: 200 }, + + /** + * Coach durable-fact extraction worker — a short structured fact list. + * 300 tokens. Lowest temperature: extraction must be faithful, not + * creative. + */ + facts: { temperature: 0.2, maxTokens: 300 }, + + /** + * Coach self-context question generation — a small set of clarifying + * questions. Temperature only; the caller pins its own token const. + */ + selfContext: { temperature: 0.4 }, +} as const satisfies Record; diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 139319d65..9d3ce33e8 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -74,6 +74,7 @@ import { metricStatusScope, } from "@/lib/insights/metric-status-registry"; import { annotate } from "@/lib/logging/context"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; const CACHE_TTL_MS = 24 * 60 * 60 * 1000; @@ -697,8 +698,8 @@ export async function generateComprehensiveInsight( params: { systemPrompt: buildSystemPromptWithReferences(locale, referenceMetrics), userPrompt, - temperature: 0.3, - maxTokens: 1500, + temperature: AI_BUDGETS.comprehensive.temperature, + maxTokens: AI_BUDGETS.comprehensive.maxTokens, }, }); result = fallback.result; diff --git a/src/lib/insights/narrative/period-narrative-generate.ts b/src/lib/insights/narrative/period-narrative-generate.ts index d383d4cb7..d1c414036 100644 --- a/src/lib/insights/narrative/period-narrative-generate.ts +++ b/src/lib/insights/narrative/period-narrative-generate.ts @@ -29,6 +29,7 @@ import { prisma as defaultPrisma } from "@/lib/db"; import { encrypt, decrypt } from "@/lib/crypto"; import { wallClockInTz } from "@/lib/tz/wall-clock"; import { annotate } from "@/lib/logging/context"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; import { runStatusCompletion } from "@/lib/insights/status-provider"; import { buildPeriodNarrativeContext, @@ -261,8 +262,8 @@ export async function generatePeriodNarrative( consentSurface: "insights", systemPrompt: locale === "de" ? SYSTEM_PROMPT_DE : SYSTEM_PROMPT_EN, userPrompt: buildNarrativeUserPrompt(context, locale), - temperature: 0.3, - maxTokens: 400, + temperature: AI_BUDGETS.narrative.temperature, + maxTokens: AI_BUDGETS.narrative.maxTokens, }); // A provider error / timeout is non-fatal — the last good row stays as-is diff --git a/src/lib/insights/status-provider.ts b/src/lib/insights/status-provider.ts index 12a66484a..104704e82 100644 --- a/src/lib/insights/status-provider.ts +++ b/src/lib/insights/status-provider.ts @@ -10,6 +10,7 @@ import { type ConsentSurface, } from "@/lib/ai/consent-guard"; import { annotate } from "@/lib/logging/context"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; import { STATUS_PROVIDER_TIMEOUT_MS, withTimeout } from "./with-timeout"; /** @@ -161,8 +162,8 @@ export async function runStatusCompletion( params: { systemPrompt, userPrompt, - temperature: args.temperature ?? 0.3, - maxTokens: args.maxTokens ?? 1000, + temperature: args.temperature ?? AI_BUDGETS.status.temperature, + maxTokens: args.maxTokens ?? AI_BUDGETS.status.maxTokens, }, }), STATUS_PROVIDER_TIMEOUT_MS, From a460c74e6c0b215f6e9e15084e46f1c840f60175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 20:58:46 +0200 Subject: [PATCH 2/9] feat(ai): support a deterministic seed on the reference surfaces Add an optional `seed` to CompletionParams and thread it onto the OpenAI and local (Ollama / OpenAI-compatible) request bodies. The field is dropped when unset, so existing callers are byte-identical. Pin a stable seed for the status/reference cards (defaulted in the status provider) and the period narrative, so a prompt change is diff-able against a fixed baseline rather than masked by low-temperature non-determinism. Anthropic's Messages API has no seed knob, so its client ignores the field (documented inline). The seed is annotated on the narrative wide-event. --- src/lib/ai/__tests__/openai-client.test.ts | 29 +++++++++++++++++++ src/lib/ai/anthropic-client.ts | 3 ++ src/lib/ai/local-client.ts | 4 +++ src/lib/ai/openai-client.ts | 4 +++ src/lib/ai/types.ts | 7 +++++ .../narrative/period-narrative-generate.ts | 7 +++-- src/lib/insights/status-provider.ts | 11 ++++++- 7 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/lib/ai/__tests__/openai-client.test.ts b/src/lib/ai/__tests__/openai-client.test.ts index 2dfb52125..ac7fa1dd9 100644 --- a/src/lib/ai/__tests__/openai-client.test.ts +++ b/src/lib/ai/__tests__/openai-client.test.ts @@ -39,6 +39,35 @@ describe("OpenAIClient", () => { expect(body.model).toBe("gpt-4o-mini"); expect(body.messages).toHaveLength(2); expect(body.response_format).toEqual({ type: "json_object" }); + // No seed passed → field omitted from the body entirely. + expect(body).not.toHaveProperty("seed"); + }); + + it("threads a deterministic seed onto the request body when supplied", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + choices: [{ message: { content: '{"summary":"x"}' } }], + usage: { total_tokens: 5 }, + }), + }); + vi.stubGlobal("fetch", mockFetch); + + const client = new OpenAIClient({ + apiKey: "sk-test", + model: "gpt-4o-mini", + baseUrl: "https://api.openai.com/v1", + }); + + await client.generateCompletion({ + systemPrompt: "test", + userPrompt: "test", + seed: 1234, + }); + + const body = JSON.parse(mockFetch.mock.calls[0][1].body); + expect(body.seed).toBe(1234); }); it("throws on non-ok response", async () => { diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index 4228fd322..b58d8e317 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -45,6 +45,9 @@ export class AnthropicClient implements AIProvider { "x-api-key": this.config.apiKey, "anthropic-version": ANTHROPIC_VERSION, }, + // NOTE: Anthropic's Messages API has no `seed` parameter, so + // `params.seed` is intentionally not forwarded here — output on this + // provider is non-deterministic regardless of the pinned seed. body: JSON.stringify({ model: this.config.model, max_tokens: params.maxTokens ?? 1000, diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index df03ff991..4e69db65a 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -65,6 +65,10 @@ export class LocalOpenAICompatibleClient implements AIProvider { ], temperature: params.temperature ?? 0.3, max_tokens: params.maxTokens ?? 1000, + // Deterministic seed for reproducible reference output. Ollama + + // most OpenAI-compatible local servers honour it; servers that + // ignore it simply disregard the field. Omitted when unset. + ...(params.seed !== undefined ? { seed: params.seed } : {}), }), }, { timeoutMs: 60_000, requirePublicHost: !allowPrivate }, diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index a898acaed..7260e6320 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -48,6 +48,10 @@ export class OpenAIClient implements AIProvider { temperature: params.temperature ?? 0.3, max_tokens: params.maxTokens ?? 1000, response_format: { type: "json_object" }, + // Deterministic seed for reproducible reference output; omitted + // (undefined → dropped by JSON.stringify) when the caller does + // not pin one (e.g. the seedless daily-briefing re-roll). + ...(params.seed !== undefined ? { seed: params.seed } : {}), }), }, // 60 s ceiling so a tar-pit upstream cannot pin a worker diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index a6dd524d2..54345b244 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -119,6 +119,13 @@ export interface CompletionParams { userPrompt: string; temperature?: number; maxTokens?: number; + /** + * Optional deterministic seed. Threaded onto the OpenAI and local + * (Ollama / OpenAI-compatible) request bodies for reproducible output on + * the reference surfaces (status cards + period narrative). Anthropic's + * Messages API has no seed knob, so the Anthropic client ignores it. + */ + seed?: number; } export interface CompletionResult { diff --git a/src/lib/insights/narrative/period-narrative-generate.ts b/src/lib/insights/narrative/period-narrative-generate.ts index d1c414036..d5be8d839 100644 --- a/src/lib/insights/narrative/period-narrative-generate.ts +++ b/src/lib/insights/narrative/period-narrative-generate.ts @@ -29,7 +29,7 @@ import { prisma as defaultPrisma } from "@/lib/db"; import { encrypt, decrypt } from "@/lib/crypto"; import { wallClockInTz } from "@/lib/tz/wall-clock"; import { annotate } from "@/lib/logging/context"; -import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { AI_BUDGETS, REFERENCE_AI_SEED } from "@/lib/ai/ai-budgets"; import { runStatusCompletion } from "@/lib/insights/status-provider"; import { buildPeriodNarrativeContext, @@ -264,6 +264,9 @@ export async function generatePeriodNarrative( userPrompt: buildNarrativeUserPrompt(context, locale), temperature: AI_BUDGETS.narrative.temperature, maxTokens: AI_BUDGETS.narrative.maxTokens, + // v1.18.7 — reference surface: pin a deterministic seed so a prompt + // change is diff-able against a stable baseline (MEDIUM-4). + seed: REFERENCE_AI_SEED, }); // A provider error / timeout is non-fatal — the last good row stays as-is @@ -316,7 +319,7 @@ export async function generatePeriodNarrative( }); annotate({ action: { name: "insights.narrative.generated" }, - meta: { period, locale, provider: providerType }, + meta: { period, locale, provider: providerType, seed: REFERENCE_AI_SEED }, }); }; diff --git a/src/lib/insights/status-provider.ts b/src/lib/insights/status-provider.ts index 104704e82..7be41ada2 100644 --- a/src/lib/insights/status-provider.ts +++ b/src/lib/insights/status-provider.ts @@ -10,7 +10,7 @@ import { type ConsentSurface, } from "@/lib/ai/consent-guard"; import { annotate } from "@/lib/logging/context"; -import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { AI_BUDGETS, REFERENCE_AI_SEED } from "@/lib/ai/ai-budgets"; import { STATUS_PROVIDER_TIMEOUT_MS, withTimeout } from "./with-timeout"; /** @@ -51,6 +51,12 @@ interface RunStatusCompletionArgs { userPrompt: string; temperature?: number; maxTokens?: number; + /** + * v1.18.7 — optional deterministic seed override. Defaults to + * `REFERENCE_AI_SEED` for every status/reference surface (reproducible + * QA); the period narrative passes the same constant explicitly. + */ + seed?: number; /** * v1.12.1 — which AI surface this generation serves, for the consent gate. * `insights` for the per-metric status cards + period narrative; `coach` @@ -164,6 +170,9 @@ export async function runStatusCompletion( userPrompt, temperature: args.temperature ?? AI_BUDGETS.status.temperature, maxTokens: args.maxTokens ?? AI_BUDGETS.status.maxTokens, + // v1.18.7 — status/reference output is reproducible: pin the + // deterministic seed unless a caller overrides it. + seed: args.seed ?? REFERENCE_AI_SEED, }, }), STATUS_PROVIDER_TIMEOUT_MS, From a61b7fe4ad85fa5807c016a7f670d01c3e766a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:01:27 +0200 Subject: [PATCH 3/9] feat(ai): force JSON on the non-OpenAI provider chains Add an opt-in `responseFormat: "json"` to CompletionParams that the structured surfaces (comprehensive briefing + per-metric status cards) set. On local (Ollama / OpenAI-compatible) it adds `format: "json"`; on Anthropic it prefills the assistant turn with `{` and re-prepends the brace to the reply, so the first emitted token is already inside a JSON object. Both cut the first-pass JSON-failure rate that fence-stripping otherwise has to absorb. OpenAI already pins `response_format: json_object` and is unchanged. The period narrative (plain text) and the Coach (streaming prose) leave the flag unset, so their non-JSON paths are untouched. --- src/lib/ai/__tests__/anthropic-client.test.ts | 59 +++++++++++++++++++ src/lib/ai/__tests__/local-client.test.ts | 31 ++++++++++ src/lib/ai/anthropic-client.ts | 29 ++++++++- src/lib/ai/local-client.ts | 5 ++ src/lib/ai/types.ts | 11 ++++ src/lib/insights/comprehensive-generate.ts | 4 ++ .../narrative/period-narrative-generate.ts | 3 + src/lib/insights/status-provider.ts | 10 ++++ 8 files changed, 149 insertions(+), 3 deletions(-) diff --git a/src/lib/ai/__tests__/anthropic-client.test.ts b/src/lib/ai/__tests__/anthropic-client.test.ts index b8aa85683..3db2580da 100644 --- a/src/lib/ai/__tests__/anthropic-client.test.ts +++ b/src/lib/ai/__tests__/anthropic-client.test.ts @@ -53,6 +53,65 @@ describe("AnthropicClient", () => { expect(body.messages[0].content.toLowerCase()).toContain("json"); }); + it("prefills the assistant turn with `{` and re-prepends it for JSON surfaces", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + // Model continues from the prefilled `{`, so its text omits it. + content: [{ type: "text", text: '"summary":"ok"}' }], + usage: { input_tokens: 5, output_tokens: 5 }, + }), + }); + vi.stubGlobal("fetch", mockFetch); + + const client = new AnthropicClient({ + apiKey: "sk-ant-test", + model: "claude-3-5-sonnet-latest", + }); + + const result = await client.generateCompletion({ + systemPrompt: "s", + userPrompt: "u", + responseFormat: "json", + }); + + // The returned content is the complete object (the `{` re-prepended). + expect(result.content).toBe('{"summary":"ok"}'); + expect(JSON.parse(result.content)).toEqual({ summary: "ok" }); + + const body = JSON.parse(mockFetch.mock.calls[0][1].body); + expect(body.messages).toHaveLength(2); + expect(body.messages[1]).toEqual({ role: "assistant", content: "{" }); + }); + + it("does NOT prefill for the prose (non-JSON) path", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + content: [{ type: "text", text: "Just some prose." }], + usage: { input_tokens: 1, output_tokens: 1 }, + }), + }); + vi.stubGlobal("fetch", mockFetch); + + const client = new AnthropicClient({ + apiKey: "sk-ant-test", + model: "claude-3-5-sonnet-latest", + }); + + const result = await client.generateCompletion({ + systemPrompt: "s", + userPrompt: "u", + }); + + expect(result.content).toBe("Just some prose."); + const body = JSON.parse(mockFetch.mock.calls[0][1].body); + expect(body.messages).toHaveLength(1); + expect(body.messages[0].role).toBe("user"); + }); + it("uses custom baseUrl when provided", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, diff --git a/src/lib/ai/__tests__/local-client.test.ts b/src/lib/ai/__tests__/local-client.test.ts index e0f6f70be..ba6c38ffb 100644 --- a/src/lib/ai/__tests__/local-client.test.ts +++ b/src/lib/ai/__tests__/local-client.test.ts @@ -55,6 +55,37 @@ describe("LocalOpenAICompatibleClient", () => { // CRUCIAL: do not send response_format because many local servers reject it. expect(body).not.toHaveProperty("response_format"); + // No JSON opt-in → no `format` field. + expect(body).not.toHaveProperty("format"); + }); + + it("sends Ollama `format: json` only when the caller opts into JSON", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + choices: [{ message: { content: '{"summary":"x"}' } }], + usage: { total_tokens: 3 }, + }), + }); + vi.stubGlobal("fetch", mockFetch); + + const client = new LocalOpenAICompatibleClient({ + apiKey: null, + model: "llama3:8b", + baseUrl: "http://localhost:11434/v1", + }); + + await client.generateCompletion({ + systemPrompt: "s", + userPrompt: "u", + responseFormat: "json", + seed: 99, + }); + + const body = JSON.parse(mockFetch.mock.calls[0][1].body); + expect(body.format).toBe("json"); + expect(body.seed).toBe(99); }); it("omits Authorization header when no API key is provided", async () => { diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index b58d8e317..87ac4d5af 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -48,12 +48,26 @@ export class AnthropicClient implements AIProvider { // NOTE: Anthropic's Messages API has no `seed` parameter, so // `params.seed` is intentionally not forwarded here — output on this // provider is non-deterministic regardless of the pinned seed. + // + // v1.18.7 — JSON-reliability prefill. For the structured surfaces + // (`responseFormat: "json"`) we seed the assistant turn with a bare + // `{`. Anthropic continues from that token, so the first emitted + // character is already inside a JSON object — this drops the + // first-pass "prose preamble before the JSON" failure the + // fence-stripping net otherwise has to catch. We re-prepend the `{` + // to the returned text below so the caller sees a complete object. body: JSON.stringify({ model: this.config.model, max_tokens: params.maxTokens ?? 1000, temperature: params.temperature ?? 0.3, system: params.systemPrompt, - messages: [{ role: "user", content: wrapForJson(params.userPrompt) }], + messages: + params.responseFormat === "json" + ? [ + { role: "user", content: wrapForJson(params.userPrompt) }, + { role: "assistant", content: "{" }, + ] + : [{ role: "user", content: wrapForJson(params.userPrompt) }], }), }, // 60 s ceiling — see openai-client.ts for the rationale. @@ -89,12 +103,21 @@ export class AnthropicClient implements AIProvider { }; const textBlock = json.content?.find((c) => c.type === "text"); - const content = textBlock?.text; + const rawText = textBlock?.text; - if (!content) { + if (!rawText) { throw new Error("Anthropic returned empty content"); } + // When we prefilled the assistant turn with `{`, the model continues + // from there and its returned text omits that leading brace — re-prepend + // it so the caller parses a complete object. Guard against a model that + // (rarely) echoes the prefill itself. + const content = + params.responseFormat === "json" && !rawText.trimStart().startsWith("{") + ? `{${rawText}` + : rawText; + const inputTokens = json.usage?.input_tokens ?? 0; const outputTokens = json.usage?.output_tokens ?? 0; const tokensUsed = inputTokens + outputTokens || null; diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index 4e69db65a..905e4e62b 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -69,6 +69,11 @@ export class LocalOpenAICompatibleClient implements AIProvider { // most OpenAI-compatible local servers honour it; servers that // ignore it simply disregard the field. Omitted when unset. ...(params.seed !== undefined ? { seed: params.seed } : {}), + // v1.18.7 — Ollama's native structured-output switch. The JSON + // surfaces opt in via `responseFormat: "json"`; it cuts the + // first-pass JSON-failure rate that the fence-stripping safety net + // otherwise has to catch. The Coach (prose) leaves it unset. + ...(params.responseFormat === "json" ? { format: "json" } : {}), }), }, { timeoutMs: 60_000, requirePublicHost: !allowPrivate }, diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index 54345b244..d5844da2c 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -126,6 +126,17 @@ export interface CompletionParams { * Messages API has no seed knob, so the Anthropic client ignores it. */ seed?: number; + /** + * v1.18.7 — opt the structured (JSON) surfaces into the provider's + * strongest JSON-reliability mode. When `"json"`: + * - local (Ollama / OpenAI-compatible): adds `format: "json"`. + * - Anthropic: prefills the assistant turn with `{` so the first token + * is forced into a JSON object. + * The OpenAI client already pins `response_format: json_object` + * unconditionally and is unaffected. The Coach (prose, NOT JSON) leaves + * this unset, so its streaming path is untouched. + */ + responseFormat?: "json"; } export interface CompletionResult { diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 9d3ce33e8..fab7310de 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -700,6 +700,10 @@ export async function generateComprehensiveInsight( userPrompt, temperature: AI_BUDGETS.comprehensive.temperature, maxTokens: AI_BUDGETS.comprehensive.maxTokens, + // v1.18.7 — structured surface: opt the non-OpenAI chains into their + // strongest JSON mode (Ollama `format`, Anthropic `{` prefill) so a + // first-pass JSON miss is rarer; stripJsonFences stays the net. + responseFormat: "json", }, }); result = fallback.result; diff --git a/src/lib/insights/narrative/period-narrative-generate.ts b/src/lib/insights/narrative/period-narrative-generate.ts index d5be8d839..55c63c73c 100644 --- a/src/lib/insights/narrative/period-narrative-generate.ts +++ b/src/lib/insights/narrative/period-narrative-generate.ts @@ -267,6 +267,9 @@ export async function generatePeriodNarrative( // v1.18.7 — reference surface: pin a deterministic seed so a prompt // change is diff-able against a stable baseline (MEDIUM-4). seed: REFERENCE_AI_SEED, + // The narrative output is PLAIN TEXT (no markdown, no JSON), so suppress + // the JSON-mode opt-in the status cards default into. + responseFormat: "text", }); // A provider error / timeout is non-fatal — the last good row stays as-is diff --git a/src/lib/insights/status-provider.ts b/src/lib/insights/status-provider.ts index 7be41ada2..cbe32d023 100644 --- a/src/lib/insights/status-provider.ts +++ b/src/lib/insights/status-provider.ts @@ -57,6 +57,13 @@ interface RunStatusCompletionArgs { * QA); the period narrative passes the same constant explicitly. */ seed?: number; + /** + * v1.18.7 — output contract of this generation. The per-metric status + * cards return a JSON `{ "summary": ... }` (the default), so they opt the + * non-OpenAI chains into their strongest JSON mode. The period narrative + * returns PLAIN TEXT and passes `"text"` to suppress that. + */ + responseFormat?: "json" | "text"; /** * v1.12.1 — which AI surface this generation serves, for the consent gate. * `insights` for the per-metric status cards + period narrative; `coach` @@ -173,6 +180,9 @@ export async function runStatusCompletion( // v1.18.7 — status/reference output is reproducible: pin the // deterministic seed unless a caller overrides it. seed: args.seed ?? REFERENCE_AI_SEED, + // Status cards are JSON by default; the narrative opts out via + // `"text"`. + responseFormat: args.responseFormat === "text" ? undefined : "json", }, }), STATUS_PROVIDER_TIMEOUT_MS, From 853a1cd301d72484f4ec6be14be9a2f35eb17b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:05:19 +0200 Subject: [PATCH 4/9] feat(insights): vary daily-briefing prose on unchanged data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content-hash gate re-stamps the prior text whenever the snapshot is unchanged, so the "daily" briefing read byte-stable day-over-day. Decouple phrasing freshness from data freshness for the dailyBriefing.paragraph alone: on a hash-unchanged hit, re-roll just the paragraph once per calendar day at a higher, seedless temperature, keeping every structured finding verbatim from the cached payload. A new `insightsBriefingRerollDate` column gates it to at most one extra call per day, and a re-roll miss still stamps the day so a failing provider cannot be re-driven on every page-open. The hash gate stays fully intact for the status cards (reference assessments — monotony is fine). --- .../migration.sql | 5 + prisma/schema.prisma | 7 + .../comprehensive-briefing-reroll.test.ts | 182 ++++++++++++++++ src/lib/insights/comprehensive-generate.ts | 201 +++++++++++++++--- 4 files changed, 365 insertions(+), 30 deletions(-) create mode 100644 prisma/migrations/0178_v1187_briefing_reroll_date/migration.sql create mode 100644 src/lib/insights/__tests__/comprehensive-briefing-reroll.test.ts diff --git a/prisma/migrations/0178_v1187_briefing_reroll_date/migration.sql b/prisma/migrations/0178_v1187_briefing_reroll_date/migration.sql new file mode 100644 index 000000000..08148180a --- /dev/null +++ b/prisma/migrations/0178_v1187_briefing_reroll_date/migration.sql @@ -0,0 +1,5 @@ +-- v1.18.7 — track the daily-briefing phrasing re-roll date. +-- The content-hash gate keeps the briefing findings byte-stable on +-- unchanged data; this column lets the paragraph be re-rolled once per +-- calendar day (UTC) for phrasing variety without re-paying for findings. +ALTER TABLE "users" ADD COLUMN "insights_briefing_reroll_date" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index c2f620a3c..75745cf83 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -71,6 +71,13 @@ model User { // so a persistently broken provider chain cannot be re-driven into a // full generation attempt by every page-open. insightsWarmFailedAt DateTime? @map("insights_warm_failed_at") + // v1.18.7 — UTC YYYY-MM-DD of the last daily-briefing phrasing re-roll. + // The content-hash gate keeps the FINDINGS byte-stable on unchanged data, + // but the daily-briefing PARAGRAPH is re-rolled once per calendar day at a + // higher, seedless temperature so the prose reads fresh day-over-day + // without re-paying for the findings. This stamps the day a re-roll ran so + // the gate stays at most one extra call per day. + insightsBriefingRerollDate String? @map("insights_briefing_reroll_date") // v1.4.36 W3 T3 — per-user AI Insights exclude-metrics list, mirrors // `coachPrefsJson.excludeMetrics`. The /api/insights/generate route // filters matching keys off `features` before JSON.stringify so the diff --git a/src/lib/insights/__tests__/comprehensive-briefing-reroll.test.ts b/src/lib/insights/__tests__/comprehensive-briefing-reroll.test.ts new file mode 100644 index 000000000..1357a76d2 --- /dev/null +++ b/src/lib/insights/__tests__/comprehensive-briefing-reroll.test.ts @@ -0,0 +1,182 @@ +/** + * v1.18.7 (MEDIUM-3) — daily-briefing phrasing re-roll on unchanged data. + * + * The content-hash gate keeps the FINDINGS byte-stable when the snapshot is + * unchanged, but the daily-briefing PARAGRAPH is re-rolled at most once per + * calendar day at a higher, seedless temperature so the prose reads fresh. + * These tests pin: the re-roll fires when today's date has not been stamped, + * it preserves every cached field except the paragraph, it is seedless + + * higher-temperature, and it does NOT fire a second time the same day. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const findUnique = vi.fn(); +const userUpdate = vi.fn(); +const resolveProviderChain = vi.fn(); +const resolveProvider = vi.fn(); +const runRawCompletionWithFallback = vi.fn(); +const extractFeatures = vi.fn(); +const invalidateUserInsights = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: (...a: unknown[]) => findUnique(...a), + update: (...a: unknown[]) => userUpdate(...a), + }, + auditLog: { deleteMany: vi.fn() }, + }, +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain: (...a: unknown[]) => resolveProviderChain(...a), + resolveProvider: (...a: unknown[]) => resolveProvider(...a), +})); +vi.mock("@/lib/ai/provider-runner", () => ({ + AllProvidersFailedError: class extends Error {}, + runRawCompletionWithFallback: (...a: unknown[]) => + runRawCompletionWithFallback(...a), +})); +vi.mock("@/lib/insights/features", () => ({ + FeaturesPayloadTooLargeError: class extends Error { + sizeBytes = 0; + }, + extractFeatures: (...a: unknown[]) => extractFeatures(...a), +})); +vi.mock("@/lib/insights/glp1-plateau", () => ({ + detectGlp1Plateau: vi.fn(async () => null), + buildGlp1PlateauPrompt: vi.fn(() => ""), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextTextForUser: vi.fn(async () => null), + buildAboutMeInsightBlock: vi.fn(() => ""), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserInsights: (...a: unknown[]) => invalidateUserInsights(...a), +})); + +import { generateComprehensiveInsight } from "../comprehensive-generate"; +import { hashInsightSnapshot } from "../snapshot-hash"; +import { compactSections } from "@/lib/ai/prompts/compact-sections"; + +const FEATURES = { weight: { count: 12, latest: 81.4, mean30: 82.1 } }; +const FEATURES_HASH = hashInsightSnapshot({ + features: compactSections(FEATURES as unknown as Record), + aboutMe: null, + comparisonBaseline: "none", + generationLocale: "de", +}); + +const todayKey = new Date().toISOString().slice(0, 10); + +/** A cached payload carrying a real briefing paragraph + sibling findings. */ +const CACHED = JSON.stringify({ + summary: "stable summary", + dailyBriefing: { + paragraph: "Yesterday's phrasing.", + keyFindings: [{ headline: "BP steady", tone: "good" }], + }, +}); + +beforeEach(() => { + vi.clearAllMocks(); + resolveProviderChain.mockResolvedValue([ + { providerType: "openai", instance: {} }, + ]); + resolveProvider.mockResolvedValue({ type: "none" }); + extractFeatures.mockResolvedValue(FEATURES); + userUpdate.mockResolvedValue({}); +}); + +describe("daily-briefing re-roll on unchanged data", () => { + it("re-rolls only the paragraph, preserves findings, seedless + 0.6 temp", async () => { + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000), + insightsCachedText: CACHED, + insightsExcludeMetrics: [], + insightsSnapshotHash: FEATURES_HASH, + insightsBriefingRerollDate: null, // never re-rolled today + }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { + content: JSON.stringify({ + dailyBriefing: { paragraph: "Today's fresh phrasing." }, + }), + tokensUsed: 50, + providerType: "openai", + model: "m", + }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "rerolled", providerType: "openai" }); + + // Seedless + higher temperature. + const params = runRawCompletionWithFallback.mock.calls[0][0].params; + expect(params.temperature).toBe(0.6); + expect(params.seed).toBeUndefined(); + + // The cache write swaps only the paragraph; findings + summary preserved. + const write = userUpdate.mock.calls.find( + (c) => + (c[0] as { data: Record }).data.insightsCachedText !== + undefined, + ); + expect(write).toBeTruthy(); + const data = (write![0] as { data: Record }).data; + const stored = JSON.parse(data.insightsCachedText as string); + expect(stored.dailyBriefing.paragraph).toBe("Today's fresh phrasing."); + expect(stored.dailyBriefing.keyFindings).toEqual([ + { headline: "BP steady", tone: "good" }, + ]); + expect(stored.summary).toBe("stable summary"); + expect(data.insightsBriefingRerollDate).toBe(todayKey); + expect(invalidateUserInsights).toHaveBeenCalledWith("u1"); + }); + + it("does NOT re-roll a second time the same calendar day", async () => { + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000), + insightsCachedText: CACHED, + insightsExcludeMetrics: [], + insightsSnapshotHash: FEATURES_HASH, + insightsBriefingRerollDate: todayKey, // already re-rolled today + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "unchanged" }); + expect(runRawCompletionWithFallback).not.toHaveBeenCalled(); + }); + + it("falls back to a plain refresh (still stamps the day) when the re-roll JSON is unusable", async () => { + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000), + insightsCachedText: CACHED, + insightsExcludeMetrics: [], + insightsSnapshotHash: FEATURES_HASH, + insightsBriefingRerollDate: null, + }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: "not json at all", tokensUsed: 5, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "unchanged" }); + // The day is stamped so a broken provider can't be re-driven all day. + const write = userUpdate.mock.calls.find( + (c) => + (c[0] as { data: Record }).data + .insightsBriefingRerollDate !== undefined, + ); + expect(write).toBeTruthy(); + }); +}); diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index fab7310de..97c1b2ea9 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -133,9 +133,107 @@ export type GenerateOutcome = * (GET read, nightly discovery) treat the cache as current. */ | { status: "unchanged" } + /** + * v1.18.7 — the snapshot was unchanged (findings byte-stable) but the + * daily-briefing PARAGRAPH was re-rolled at a higher, seedless + * temperature for phrasing variety. At most one re-roll per calendar day. + */ + | { status: "rerolled"; providerType: string } | { status: "skipped"; reason: "no-provider" | "no-consent" } | { status: "failed"; reason: string }; +/** Higher, seedless temperature for the daily-briefing phrasing re-roll. */ +const BRIEFING_REROLL_TEMPERATURE = 0.6; + +/** UTC YYYY-MM-DD calendar-day key gating the once-per-day briefing re-roll. */ +function buildRerollDateKey(at: Date = new Date()): string { + return at.toISOString().slice(0, 10); +} + +/** + * Re-roll ONLY the daily-briefing paragraph on a hash-unchanged hit. + * + * Runs the same comprehensive prompt at a higher, seedless temperature, then + * keeps the cached payload verbatim and swaps in just the freshly-generated + * `dailyBriefing.paragraph`. So the structured findings stay byte-stable + * (reference assessments, monotony is fine) while the prose the user reads at + * the top of /insights varies day-over-day. Returns null on any miss (no + * provider success, invalid JSON, no cached/new paragraph) so the caller + * falls back to the plain timestamp refresh. + */ +async function rerollBriefingParagraph(args: { + userId: string; + chain: Awaited>; + systemPrompt: string; + userPrompt: string; + cachedText: string; + locale: "de" | "en"; +}): Promise<{ text: string; providerType: string } | null> { + let cached: Record; + try { + cached = JSON.parse(args.cachedText) as Record; + } catch { + return null; + } + const cachedBriefing = cached.dailyBriefing; + // Nothing to re-roll when the cached payload carries no briefing paragraph. + if ( + cachedBriefing === null || + typeof cachedBriefing !== "object" || + typeof (cachedBriefing as { paragraph?: unknown }).paragraph !== "string" + ) { + return null; + } + + let result; + let providerType: string; + try { + const fallback = await runRawCompletionWithFallback({ + userId: args.userId, + providers: args.chain, + params: { + systemPrompt: args.systemPrompt, + userPrompt: args.userPrompt, + // Seedless on purpose: the seed would pin the same phrasing, which is + // exactly what we are varying. Higher temperature for prose variety. + temperature: BRIEFING_REROLL_TEMPERATURE, + maxTokens: AI_BUDGETS.comprehensive.maxTokens, + responseFormat: "json", + }, + }); + result = fallback.result; + providerType = fallback.workingProvider.providerType; + } catch { + return null; + } + + let parsed: Record; + try { + parsed = JSON.parse(stripJsonFences(result.content)) as Record< + string, + unknown + >; + } catch { + return null; + } + const freshBriefing = parsed.dailyBriefing; + const freshParagraph = + freshBriefing && typeof freshBriefing === "object" + ? (freshBriefing as { paragraph?: unknown }).paragraph + : undefined; + if (typeof freshParagraph !== "string" || freshParagraph.trim().length === 0) { + return null; + } + + // Keep the cached payload verbatim; swap in only the fresh paragraph. + const mergedBriefing = { + ...(cachedBriefing as Record), + paragraph: freshParagraph, + }; + const merged = { ...cached, dailyBriefing: mergedBriefing }; + return { text: JSON.stringify(merged), providerType }; +} + /** * Comparison-snapshot builder — shared with the on-demand route. Returns * null when the user's comparison toggle is off (most users), so the @@ -532,6 +630,7 @@ export async function generateComprehensiveInsight( insightsCachedText: true, insightsExcludeMetrics: true, insightsSnapshotHash: true, + insightsBriefingRerollDate: true, // The comparison-baseline setting joins the snapshot fingerprint // (see the hash construction below), so it must be readable BEFORE // the gate — the comparison snapshot itself is only built after. @@ -643,20 +742,15 @@ export async function generateComprehensiveInsight( comparisonBaseline, generationLocale: locale, }); - if ( - dbUser?.insightsCachedText && - dbUser.insightsSnapshotHash === snapshotHash - ) { - await prisma.user.update({ - where: { id: userId }, - data: { insightsCachedAt: new Date() }, - }); - annotate({ - action: { name: "insights.generate.skipped_unchanged" }, - meta: { locale }, - }); - return { status: "unchanged" }; - } + // v1.12.7 (B5) — the curated SOURCES block for the metrics present, shared + // by both the full generation below and the unchanged-data re-roll. + const referenceMetrics = metricsFromPresentSections({ + bloodPressure: features.bloodPressure != null, + weight: features.weight != null, + pulse: features.pulse != null, + mood: features.mood != null, + medication: (features.medications?.length ?? 0) > 0, + }); const comparisonSnapshot = await buildComparisonSnapshotForUser(userId); const plateauContext = await detectGlp1Plateau(userId); @@ -669,25 +763,72 @@ export async function generateComprehensiveInsight( if (plateauContext) { userPrompt += buildGlp1PlateauPrompt(plateauContext, locale); } - // v1.15.20 — fold the user-authored "about me" self-description - // (Settings → AI) into the nightly briefing exactly like the - // on-demand route. Null (no text / undecryptable) costs nothing. - // (Fetched above the hash gate — it is part of the fingerprint.) if (aboutMe) { userPrompt += buildAboutMeInsightBlock(aboutMe, locale); } + const systemPrompt = buildSystemPromptWithReferences(locale, referenceMetrics); - // v1.12.7 (B5) — inject the curated SOURCES block for the metric sections - // this generation actually carries, so a normative claim ("target < 140/90") - // can cite a real `referenceId` the schema + UI footnote already support. - // Returns the plain prompt unchanged when no applicable metric is present. - const referenceMetrics = metricsFromPresentSections({ - bloodPressure: features.bloodPressure != null, - weight: features.weight != null, - pulse: features.pulse != null, - mood: features.mood != null, - medication: (features.medications?.length ?? 0) > 0, - }); + if ( + dbUser?.insightsCachedText && + dbUser.insightsSnapshotHash === snapshotHash + ) { + // v1.18.7 (MEDIUM-3) — findings are byte-stable, but re-roll the daily- + // briefing PARAGRAPH once per calendar day so the prose reads fresh + // day-over-day. Gated on `insightsBriefingRerollDate` so it costs at + // most one extra call/day; the higher, seedless temperature varies the + // phrasing while the structured findings are preserved verbatim from the + // cached payload. Any miss falls through to the plain timestamp refresh. + const todayKey = buildRerollDateKey(); + if (dbUser.insightsBriefingRerollDate !== todayKey) { + const rerolled = await rerollBriefingParagraph({ + userId, + chain, + systemPrompt, + userPrompt, + cachedText: dbUser.insightsCachedText, + locale, + }); + if (rerolled) { + await prisma.user.update({ + where: { id: userId }, + data: { + insightsCachedAt: new Date(), + insightsCachedText: rerolled.text, + insightsBriefingRerollDate: todayKey, + }, + }); + invalidateUserInsights(userId); + annotate({ + action: { name: "insights.generate.briefing_rerolled" }, + meta: { locale, provider: rerolled.providerType }, + }); + return { status: "rerolled", providerType: rerolled.providerType }; + } + // A re-roll miss still stamps the day so a persistently failing + // provider cannot be re-driven on every page-open; the next day retries. + await prisma.user.update({ + where: { id: userId }, + data: { + insightsCachedAt: new Date(), + insightsBriefingRerollDate: todayKey, + }, + }); + annotate({ + action: { name: "insights.generate.skipped_unchanged" }, + meta: { locale, rerollAttempted: true }, + }); + return { status: "unchanged" }; + } + await prisma.user.update({ + where: { id: userId }, + data: { insightsCachedAt: new Date() }, + }); + annotate({ + action: { name: "insights.generate.skipped_unchanged" }, + meta: { locale }, + }); + return { status: "unchanged" }; + } let result; let workingProviderType: string; @@ -696,7 +837,7 @@ export async function generateComprehensiveInsight( userId, providers: chain, params: { - systemPrompt: buildSystemPromptWithReferences(locale, referenceMetrics), + systemPrompt, userPrompt, temperature: AI_BUDGETS.comprehensive.temperature, maxTokens: AI_BUDGETS.comprehensive.maxTokens, From 57cf8340e27f82507555df6bf62dd32573acb2e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:06:53 +0200 Subject: [PATCH 5/9] fix(insights): retry the comprehensive JSON parse once before failing The comprehensive generator did an ad-hoc JSON.parse with no corrective retry and failed cold to `invalid-json` on a first-pass miss, while the strict insight wrapper's `buildRetryCorrectionMessage` sat right next door. Export that helper and reuse it: on a parse miss, append the correction to the user prompt and re-run the chain once before declaring failure. Unifies the JSON-robustness story across the two paths. --- src/lib/ai/generate-insight.ts | 5 +- .../comprehensive-json-retry.test.ts | 128 ++++++++++++++++++ src/lib/insights/comprehensive-generate.ts | 72 ++++++++-- 3 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 src/lib/insights/__tests__/comprehensive-json-retry.test.ts diff --git a/src/lib/ai/generate-insight.ts b/src/lib/ai/generate-insight.ts index 3cb1444f1..4b8fb939c 100644 --- a/src/lib/ai/generate-insight.ts +++ b/src/lib/ai/generate-insight.ts @@ -46,7 +46,10 @@ export interface GenerateInsightOutcome { * what went wrong on the first attempt. Includes the violated zod * issues verbatim (truncated to 1KB) so the model can self-correct. */ -function buildRetryCorrectionMessage(reason: string, details: string): string { +export function buildRetryCorrectionMessage( + reason: string, + details: string, +): string { return ` Your previous response did not satisfy the required JSON schema. Reason: ${reason} diff --git a/src/lib/insights/__tests__/comprehensive-json-retry.test.ts b/src/lib/insights/__tests__/comprehensive-json-retry.test.ts new file mode 100644 index 000000000..047525350 --- /dev/null +++ b/src/lib/insights/__tests__/comprehensive-json-retry.test.ts @@ -0,0 +1,128 @@ +/** + * v1.18.7 (MEDIUM-5) — comprehensive JSON-retry robustness. + * + * The comprehensive path used to fail cold to `invalid-json` on a first-pass + * parse miss. It now reuses `buildRetryCorrectionMessage` for ONE corrective + * retry before declaring failure. These tests pin: a first-pass miss followed + * by a valid retry succeeds; two misses fail; and a first-pass success runs + * no retry. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const findUnique = vi.fn(); +const userUpdate = vi.fn(); +const resolveProviderChain = vi.fn(); +const resolveProvider = vi.fn(); +const runRawCompletionWithFallback = vi.fn(); +const extractFeatures = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: (...a: unknown[]) => findUnique(...a), + update: (...a: unknown[]) => userUpdate(...a), + }, + auditLog: { deleteMany: vi.fn() }, + }, +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain: (...a: unknown[]) => resolveProviderChain(...a), + resolveProvider: (...a: unknown[]) => resolveProvider(...a), +})); +vi.mock("@/lib/ai/provider-runner", () => ({ + AllProvidersFailedError: class extends Error {}, + runRawCompletionWithFallback: (...a: unknown[]) => + runRawCompletionWithFallback(...a), +})); +vi.mock("@/lib/insights/features", () => ({ + FeaturesPayloadTooLargeError: class extends Error { + sizeBytes = 0; + }, + extractFeatures: (...a: unknown[]) => extractFeatures(...a), +})); +vi.mock("@/lib/insights/glp1-plateau", () => ({ + detectGlp1Plateau: vi.fn(async () => null), + buildGlp1PlateauPrompt: vi.fn(() => ""), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextTextForUser: vi.fn(async () => null), + buildAboutMeInsightBlock: vi.fn(() => ""), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserInsights: vi.fn(), +})); + +import { generateComprehensiveInsight } from "../comprehensive-generate"; + +const FEATURES = { weight: { count: 12, latest: 81.4, mean30: 82.1 } }; + +beforeEach(() => { + vi.clearAllMocks(); + resolveProviderChain.mockResolvedValue([ + { providerType: "openai", instance: {} }, + ]); + resolveProvider.mockResolvedValue({ type: "none" }); + extractFeatures.mockResolvedValue(FEATURES); + userUpdate.mockResolvedValue({}); + // No cached text / hash → always runs a full generation (no gate hit). + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: null, + insightsCachedText: null, + insightsExcludeMetrics: [], + insightsSnapshotHash: null, + insightsBriefingRerollDate: null, + }); +}); + +const VALID = JSON.stringify({ dailyBriefing: { paragraph: "ok" } }); + +describe("comprehensive JSON-retry", () => { + it("recovers via one corrective retry after a first-pass JSON miss", async () => { + runRawCompletionWithFallback + .mockResolvedValueOnce({ + result: { content: "I'm sorry, here is the data:", tokensUsed: 5, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }) + .mockResolvedValueOnce({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "generated", providerType: "openai" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(2); + // The retry call appends the correction to the user prompt. + const retryParams = runRawCompletionWithFallback.mock.calls[1][0].params; + expect(retryParams.userPrompt).toContain("did not satisfy the required"); + }); + + it("fails with invalid-json when both attempts miss", async () => { + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: "still not json", tokensUsed: 5, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "failed", reason: "invalid-json" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(2); + }); + + it("runs no retry when the first pass is valid", async () => { + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "generated", providerType: "openai" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 97c1b2ea9..7f263103f 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -34,6 +34,7 @@ import { type ComparisonSnapshot, } from "@/lib/ai/prompts/insight-system-prompt"; import { buildSystemPromptWithReferences } from "@/lib/ai/prompts/insight-generator"; +import { buildRetryCorrectionMessage } from "@/lib/ai/generate-insight"; import { buildAboutMeInsightBlock, getSelfContextTextForUser, @@ -234,6 +235,28 @@ async function rerollBriefingParagraph(args: { return { text: JSON.stringify(merged), providerType }; } +/** + * Parse + (best-effort) validate a comprehensive provider reply. Returns the + * parsed payload (validated when it matches `insightResultSchema`, else the + * raw object so the richer optional blocks survive), or null when the reply + * is not valid JSON even after fence-stripping. Null signals the caller to + * run its one corrective retry. + */ +function parseComprehensiveResult( + content: string, +): InsightResult | Record | null { + let parsed: unknown; + try { + parsed = JSON.parse(stripJsonFences(content)); + } catch { + return null; + } + const validated = insightResultSchema.safeParse(parsed); + return validated.success + ? validated.data + : (parsed as Record); +} + /** * Comparison-snapshot builder — shared with the on-demand route. Returns * null when the user's comparison toggle is off (most users), so the @@ -856,16 +879,45 @@ export async function generateComprehensiveInsight( return { status: "failed", reason: "provider-error" }; } - let insights: InsightResult | Record; - try { - // Anthropic + local have no native JSON mode, so a ```json-fenced or - // sentence-prefixed reply would otherwise fail the whole generation. - // Strip the fence before parsing; clean JSON passes through unchanged. - const parsed = JSON.parse(stripJsonFences(result.content)); - const validated = insightResultSchema.safeParse(parsed); - insights = validated.success ? validated.data : parsed; - } catch { - return { status: "failed", reason: "invalid-json" }; + // Anthropic + local have no native JSON mode, so a ```json-fenced or + // sentence-prefixed reply would otherwise fail the whole generation. + // Strip the fence before parsing; clean JSON passes through unchanged. + let insights = parseComprehensiveResult(result.content); + if (insights === null) { + // v1.18.7 (MEDIUM-5) — one corrective retry before declaring failure, + // reusing the same `buildRetryCorrectionMessage` the strict insight + // wrapper uses next door. The comprehensive path previously failed cold + // to `invalid-json` on a first-pass miss even though the recovery helper + // sat right there. The retry appends the correction to the user prompt + // and re-runs the chain once. + annotate({ + action: { name: "insights.generate.json_retry" }, + meta: { locale }, + }); + try { + const retry = await runRawCompletionWithFallback({ + userId, + providers: chain, + params: { + systemPrompt, + userPrompt: `${userPrompt}\n\n${buildRetryCorrectionMessage( + "Response was not valid JSON", + "The previous reply could not be parsed as a JSON object.", + )}`, + temperature: AI_BUDGETS.comprehensive.temperature, + maxTokens: AI_BUDGETS.comprehensive.maxTokens, + responseFormat: "json", + }, + }); + result = retry.result; + workingProviderType = retry.workingProvider.providerType; + } catch { + return { status: "failed", reason: "invalid-json" }; + } + insights = parseComprehensiveResult(result.content); + if (insights === null) { + return { status: "failed", reason: "invalid-json" }; + } } // v1.9.0 — the caller abandoned this generation (its bounded timeout From 57242b9884f49a26a2d19d2bec8aeff396d9ee28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:11:28 +0200 Subject: [PATCH 6/9] refactor(ai): extract shared prompt contracts as a single source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grounding, tone/banned-opener, GLP-1 dose-safety, metric-identifier ban and forbidden-filler rules were independently encoded across the comprehensive, status, Coach and narrative prompts and had drifted. Move the canonical wording into `prompts/shared-contracts.ts` (locale-keyed fragments) and compose each surface from it. Where the surfaces had drifted, adopt the stricter wording: most notably the GLP-1 dose-prescription contract — previously present only in the comprehensive and Coach prompts — now reaches the status cards and the period narrative too. A cross-surface coverage test asserts every enforced fragment appears verbatim in every surface, in de and en. --- src/lib/ai/coach/system-prompt.ts | 17 ++- .../assessment-prompt-contract.test.ts | 5 +- .../shared-contracts-coverage.test.ts | 103 ++++++++++++++++ src/lib/ai/prompts/base-system.ts | 28 ++++- src/lib/ai/prompts/insight-generator.ts | 24 ++++ src/lib/ai/prompts/shared-contracts.ts | 115 ++++++++++++++++++ .../narrative/period-narrative-generate.ts | 19 ++- 7 files changed, 302 insertions(+), 9 deletions(-) create mode 100644 src/lib/ai/prompts/__tests__/shared-contracts-coverage.test.ts create mode 100644 src/lib/ai/prompts/shared-contracts.ts diff --git a/src/lib/ai/coach/system-prompt.ts b/src/lib/ai/coach/system-prompt.ts index 826b7af66..35aad7aad 100644 --- a/src/lib/ai/coach/system-prompt.ts +++ b/src/lib/ai/coach/system-prompt.ts @@ -29,6 +29,7 @@ import { SELF_REPORT_FENCE_END, fenceSelfReport, } from "@/lib/ai/coach/self-report-fence"; +import { composeSharedContracts } from "@/lib/ai/prompts/shared-contracts"; const COACH_PROMPT_EN = `You are the HealthLog Coach — the user's warm, motivating advisor. You sit alongside them as they look at their own health data — blood pressure, @@ -863,8 +864,22 @@ export function getCoachSystemPrompt( base = COACH_PROMPT_EN + LOCALE_REPLY_FOOTER_FALLBACK[locale]; } } + // v1.18.7 (HIGH-2) — append the shared cross-surface contracts (canonical + // wording, single source of truth in `shared-contracts.ts`) for the + // hand-composed de/en bodies. The native FR/ES/IT/PL bodies carry their own + // per-locale safety wording from the contract matrix, so they are left as-is. + const withContracts = + locale === "de" || locale === "en" + ? `${base}\n\nSHARED CONTRACTS\n\n${composeSharedContracts(locale, [ + "grounding", + "toneContract", + "safetyGlp1", + "metricIdentifierBan", + "forbiddenFiller", + ])}` + : base; const prefix = buildPrefsPrefix(locale, prefs); - const withPrefix = prefix ? `${prefix}\n\n${base}` : base; + const withPrefix = prefix ? `${prefix}\n\n${withContracts}` : withContracts; // v1.18.7 — article-awareness: a compact catalog of the public /learn // guides so the Coach can point the user at the relevant deeper read. The // block is locale-independent (English topic labels + URLs) and the model diff --git a/src/lib/ai/prompts/__tests__/assessment-prompt-contract.test.ts b/src/lib/ai/prompts/__tests__/assessment-prompt-contract.test.ts index 8f9178570..780982930 100644 --- a/src/lib/ai/prompts/__tests__/assessment-prompt-contract.test.ts +++ b/src/lib/ai/prompts/__tests__/assessment-prompt-contract.test.ts @@ -34,8 +34,11 @@ describe("base-system assessment prompt — grounding floor preserved", () => { it(`${locale}: keeps the filler-phrase ban`, () => { const p = getBaseSystemPrompt(locale); + // v1.18.7 — the filler ban is now sourced from the single shared + // contract fragment ("FORBIDDEN FILLER"); the banned-phrase list is + // preserved verbatim. if (locale === "en") { - expect(p).toMatch(/FORBIDDEN PHRASES/); + expect(p).toMatch(/FORBIDDEN FILLER/); expect(p).toContain("make sure to get enough sleep"); expect(p).toContain("consult your doctor"); } else { diff --git a/src/lib/ai/prompts/__tests__/shared-contracts-coverage.test.ts b/src/lib/ai/prompts/__tests__/shared-contracts-coverage.test.ts new file mode 100644 index 000000000..e2c790a07 --- /dev/null +++ b/src/lib/ai/prompts/__tests__/shared-contracts-coverage.test.ts @@ -0,0 +1,103 @@ +/** + * v1.18.7 (HIGH-2) — cross-surface shared-contract coverage. + * + * The grounding / tone / GLP-1-safety / metric-identifier-ban / forbidden- + * filler contracts live once in `shared-contracts.ts`. These guards assert + * that each surface that enforces a contract carries the canonical fragment + * VERBATIM, in both hand-composed locales (de/en). A future edit that lets a + * surface drift from the shared wording — the exact failure mode HIGH-2 + * documents — trips here instead of shipping silently. + */ +import { describe, expect, it } from "vitest"; + +import { + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, + type ContractLocale, +} from "../shared-contracts"; +import { getBaseSystemPrompt } from "../base-system"; +import { getStrictInsightsSystemPrompt } from "../insight-generator"; +import { getCoachSystemPrompt } from "@/lib/ai/coach/system-prompt"; + +// The narrative system prompts are module-private; re-derive the same +// composed shared block the generator appends, and assert each fragment is +// present by checking the generator module's exported builder indirectly. +import { SYSTEM_PROMPTS_FOR_TEST } from "@/lib/insights/narrative/period-narrative-generate"; + +const LOCALES: ContractLocale[] = ["de", "en"]; + +/** Which contracts each surface enforces (and must carry verbatim). */ +const SURFACES: Record< + string, + { + prompt: (l: ContractLocale) => string; + contracts: Record[]; + } +> = { + "insight-generator (comprehensive)": { + prompt: (l) => getStrictInsightsSystemPrompt(l), + contracts: [ + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, + ], + }, + "base-system (status cards)": { + prompt: (l) => getBaseSystemPrompt(l), + contracts: [ + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, + ], + }, + coach: { + prompt: (l) => getCoachSystemPrompt(l), + contracts: [ + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, + ], + }, + // The narrative is descriptive-only, so it enforces every contract EXCEPT + // the motivating tone contract. + "period-narrative": { + prompt: (l) => SYSTEM_PROMPTS_FOR_TEST[l], + contracts: [grounding, safetyGlp1, metricIdentifierBan, forbiddenFiller], + }, +}; + +describe("shared-contract cross-surface coverage", () => { + for (const [surface, { prompt, contracts }] of Object.entries(SURFACES)) { + for (const locale of LOCALES) { + const text = prompt(locale); + for (const contract of contracts) { + const fragment = contract[locale]; + // Use a short, stable slice of the fragment as the assertion anchor — + // the full fragment is long but must be present verbatim. + it(`${surface} carries every enforced contract verbatim (${locale})`, () => { + expect(text).toContain(fragment); + }); + } + } + } + + it("the GLP-1 dose-safety contract reaches all four surfaces", () => { + for (const locale of LOCALES) { + expect(getStrictInsightsSystemPrompt(locale)).toContain( + safetyGlp1[locale], + ); + expect(getBaseSystemPrompt(locale)).toContain(safetyGlp1[locale]); + expect(getCoachSystemPrompt(locale)).toContain(safetyGlp1[locale]); + expect(SYSTEM_PROMPTS_FOR_TEST[locale]).toContain(safetyGlp1[locale]); + } + }); +}); diff --git a/src/lib/ai/prompts/base-system.ts b/src/lib/ai/prompts/base-system.ts index 63ff478cf..08aeaf6f5 100644 --- a/src/lib/ai/prompts/base-system.ts +++ b/src/lib/ai/prompts/base-system.ts @@ -1,4 +1,11 @@ import type { Locale } from "@/lib/i18n/config"; +import { + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, +} from "./shared-contracts"; /** * Version of the per-metric assessment base prompt + its signal contract. @@ -151,13 +158,24 @@ const ASSESSMENT_SECTIONS: readonly AssessmentSection[] = [ - Jüngste Messung deutlich älter als ~7 Tage (dataCoverage.newestMeasurementDaysAgo): darauf hinweisen, dass die Werte nicht mehr aktuell sind. - Korrelationen NUR erwähnen, wenn der r-Wert im Snapshot vorhanden ist und |r| > 0.4 ist. Fehlt das Feld, keine Korrelation interpretieren oder erfinden. Immer als "Zusammenhang" formulieren, nie als "Ursache".`, }, + // v1.18.7 (HIGH-2) — sourced from the single shared-contract fragment so a + // forbidden-filler edit lands on every surface at once. + { id: "forbidden", en: forbiddenFiller.en, de: forbiddenFiller.de }, + // v1.18.7 (HIGH-2) — the shared metric-identifier ban + the GLP-1 dose + // safety contract. The status cards can surface a named medication via the + // medication-compliance snapshot, so the dose-safety contract (previously + // absent from this surface) now applies here too. { - id: "forbidden", - en: `FORBIDDEN PHRASES (they signal ungrounded filler — never emit, except in the disclaimer): -"make sure to get enough sleep", "drink enough water", "regular exercise", "consult your doctor".`, - de: `VERBOTENE FLOSKELN (signalisieren ungegroundeten Fülltext — nie ausgeben, außer im disclaimer): -"achte auf ausreichend Schlaf", "trinke genug Wasser", "regelmäßige Bewegung", "ärztlicher Rat empfohlen".`, + id: "metricIdentifierBan", + en: metricIdentifierBan.en, + de: metricIdentifierBan.de, }, + { id: "safetyGlp1", en: safetyGlp1.en, de: safetyGlp1.de }, + // v1.18.7 (HIGH-2) — the shared grounding + tone contracts, canonical + // wording. The metric-specific build/tone sections above stay; these pin + // the cross-surface wording so an edit lands here and everywhere at once. + { id: "sharedGrounding", en: grounding.en, de: grounding.de }, + { id: "sharedTone", en: toneContract.en, de: toneContract.de }, { id: "examples", en: `EXAMPLES (they illustrate form and grounding — do not copy them verbatim; every assessment uses the real snapshot numbers): diff --git a/src/lib/ai/prompts/insight-generator.ts b/src/lib/ai/prompts/insight-generator.ts index 0aa9ae197..5d6a4e315 100644 --- a/src/lib/ai/prompts/insight-generator.ts +++ b/src/lib/ai/prompts/insight-generator.ts @@ -29,6 +29,13 @@ import { buildNativeInsightsPrompt, buildOutOfScopeRefusal, } from "./native-prompts"; +import { + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, +} from "./shared-contracts"; /** Stable identifier for the active system prompt revision. */ export const PROMPT_VERSION = "4.29.0" as const; @@ -745,6 +752,23 @@ user-logged event with a neutral label + detail. Hard cap 20 entries.`, Eintrag verweist auf ein reales, vom Nutzer geloggtes Ereignis mit neutralem Label + Detail. Höchstgrenze: 20 Einträge.`, }, + // v1.18.7 (HIGH-2) — the shared cross-surface contracts as the canonical + // source. The detailed rules above stay for this calibrated surface; these + // restate the five contracts in the wording every surface now shares, so a + // future safety edit is made once in `shared-contracts.ts`. + { id: "sharedGrounding", en: grounding.en, de: grounding.de }, + { id: "sharedTone", en: toneContract.en, de: toneContract.de }, + { id: "sharedSafetyGlp1", en: safetyGlp1.en, de: safetyGlp1.de }, + { + id: "sharedMetricIdentifierBan", + en: metricIdentifierBan.en, + de: metricIdentifierBan.de, + }, + { + id: "sharedForbiddenFiller", + en: forbiddenFiller.en, + de: forbiddenFiller.de, + }, { id: "language", en: `LANGUAGE diff --git a/src/lib/ai/prompts/shared-contracts.ts b/src/lib/ai/prompts/shared-contracts.ts new file mode 100644 index 000000000..72aa3fdfa --- /dev/null +++ b/src/lib/ai/prompts/shared-contracts.ts @@ -0,0 +1,115 @@ +/** + * Single source of truth for the cross-surface AI prompt contracts. + * + * Before this module the grounding, tone/banned-opener, GLP-1 dose-safety, + * metric-identifier ban and forbidden-filler rules were independently encoded + * in `insight-generator.ts` (comprehensive briefing), `base-system.ts` (status + * cards), `coach/system-prompt.ts` (Coach) and `period-narrative-generate.ts` + * (narrative) — and had DRIFTED (different banned-opener lists, a GLP-1 + * contract present only on two of the four surfaces, …). A safety-rule edit + * had to be made in up to four places and was easy to miss in one. + * + * These named, locale-keyed fragments are now the one place each contract + * lives. Each surface composes the fragment it enforces. Where the surfaces + * had drifted, the STRICTER / more-complete wording was adopted here (most + * notably the GLP-1 dose-prescription contract, which previously lived only + * in the comprehensive + Coach prompts and is now enforced on every surface + * that can ever name a medication). + * + * Only `de` / `en` are modelled here — they are the two hand-composed + * surfaces. The FR/ES/IT/PL bodies are assembled from the safety-contract + * matrix in `native-prompts.ts`, which already carries its own per-locale + * safety wording. + * + * The cross-surface coverage test (`shared-contracts-coverage.test.ts`) + * asserts each fragment appears verbatim in every surface that enforces it. + */ + +export type ContractLocale = "de" | "en"; + +/** A named contract fragment with both hand-composed locale texts. */ +export type SharedContract = Record; + +/** + * Zero-hallucination grounding: every claim traces to a number in the + * snapshot, and the comparison is the user's OWN baseline, never a population + * norm. + */ +export const grounding: SharedContract = { + en: `GROUNDING — ZERO HALLUCINATIONS +Every claim must trace to a number visible in the snapshot you were given. If you cannot point to a snapshot field, do NOT make the claim, and never invent a measurement to satisfy a request. Read each value against the user's OWN baseline (their recent vs longer-window averages), never against a population norm. State a correlation only when its r-value is present and |r| > 0.4, always as an "association", never a "cause".`, + de: `ERDUNG — NULL HALLUZINATIONEN +Jede Aussage muss auf einer Zahl beruhen, die im übergebenen Snapshot sichtbar ist. Lässt sich die Aussage keinem Snapshot-Feld zuordnen, lass sie weg und erfinde nie einen Messwert, um einer Anfrage zu entsprechen. Ordne jeden Wert gegen die EIGENE Baseline des Nutzers ein (jüngeres vs. längeres Mittel), nie gegen eine Bevölkerungsnorm. Nenne einen Zusammenhang nur, wenn der r-Wert vorhanden und |r| > 0,4 ist — immer als "Zusammenhang", nie als "Ursache".`, +}; + +/** + * Tone contract + the single banned-opener list. Warm, motivating, + * autonomy-supporting; encouragement EARNED by the numbers; no generic + * data-quality compliment opener. + */ +export const toneContract: SharedContract = { + en: `TONE — a warm, motivating advisor, never clinical or alarming +Write in the second person, warm and direct. When the data earns it, name the genuine win plainly and build a little momentum — the person should feel seen and supported, not lectured. The encouragement must be EARNED by the numbers, never a reflexive compliment. Be autonomy-supporting ("worth a try", "can help", never "you must"). Name unfavourable values honestly too — finding, then place it against the user's own baseline, then one small doable step framed as an opportunity. Never alarm, never moralise, never diagnose. No platitudes and no bare number-echoing. Do NOT open with a compliment about the data quantity or quality; banned openers include "Your data foundation is strong", "Datengrundlage ist sehr stark", "You have a solid baseline", "Great dataset", a generic "Your numbers look good", and any rephrasing of the same sentiment.`, + de: `TONALITÄT — ein warmer, motivierender Begleiter, nie klinisch oder alarmierend +Schreibe in der zweiten Person, warm und direkt. Wenn die Daten es hergeben, benenne den echten Erfolg klar und baue ein wenig Schwung auf — die Person soll sich gesehen und unterstützt fühlen, nicht belehrt. Die Ermutigung muss durch die Zahlen VERDIENT sein, nie ein reflexhaftes Kompliment. Sei autonomie-unterstützend ("einen Versuch wert", "kann helfen", nie "du musst"). Benenne auch ungünstige Werte ehrlich — Befund, dann gegen die eigene Baseline einordnen, dann ein kleiner machbarer Schritt, als Chance formuliert. Nie alarmierend, nie moralisierend, nie diagnostisch. Keine Floskeln und keine bloße Zahlenwiederholung. Beginne NICHT mit einem Kompliment über Datenmenge oder -qualität; verbotene Eröffnungen sind u.a. "Datengrundlage ist sehr stark", "Your data foundation is strong", "Du hast eine solide Baseline", "Großartiger Datensatz", ein generisches "Deine Werte sehen gut aus" und jede sinngemäße Umformulierung.`, +}; + +/** + * GLP-1 dose-prescription safety contract. Previously present ONLY in the + * comprehensive + Coach prompts; now enforced on every surface that can name + * a medication (the stricter, more-complete posture). + */ +export const safetyGlp1: SharedContract = { + en: `GLP-1 DOSE SAFETY (contract, not style) +NEVER prescribe, recommend, or modify medication doses, even when the snapshot names a GLP-1 receptor agonist (Mounjaro, Ozempic, Wegovy, Zepbound, Trulicity, Saxenda, Rybelsus). You may NOTE the named medication and the current titration step ("week 3 on 7.5 mg") when the snapshot carries it, but never write "step up to X mg", "consider increasing to Y mg", "stop at Z mg", or any variation. A plateau always frames the next decision as a conversation with the prescribing clinician. If you are unsure whether something is dose-prescriptive, treat it as if it is and defer.`, + de: `GLP-1-DOSIS-SICHERHEIT (Vertrag, kein Stil) +Verschreibe, empfiehl oder ändere NIEMALS Medikamenten-Dosen, auch wenn der Snapshot einen GLP-1-Rezeptoragonisten benennt (Mounjaro, Ozempic, Wegovy, Zepbound, Trulicity, Saxenda, Rybelsus). Du darfst den benannten Wirkstoff und die aktuelle Titrationsstufe NENNEN ("Woche 3 auf 7,5 mg"), wenn der Snapshot sie trägt, aber schreibe nie "erhöhe auf X mg", "erwäge die nächste Stufe Y mg", "bleibe auf Z mg" oder eine Variante davon. Ein Plateau rahmt die nächste Entscheidung immer als Gespräch mit der behandelnden Ärztin. Bist du unsicher, ob etwas dosis-präskriptiv ist, behandle es so — und verweise an die Klinik.`, +}; + +/** + * Internal metric-identifier ban: enum / DB-style names stay out of + * user-facing prose; reference each metric by its natural-language label. + */ +export const metricIdentifierBan: SharedContract = { + en: `METRIC IDENTIFIERS STAY OUT OF PROSE +Never write database / enum-style names like "BLOOD_PRESSURE_SYS", "PULSE_BPM", "MOOD_SCORE", "MEDICATION_COMPLIANCE_PCT", "HEART_RATE_VARIABILITY", "RESTING_HEART_RATE", "ACTIVE_ENERGY_BURNED", "FLIGHTS_CLIMBED", "WALKING_RUNNING_DISTANCE", "VO2_MAX", "BODY_TEMPERATURE" or "SLEEP_DURATION" in any user-facing string, and never write the literal "metric:" chart-token in prose. Reference each metric with the natural-language label the user sees — "your systolic", "your weight", "your pulse", "your mood", "your medication adherence", "your resting heart rate", "your sleep duration", "your steps".`, + de: `METRIK-IDENTIFIER GEHÖREN NICHT IN DEN FLIESSTEXT +Schreibe niemals Datenbank- bzw. Enum-Namen wie "BLOOD_PRESSURE_SYS", "PULSE_BPM", "MOOD_SCORE", "MEDICATION_COMPLIANCE_PCT", "HEART_RATE_VARIABILITY", "RESTING_HEART_RATE", "ACTIVE_ENERGY_BURNED", "FLIGHTS_CLIMBED", "WALKING_RUNNING_DISTANCE", "VO2_MAX", "BODY_TEMPERATURE" oder "SLEEP_DURATION" in nutzersichtbare Strings, und schreibe nie das wörtliche "metric:"-Chart-Token in den Fließtext. Verweise auf jede Metrik mit der natürlichsprachlichen Bezeichnung, die der Nutzer sieht — "deine Systole", "dein Gewicht", "dein Puls", "deine Stimmung", "deine Medikamentenadhärenz", "dein Ruhepuls", "deine Schlafdauer", "deine Schritte".`, +}; + +/** + * Forbidden-filler list: the ungrounded platitudes that signal filler. + */ +export const forbiddenFiller: SharedContract = { + en: `FORBIDDEN FILLER (signals ungrounded platitudes — never emit, except in a disclaimer) +"make sure to get enough sleep", "drink enough water", "regular exercise", "consult your doctor".`, + de: `VERBOTENE FLOSKELN (signalisieren ungegroundeten Fülltext — nie ausgeben, außer im Disclaimer) +"achte auf ausreichend Schlaf", "trinke genug Wasser", "regelmäßige Bewegung", "ärztlicher Rat empfohlen".`, +}; + +/** + * The full set, keyed by name — consumed by surfaces and by the + * cross-surface coverage test. + */ +export const SHARED_CONTRACTS = { + grounding, + toneContract, + safetyGlp1, + metricIdentifierBan, + forbiddenFiller, +} as const satisfies Record; + +export type SharedContractName = keyof typeof SHARED_CONTRACTS; + +/** + * Compose the named contract fragments for a locale, joined by a blank line. + * Used by the template-literal surfaces (Coach, period narrative) to append + * the shared block; the section-array surfaces (base-system, + * insight-generator) splice the fragments in directly. + */ +export function composeSharedContracts( + locale: ContractLocale, + names: readonly SharedContractName[], +): string { + return names.map((name) => SHARED_CONTRACTS[name][locale]).join("\n\n"); +} diff --git a/src/lib/insights/narrative/period-narrative-generate.ts b/src/lib/insights/narrative/period-narrative-generate.ts index 55c63c73c..9e1efaa8e 100644 --- a/src/lib/insights/narrative/period-narrative-generate.ts +++ b/src/lib/insights/narrative/period-narrative-generate.ts @@ -40,6 +40,7 @@ import { buildDeterministicNarrative, DETERMINISTIC_PROVIDER_TYPE, } from "@/lib/insights/narrative/period-narrative-deterministic"; +import { composeSharedContracts } from "@/lib/ai/prompts/shared-contracts"; /** * Stable identifier for the narrative prompt revision. Bumped whenever the @@ -101,7 +102,9 @@ Hard rules: - The listed drivers already survived statistical multiple-comparison control; restate them only as associations and keep their conservative meaning. - No diagnosis, no medical advice, no alarm. Calm, factual, second person ("your"). - 2 to 4 short sentences. Plain text only — no markdown, no headings, no bullet points, no emojis. -- If the context is thin, say plainly that there is little to report this period rather than inventing detail.`; +- If the context is thin, say plainly that there is little to report this period rather than inventing detail. + +${composeSharedContracts("en", ["grounding", "safetyGlp1", "metricIdentifierBan", "forbiddenFiller"])}`; const SYSTEM_PROMPT_DE = `Du fasst den Gesundheits-Tracking-ZEITRAUM einer Person (eine Woche oder einen Monat) für diese Person zusammen. Prompt-Version: ${NARRATIVE_PROMPT_VERSION}. @@ -112,7 +115,19 @@ Feste Regeln: - Die genannten Zusammenhänge haben bereits die statistische Mehrfachvergleichskorrektur überstanden; gib sie nur als Assoziationen wieder und bewahre ihre vorsichtige Bedeutung. - Keine Diagnose, kein medizinischer Rat, keine Panik. Ruhig, sachlich, in der zweiten Person ("dein"). - 2 bis 4 kurze Sätze. Nur Klartext — kein Markdown, keine Überschriften, keine Aufzählungen, keine Emojis. -- Wenn der Kontext dünn ist, sage klar, dass es in diesem Zeitraum wenig zu berichten gibt, statt Details zu erfinden.`; +- Wenn der Kontext dünn ist, sage klar, dass es in diesem Zeitraum wenig zu berichten gibt, statt Details zu erfinden. + +${composeSharedContracts("de", ["grounding", "safetyGlp1", "metricIdentifierBan", "forbiddenFiller"])}`; + +/** + * Test-only view of the composed system prompts (incl. the appended shared + * contracts), so the cross-surface coverage test can assert fragment presence + * without re-deriving the composition. + */ +export const SYSTEM_PROMPTS_FOR_TEST: Record<"de" | "en", string> = { + de: SYSTEM_PROMPT_DE, + en: SYSTEM_PROMPT_EN, +}; /** Render the typed context into a compact, model-readable block. */ export function buildNarrativeUserPrompt( From e7d14a07e81d1c174812b665a0d678665d5e479b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:11:51 +0200 Subject: [PATCH 7/9] docs(ai): record the last-working-provider coordination decision The per-worker in-process "last working provider" hint stays volatile and is deliberately NOT folded into the durable Postgres health ledger. The ledger already shares the load-bearing skip signal across workers; the only uncoordinated piece is a soft reorder whose worst case is one redundant first-hop per worker per hour. Promoting it to a per-request ledger write would add hot-path write amplification for a negligible saving. Documented inline so the trade-off is explicit. --- src/lib/ai/provider-runner.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/lib/ai/provider-runner.ts b/src/lib/ai/provider-runner.ts index 16a4c4bd8..cfb8e5795 100644 --- a/src/lib/ai/provider-runner.ts +++ b/src/lib/ai/provider-runner.ts @@ -53,6 +53,18 @@ const NEVER_SKIP: ReadonlySet = new Set(["local"]); * across workers — when scaling out we'd move this to Redis, but for * v1.4.16 the per-worker cache is enough to absorb the typical * "polling reload" pattern. + * + * v1.18.7 (LOW-8) — DECISION: the volatile per-worker hint is left + * in-process and NOT folded into the durable Postgres health ledger. The + * ledger already shares the load-bearing signal across workers (the + * skip-hint reorder in `resolveChainOrder` pushes a dead-credential / + * backoff provider to the chain tail for every worker), so the only thing + * uncoordinated is a soft "tried this one last, put it first" reorder whose + * worst case is a single redundant first-hop per worker per hour — exactly + * the bound the v1.18.7 AI audit deemed acceptable at single-maintainer + * scale. Promoting the hint to a per-request ledger write would add write + * amplification on the hot path for a negligible saving; revisit only if a + * multi-worker deploy shows measurable redundant-hop cost. */ const LAST_WORKING_TTL_MS = 60 * 60 * 1000; // 1h From 5f2a76ddbcd2a4da72a53ffb62b5c3f30ebbbbd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:16:43 +0200 Subject: [PATCH 8/9] fix(coach): close the budget-gate TOCTOU and count burned tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daily token cap was read before the provider call and recorded only after a successful reply, so concurrent requests could all read `spent < cap` and all hit the provider, and an empty/sentinel reply recorded zero spend despite burning upstream tokens. Reserve the budget atomically before the call: a single upsert-increment returns the new total, so concurrent requests serialise on the row and cannot both pass the cap; over-cap refunds the reservation and surfaces `coach.budget.exceeded` as an SSE error frame. Reconcile the reservation to the actual token count immediately after the provider returns — before any empty/sentinel short-circuit — so burned tokens are always recorded. An integration test proves two concurrent requests cannot both exceed the cap and that an empty reply still records its cost. --- src/app/api/insights/chat/route.ts | 61 ++++++++-- src/lib/ai/coach/budget.ts | 113 ++++++++++++++++++ .../coach-budget-reservation.test.ts | 109 +++++++++++++++++ tests/integration/coach-chat.test.ts | 19 ++- 4 files changed, 288 insertions(+), 14 deletions(-) create mode 100644 tests/integration/coach-budget-reservation.test.ts diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index 09ecf7d5d..89ead0497 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -62,8 +62,8 @@ import { enqueueCoachMemoryRefresh } from "@/lib/ai/coach/coach-memory-shared"; import { storeDeterministicFacts } from "@/lib/ai/coach/facts"; import { buildDateKey, - enforceBudget, - recordSpend, + reserveBudget, + reconcileSpend, } from "@/lib/ai/coach/budget"; import { detectRefusal } from "@/lib/ai/coach/refusal"; import { getCoachSystemPrompt } from "@/lib/ai/coach/system-prompt"; @@ -215,7 +215,10 @@ async function handleChatRequest(request: NextRequest): Promise { return apiError("Too many Coach requests, please wait a moment", 429); } - await enforceBudget(userId); + // v1.18.7 (SENIOR-DEV HIGH) — the daily token cap is enforced atomically + // by reserving budget BEFORE the provider call (see the reservation right + // before `runRawCompletionWithFallback`), not by a read-then-write here. + // The old read-before-call gate let concurrent requests all pass the cap. const locale = await resolveServerLocale({ request, @@ -431,6 +434,27 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`; // ConsentRequiredError → apiHandler returns 403 + `consent.ai.required`. await assertConsentForChain({ userId, chain, surface: "coach" }); + // v1.18.7 (SENIOR-DEV HIGH) — atomically RESERVE the day's budget before + // the provider call. The reservation increments the day's total by the + // per-call ceiling (`maxTokens`) in one upsert and returns the new total; + // concurrent requests serialise on the row so they cannot all pass the cap. + // Over-cap → 429 refusal frame, reservation already refunded. The actual + // token count is reconciled against this reservation after the call, + // including on empty / sentinel replies (their tokens were still burned). + const reqDateKey = buildDateKey(); + const reservation = await reserveBudget( + userId, + AI_BUDGETS.coach.maxTokens, + reqDateKey, + ); + if (!reservation.allowed) { + annotate({ + action: { name: "coach.budget.exceeded" }, + meta: { totalAfter: reservation.totalAfter }, + }); + return streamProviderError({ code: "coach.budget.exceeded" }); + } + let result: CompletionResult; let workingProviderType: string; try { @@ -447,6 +471,11 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`; result = fallback.result; workingProviderType = fallback.workingProvider.providerType; } catch (err) { + // The provider chain failed outright — no tokens were billed, so refund + // the full reservation before surfacing the error frame. + await reconcileSpend(userId, reservation.reserved, 0, reqDateKey).catch( + () => {}, + ); if (err instanceof AllProvidersFailedError) { annotate({ action: { name: "insights.coach.providerFailed" }, @@ -484,6 +513,21 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`; throw err; } + // v1.18.7 (SENIOR-DEV MEDIUM) — the provider call returned, so its tokens + // were billed regardless of reply quality. Reconcile the reservation + // against the actual count NOW, before any empty/sentinel short-circuit, so + // an empty or sentinel-only reply still records its burned cost (the old + // post-hoc `recordSpend` ran only on the happy path, undercounting these). + await reconcileSpend( + userId, + reservation.reserved, + result.tokensUsed ?? 0, + reqDateKey, + ).catch(() => { + // Ledger reconcile is best-effort; a failure leaves the conservative + // reservation in place (never an undercount) and never breaks the turn. + }); + const rawReply = (result.content ?? "").trim(); if (!rawReply) { return streamProviderError({ code: "coach.provider.empty" }); @@ -611,13 +655,10 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}.`; promptVersion: PROMPT_VERSION, }); - // Bump the day's spend ledger AFTER persistence so a retried request - // doesn't double-count when the persistence layer rolled back. - await recordSpend({ - userId, - tokens: result.tokensUsed ?? 0, - dateKey: buildDateKey(), - }); + // v1.18.7 — the day's spend was already reconciled against the + // reservation immediately after the provider returned (above), so there is + // no post-persistence ledger bump here. The reservation guarantees the + // tokens are counted even if persistence or streaming fails afterwards. annotate({ action: { name: "insights.coach.replied" }, diff --git a/src/lib/ai/coach/budget.ts b/src/lib/ai/coach/budget.ts index 897056628..02df866e7 100644 --- a/src/lib/ai/coach/budget.ts +++ b/src/lib/ai/coach/budget.ts @@ -61,6 +61,119 @@ export async function enforceBudget( } } +/** + * v1.18.7 (SENIOR-DEV HIGH) — atomically reserve budget BEFORE the provider + * call, closing the read-then-write TOCTOU window. + * + * `enforceBudget` reads spend, then `recordSpend` bumps it only AFTER a + * successful reply, so up to `rate-limit` (20/min) concurrent requests could + * each read `spent < cap` before any spend landed and all hit the provider. + * This instead does a single atomic SQL upsert that increments the day's + * total by an ESTIMATED reservation and returns the new total. The caller + * checks the returned total against the cap: if the reservation pushed it + * over, the request is refused and the reservation refunded; otherwise the + * call proceeds and the actual token count is reconciled afterwards. Mirrors + * the `rate-limit.ts` atomic-upsert pattern — multi-instance correctness is + * structural, not a hopeful read-then-write. + * + * The first request of a fresh day still lands cheaply via the upsert + * `create` branch; `messageCount` is bumped at reservation time so a + * reserved-but-failed turn still counts as an attempt. + */ +export interface ReserveBudgetResult { + /** True when the reservation kept the day's total within the cap. */ + allowed: boolean; + /** Tokens reserved by this call (refunded/reconciled by the caller). */ + reserved: number; + /** The day's total AFTER this reservation (for observability). */ + totalAfter: number; +} + +export async function reserveBudget( + userId: string, + estimatedTokens: number, + dateKey: string = buildDateKey(), + cap: number = MAX_TOKENS_PER_USER_PER_DAY, +): Promise { + const reserved = + Number.isFinite(estimatedTokens) && estimatedTokens > 0 + ? Math.floor(estimatedTokens) + : 0; + + // Single atomic upsert-increment returning the new total. Two concurrent + // requests serialise on the row's unique (user_id, date_key) constraint, so + // each observes a distinct post-increment total — they cannot both read a + // sub-cap value and both proceed. + const rows = await prisma.$queryRaw<{ total_tokens: number }[]>` + INSERT INTO coach_usage (id, user_id, date_key, total_tokens, message_count, created_at, updated_at) + VALUES (gen_random_uuid()::text, ${userId}, ${dateKey}, ${reserved}, 1, NOW(), NOW()) + ON CONFLICT (user_id, date_key) DO UPDATE SET + total_tokens = coach_usage.total_tokens + ${reserved}, + message_count = coach_usage.message_count + 1, + updated_at = NOW() + RETURNING total_tokens + `; + const totalAfter = Number(rows[0]?.total_tokens ?? reserved); + + // The cap is a ceiling on tokens already spent BEFORE this request, matching + // the prior `spent >= cap` semantics: a request is allowed when the spend + // PRIOR to its reservation was under the cap. So compare `totalAfter - + // reserved` (the prior total) against the cap. + const priorTotal = totalAfter - reserved; + if (priorTotal >= cap) { + // Already over before this request — refund the reservation + the + // message-count bump and refuse. + await refundReservation(userId, reserved, dateKey); + return { allowed: false, reserved, totalAfter: priorTotal }; + } + + return { allowed: true, reserved, totalAfter }; +} + +/** + * Reconcile a reservation against the actual tokens the provider reported. + * `actual - reserved` is applied as a signed delta (clamped so the row never + * goes negative). Called after every provider call — including empty / + * sentinel / refusal replies, whose upstream tokens were still burned + * (SENIOR-DEV MEDIUM: spend undercount). + */ +export async function reconcileSpend( + userId: string, + reserved: number, + actualTokens: number, + dateKey: string = buildDateKey(), +): Promise { + const actual = + Number.isFinite(actualTokens) && actualTokens > 0 + ? Math.floor(actualTokens) + : 0; + const delta = actual - reserved; + if (delta === 0) return; + // Clamp at zero so a smaller-than-reserved actual can't drive the row + // negative under a racing reconcile. + await prisma.$executeRaw` + UPDATE coach_usage + SET total_tokens = GREATEST(0, total_tokens + ${delta}), + updated_at = NOW() + WHERE user_id = ${userId} AND date_key = ${dateKey} + `; +} + +/** Refund a reservation (tokens + the message-count bump) on a refusal. */ +async function refundReservation( + userId: string, + reserved: number, + dateKey: string, +): Promise { + await prisma.$executeRaw` + UPDATE coach_usage + SET total_tokens = GREATEST(0, total_tokens - ${reserved}), + message_count = GREATEST(0, message_count - 1), + updated_at = NOW() + WHERE user_id = ${userId} AND date_key = ${dateKey} + `; +} + export interface RecordSpendParams { userId: string; tokens: number; diff --git a/tests/integration/coach-budget-reservation.test.ts b/tests/integration/coach-budget-reservation.test.ts new file mode 100644 index 000000000..86a7dcbd5 --- /dev/null +++ b/tests/integration/coach-budget-reservation.test.ts @@ -0,0 +1,109 @@ +/** + * v1.18.7 (SENIOR-DEV HIGH + MEDIUM) — atomic Coach budget reservation. + * + * `reserveBudget` does a single atomic upsert-increment so concurrent + * requests cannot all pass the daily cap (the old read-before-call gate + * could). `reconcileSpend` then adjusts the reservation to the actual token + * count — including on empty / sentinel replies whose tokens were still + * burned (the prior post-hoc `recordSpend` undercounted these). These tests + * prove both contracts against a real Postgres; a mocked client could not + * detect a missing atomic increment. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; + +async function seedUser(): Promise { + const user = await getPrismaClient().user.create({ + data: { username: "budget-user", email: "budget@example.test", role: "USER" }, + }); + return user.id; +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); +}); + +describe("reserveBudget (real Postgres)", () => { + it("lets at most one of two concurrent requests push past the cap", async () => { + const { reserveBudget } = await import("@/lib/ai/coach/budget"); + const userId = await seedUser(); + const dateKey = "2026-06-19"; + const cap = 1000; + + // Pre-spend 900 so a single 600-token reservation stays under the cap + // but two concurrent ones cannot both be admitted. + await getPrismaClient().coachUsage.create({ + data: { userId, dateKey, totalTokens: 900, messageCount: 1 }, + }); + + const [a, b] = await Promise.all([ + reserveBudget(userId, 600, dateKey, cap), + reserveBudget(userId, 600, dateKey, cap), + ]); + + // Exactly one is admitted: the first to commit sees prior 900 (< cap) and + // reserves to 1500; the second sees prior 1500 (>= cap) and is refused + + // refunded. Order is non-deterministic, so assert the SET of outcomes. + const allowed = [a, b].filter((r) => r.allowed).length; + expect(allowed).toBe(1); + + // The refused reservation was refunded, so the row reflects exactly one + // admitted 600-token reservation on top of the 900 baseline. + const row = await getPrismaClient().coachUsage.findUnique({ + where: { userId_dateKey: { userId, dateKey } }, + }); + expect(row?.totalTokens).toBe(1500); + }); + + it("admits the first request of a fresh day via the upsert create branch", async () => { + const { reserveBudget } = await import("@/lib/ai/coach/budget"); + const userId = await seedUser(); + const dateKey = "2026-06-20"; + + const res = await reserveBudget(userId, 600, dateKey, 25_000); + expect(res.allowed).toBe(true); + + const row = await getPrismaClient().coachUsage.findUnique({ + where: { userId_dateKey: { userId, dateKey } }, + }); + expect(row?.totalTokens).toBe(600); + expect(row?.messageCount).toBe(1); + }); + + it("reconciles the reservation down to a smaller actual count", async () => { + const { reserveBudget, reconcileSpend } = await import( + "@/lib/ai/coach/budget" + ); + const userId = await seedUser(); + const dateKey = "2026-06-21"; + + const res = await reserveBudget(userId, 600, dateKey, 25_000); + await reconcileSpend(userId, res.reserved, 120, dateKey); + + const row = await getPrismaClient().coachUsage.findUnique({ + where: { userId_dateKey: { userId, dateKey } }, + }); + // 600 reserved → reconciled to the actual 120 burned. + expect(row?.totalTokens).toBe(120); + }); + + it("still records burned tokens on an empty/sentinel reply (no undercount)", async () => { + const { reserveBudget, reconcileSpend } = await import( + "@/lib/ai/coach/budget" + ); + const userId = await seedUser(); + const dateKey = "2026-06-22"; + + // Provider returned 450 tokens but the reply was empty/sentinel: the + // reconcile must still record the 450 burned, not zero. + const res = await reserveBudget(userId, 600, dateKey, 25_000); + await reconcileSpend(userId, res.reserved, 450, dateKey); + + const row = await getPrismaClient().coachUsage.findUnique({ + where: { userId_dateKey: { userId, dateKey } }, + }); + expect(row?.totalTokens).toBe(450); + expect(row?.totalTokens).toBeGreaterThan(0); + }); +}); diff --git a/tests/integration/coach-chat.test.ts b/tests/integration/coach-chat.test.ts index 8ea8e62e5..508b4e78d 100644 --- a/tests/integration/coach-chat.test.ts +++ b/tests/integration/coach-chat.test.ts @@ -214,7 +214,12 @@ describe("POST /api/insights/chat — integration", () => { expect(usage?.messageCount).toBe(1); }); - it("returns 429 when daily token budget is exhausted", async () => { + it("refuses with a budget-exceeded error frame when the daily cap is spent", async () => { + // v1.18.7 — the cap is now enforced by an atomic reservation right + // before the provider call. An over-budget request surfaces the + // structured `coach.budget.exceeded` code as an SSE `error` frame + // (HTTP 200, like every other coach error) rather than a JSON 429, and + // the reservation is refunded so the ledger stays at the cap. const { userId } = await seedUserWithSession(); const prisma = getPrismaClient(); const dateKey = new Date().toISOString().slice(0, 10); @@ -237,10 +242,16 @@ describe("POST /api/insights/chat — integration", () => { }), }), ); - expect(res.status).toBe(429); - const env = (await res.json()) as { error: string }; - expect(env.error).toBe("coach.budget.exceeded"); + expect(res.status).toBe(200); + const body = await readStream(res); + expect(body).toContain("coach.budget.exceeded"); expect(runProviderMock).not.toHaveBeenCalled(); + + // The refused reservation was refunded — the ledger holds at the cap. + const usage = await prisma.coachUsage.findUnique({ + where: { userId_dateKey: { userId, dateKey } }, + }); + expect(usage?.totalTokens).toBe(MAX_TOKENS_PER_USER_PER_DAY); }); it("returns 404 (not 403) when conversationId belongs to another user", async () => { From ea9582641502e7b74bcd458a0550efb05ba73434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 21:20:01 +0200 Subject: [PATCH 9/9] feat(ai): make the local-host SSRF escape hatch a host allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ALLOW_LOCAL_AI_PRIVATE_HOSTS` was a binary flag that, when `true`, opened every private/internal host at once — including the cloud metadata endpoint and any internal panel. Replace the binary check at all three sites (write-time validation, local client, test resolver) with a shared `isLocalAiHostAllowed` helper: a comma-separated host list permits only those exact hostnames, while the legacy `true` still permits any private host for backward compatibility. The safeFetch / isPublicUrl floor is untouched; the allowlist only widens what the operator deliberately permits. Docs and the SSRF-pin inventory test are updated accordingly. --- docs/integrations/ai-providers.md | 21 +++-- src/app/api/user/ai-provider/route.ts | 11 ++- src/lib/__tests__/ssrf-pin-inventory.test.ts | 13 +++- .../ai/__tests__/local-host-allowlist.test.ts | 60 +++++++++++++++ src/lib/ai/local-client.ts | 9 ++- src/lib/ai/local-host-allowlist.ts | 76 +++++++++++++++++++ src/lib/ai/provider.ts | 5 +- 7 files changed, 176 insertions(+), 19 deletions(-) create mode 100644 src/lib/ai/__tests__/local-host-allowlist.test.ts create mode 100644 src/lib/ai/local-host-allowlist.ts diff --git a/docs/integrations/ai-providers.md b/docs/integrations/ai-providers.md index 547364392..2aca1fda8 100644 --- a/docs/integrations/ai-providers.md +++ b/docs/integrations/ai-providers.md @@ -134,11 +134,14 @@ API key at rest like every other provider credential. **SSRF guard.** Local endpoints route through the same validation as every other outbound URL HealthLog hits. The guard rejects -private-IP-range targets by default — set -`ALLOW_LOCAL_AI_PRIVATE_HOSTS=true` in the container environment to -allow `localhost`, RFC1918, and link-local destinations -(`src/lib/ai/provider.ts:456-462`). Leave it unset on public-facing -instances unless you specifically run an internal endpoint. +private-IP-range targets by default. `ALLOW_LOCAL_AI_PRIVATE_HOSTS` +opens the escape hatch and accepts two forms: a comma-separated host +allowlist (e.g. `ALLOW_LOCAL_AI_PRIVATE_HOSTS=ollama.lan,10.0.0.5`), +which permits only those exact hostnames, or the legacy +`ALLOW_LOCAL_AI_PRIVATE_HOSTS=true`, which permits any private host — +including the cloud-metadata endpoint, so prefer the host list. Leave +it unset on public-facing instances unless you specifically run an +internal endpoint. **Model sizing.** Roughly: 7-8B for 8 GB GPU / Apple Silicon base (briefings fine, deeper Coach reasoning gets terse), 14-24B for @@ -232,9 +235,11 @@ error. provider in `/settings/ai`, or configure an admin-shared OpenAI key in the admin panel. - **Local endpoint rejected as "internal/private host".** Set - `ALLOW_LOCAL_AI_PRIVATE_HOSTS=true` in the container environment. - Leave it unset on internet-facing instances unless you specifically - want to allow private-range targets. + `ALLOW_LOCAL_AI_PRIVATE_HOSTS` to the endpoint's exact hostname + (e.g. `ollama.lan`; comma-separate several) in the container + environment, or to `true` to allow any private host. Prefer the host + list; leave it unset on internet-facing instances unless you + specifically want to allow private-range targets. - **Codex disconnects after a few weeks of inactivity.** ChatGPT refresh tokens lapse after extended idle periods. Reconnect via the **Connect ChatGPT** button — HealthLog re-runs the device-code diff --git a/src/app/api/user/ai-provider/route.ts b/src/app/api/user/ai-provider/route.ts index 5e3c0d197..885a66a32 100644 --- a/src/app/api/user/ai-provider/route.ts +++ b/src/app/api/user/ai-provider/route.ts @@ -3,6 +3,7 @@ import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler"; import { prisma } from "@/lib/db"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { isLocalAiHostAllowed } from "@/lib/ai/local-host-allowlist"; import { encrypt, decrypt } from "@/lib/crypto"; import { resolveProviderAvailability } from "@/lib/ai/provider"; import { annotate } from "@/lib/logging/context"; @@ -92,12 +93,14 @@ export const PATCH = apiHandler(async (request: NextRequest) => { const trimmed = body.baseUrl.trim(); // SSRF guard: by default reject private/internal hostnames so a // compromised user account cannot point the server at the cloud - // metadata endpoint or internal admin panels. Ops can enable local - // Ollama / LM Studio on this instance via env flag. - const allowPrivate = process.env.ALLOW_LOCAL_AI_PRIVATE_HOSTS === "true"; + // metadata endpoint or internal admin panels. v1.18.7 (SECURITY LOW) — + // ops opt in via `ALLOW_LOCAL_AI_PRIVATE_HOSTS`, now a host allowlist: + // `true` permits any private host (legacy), a comma-separated host list + // permits only those exact hostnames. + const allowPrivate = isLocalAiHostAllowed(trimmed); if (!allowPrivate && !isPublicUrl(trimmed)) { return apiError( - "Base URL points to an internal/private host. Ops must set ALLOW_LOCAL_AI_PRIVATE_HOSTS=true on this instance to allow it (intended for self-hosted Ollama / LM Studio).", + "Base URL points to an internal/private host. Ops must allow it on this instance via ALLOW_LOCAL_AI_PRIVATE_HOSTS — set it to the exact host (e.g. ollama.lan) or to true for any private host (intended for self-hosted Ollama / LM Studio).", 422, ); } diff --git a/src/lib/__tests__/ssrf-pin-inventory.test.ts b/src/lib/__tests__/ssrf-pin-inventory.test.ts index edc8498c4..d4abdc1a6 100644 --- a/src/lib/__tests__/ssrf-pin-inventory.test.ts +++ b/src/lib/__tests__/ssrf-pin-inventory.test.ts @@ -10,9 +10,12 @@ * * The LOCAL AI client is the deliberate exception: it stays CONDITIONAL * (`requirePublicHost: !allowPrivate`) so an operator can opt into LAN hosts - * via `ALLOW_LOCAL_AI_PRIVATE_HOSTS`. We assert it keeps that conditional - * shape — never an unconditional `true` (which would break LAN local models) - * and never absent (which would re-open it for the default case). + * via `ALLOW_LOCAL_AI_PRIVATE_HOSTS`. v1.18.7 (SECURITY LOW) — that flag is + * now a host ALLOWLIST (`true` = any private host; a comma-separated list = + * only those), resolved by `isLocalAiHostAllowed`. We assert the client keeps + * the conditional shape AND derives `allowPrivate` from the allowlist helper + * (not a raw `=== "true"` binary), never an unconditional `true` (which would + * break LAN local models) and never absent (which would re-open the default). */ import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -59,5 +62,9 @@ describe("SSRF requirePublicHost pin inventory", () => { // … and must NOT be hardened to an unconditional true (that would break // a deliberately-private LAN local model). expect(src).not.toMatch(/requirePublicHost:\s*true/); + // … and `allowPrivate` must come from the host-allowlist helper, not a + // raw binary `=== "true"` (v1.18.7 — the flag became an allowlist). + expect(src).toMatch(/allowPrivate\s*=\s*isLocalAiHostAllowed\(/); + expect(src).not.toMatch(/ALLOW_LOCAL_AI_PRIVATE_HOSTS\s*===\s*"true"/); }); }); diff --git a/src/lib/ai/__tests__/local-host-allowlist.test.ts b/src/lib/ai/__tests__/local-host-allowlist.test.ts new file mode 100644 index 000000000..b989306f8 --- /dev/null +++ b/src/lib/ai/__tests__/local-host-allowlist.test.ts @@ -0,0 +1,60 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + isLocalAiHostAllowed, + requirePublicHostFor, +} from "../local-host-allowlist"; + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("isLocalAiHostAllowed", () => { + it("denies every private host when unset (secure default)", () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", ""); + expect(isLocalAiHostAllowed("http://10.0.0.5:11434/v1")).toBe(false); + expect(isLocalAiHostAllowed("http://ollama.lan/v1")).toBe(false); + }); + + it('treats "false" as denying every private host', () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "false"); + expect(isLocalAiHostAllowed("http://10.0.0.5/v1")).toBe(false); + }); + + it('keeps the legacy "true" = any private host', () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true"); + expect(isLocalAiHostAllowed("http://10.0.0.5:11434/v1")).toBe(true); + expect(isLocalAiHostAllowed("http://anything.lan/v1")).toBe(true); + }); + + it("permits ONLY the listed hosts when given a comma-separated allowlist", () => { + vi.stubEnv( + "ALLOW_LOCAL_AI_PRIVATE_HOSTS", + "ollama.lan, 10.0.0.5", + ); + expect(isLocalAiHostAllowed("http://ollama.lan:11434/v1")).toBe(true); + expect(isLocalAiHostAllowed("http://10.0.0.5:11434/v1")).toBe(true); + // A different private host is still rejected — the metadata endpoint and + // internal panels no longer ride the binary flag. + expect(isLocalAiHostAllowed("http://169.254.169.254/latest/")).toBe(false); + expect(isLocalAiHostAllowed("http://10.0.0.6/v1")).toBe(false); + }); + + it("matches hostnames case-insensitively and ignores blank entries", () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", " , Ollama.LAN , "); + expect(isLocalAiHostAllowed("http://ollama.lan/v1")).toBe(true); + }); + + it("never permits an unparseable URL", () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true"); + expect(isLocalAiHostAllowed("not a url")).toBe(true); // host-agnostic "any" + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "ollama.lan"); + expect(isLocalAiHostAllowed("not a url")).toBe(false); + }); + + it("requirePublicHostFor is the inverse of the allowlist", () => { + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "ollama.lan"); + expect(requirePublicHostFor("http://ollama.lan/v1")).toBe(false); + expect(requirePublicHostFor("http://10.0.0.6/v1")).toBe(true); + }); +}); diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index 905e4e62b..19abec405 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -1,4 +1,5 @@ import { safeFetch } from "@/lib/safe-fetch"; +import { isLocalAiHostAllowed } from "./local-host-allowlist"; import type { AIProvider, CompletionParams, CompletionResult } from "./types"; interface LocalClientConfig { @@ -49,9 +50,11 @@ export class LocalOpenAICompatibleClient implements AIProvider { // pin (issue #217) extends the same flag with a connect-time // resolved-IP check to also defeat DNS rebinding. Operators who // legitimately point at a self-hosted Ollama / LM Studio on an - // RFC1918 address opt out via `ALLOW_LOCAL_AI_PRIVATE_HOSTS=true` - // (also enforced at write-time in /api/user/ai-provider). - const allowPrivate = process.env.ALLOW_LOCAL_AI_PRIVATE_HOSTS === "true"; + // RFC1918 address opt in via `ALLOW_LOCAL_AI_PRIVATE_HOSTS` — either the + // legacy `=true` (any private host) or a comma-separated host allowlist + // (only those hostnames). Enforced at write-time in /api/user/ai-provider + // too. v1.18.7 (SECURITY LOW) — the binary flag became a host allowlist. + const allowPrivate = isLocalAiHostAllowed(url); const res = await safeFetch( url, { diff --git a/src/lib/ai/local-host-allowlist.ts b/src/lib/ai/local-host-allowlist.ts new file mode 100644 index 000000000..bf26845f0 --- /dev/null +++ b/src/lib/ai/local-host-allowlist.ts @@ -0,0 +1,76 @@ +/** + * v1.18.7 (SECURITY LOW) — host allowlist for local/private AI endpoints. + * + * The operator escape-hatch for pointing the server at a self-hosted + * Ollama / LM Studio on an RFC1918 address used to be a single binary flag, + * `ALLOW_LOCAL_AI_PRIVATE_HOSTS=true`, which opened EVERY private/internal + * host at once — including the cloud-metadata endpoint and any internal admin + * panel. This narrows it to an explicit allowlist while keeping the binary + * `true` for backward compatibility: + * + * - unset / "" / "false" → no private host allowed (the secure default). + * - "true" → ANY private host allowed (legacy behaviour). + * - "ollama.lan, 10.0.0.5" → ONLY those exact hostnames allowed; every + * other private host is still rejected. + * + * This is layered ON TOP of the `safeFetch` / `isPublicUrl` floor: a public + * URL never needs the allowlist, and a private host not on the list is + * rejected exactly as before. The allowlist only ever WIDENS what the + * operator deliberately permits, never narrows the public-host floor. + */ + +const ENV_VAR = "ALLOW_LOCAL_AI_PRIVATE_HOSTS"; + +/** Parsed allowlist policy from the env var. */ +type LocalHostPolicy = + | { kind: "none" } + | { kind: "any" } + | { kind: "hosts"; hosts: ReadonlySet }; + +function parsePolicy(): LocalHostPolicy { + const raw = process.env[ENV_VAR]?.trim(); + if (!raw || raw.toLowerCase() === "false") return { kind: "none" }; + if (raw.toLowerCase() === "true") return { kind: "any" }; + const hosts = raw + .split(",") + .map((h) => h.trim().toLowerCase()) + .filter((h) => h.length > 0); + if (hosts.length === 0) return { kind: "none" }; + return { kind: "hosts", hosts: new Set(hosts) }; +} + +/** Extract the lowercased hostname from a URL string, or null if unparseable. */ +function hostnameOf(url: string): string | null { + try { + return new URL(url).hostname.toLowerCase(); + } catch { + return null; + } +} + +/** + * True when the operator has explicitly permitted this (private) host for a + * local AI endpoint. `"true"` permits any host (legacy); a host list permits + * only listed hostnames. An unparseable URL is never permitted. + * + * NOTE: this answers "did the operator opt this host in?", NOT "is this host + * public?". Callers still run the `isPublicUrl` / `safeFetch` floor; this is + * the private-host override on top of it. + */ +export function isLocalAiHostAllowed(url: string): boolean { + const policy = parsePolicy(); + if (policy.kind === "none") return false; + if (policy.kind === "any") return true; + const host = hostnameOf(url); + return host !== null && policy.hosts.has(host); +} + +/** + * Whether `safeFetch` must require a public host for this URL: true unless the + * operator has opted this private host into the allowlist. Kept as a thin + * wrapper so the call sites read `requirePublicHost: requirePublicHostFor(url)` + * and the conditional intent stays obvious. + */ +export function requirePublicHostFor(url: string): boolean { + return !isLocalAiHostAllowed(url); +} diff --git a/src/lib/ai/provider.ts b/src/lib/ai/provider.ts index 93e5e2df2..f4c9884f1 100644 --- a/src/lib/ai/provider.ts +++ b/src/lib/ai/provider.ts @@ -11,6 +11,7 @@ import { decryptCodexCreds, } from "./codex-oauth"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { isLocalAiHostAllowed } from "./local-host-allowlist"; import { parseProviderChain, type ProviderChainType } from "./provider-chain"; import type { ProviderChainResolved } from "./provider-runner"; @@ -668,7 +669,9 @@ export async function resolveProviderForTest( if (!baseUrl) { throw new AITestConfigError(422, "Local provider requires a base URL"); } - const allowPrivate = process.env.ALLOW_LOCAL_AI_PRIVATE_HOSTS === "true"; + // v1.18.7 (SECURITY LOW) — host allowlist (`true` = any private host; + // a comma-separated host list = only those) replaces the binary flag. + const allowPrivate = isLocalAiHostAllowed(baseUrl); if (!allowPrivate && !isPublicUrl(baseUrl)) { throw new AITestConfigError( 422,