Merge AI client + coach cache fixes into release/v1.20.0

Send the OpenAI JSON response format only when JSON is requested and no
tools are in play, so the Coach prose path (and the tool loop's final
round) is no longer corrupted into a JSON blob. Resolve the Anthropic
finish reason to tool_calls whenever tool_use blocks are present. Memoize
the per-request feature extraction and prefs read and project the feature
query, so the coach tool fan-out shares one read instead of six.
This commit is contained in:
Marc-André Bombeck committed 2026-06-22 08:19:12 +02:00
commit d3b341f8a7
12 files changed
+279 -43

No files matched your search

+3
View File
@@ -72,6 +72,9 @@ export const POST = apiHandler(async (request: NextRequest) => {
user: 'Reply with the JSON object {"ok": true} and nothing else.',
temperature: 0,
maxTokens: 32,
// The probe asks for a JSON object — keep the OpenAI / Codex strict
// JSON mode it relied on before the response_format gate landed.
responseFormat: "json",
}),
);
+5
View File
@@ -682,6 +682,9 @@ export const POST = apiHandler((request: NextRequest) =>
user: userPrompt,
temperature: 0.3,
maxTokens: 1500,
// The reply is parsed with `JSON.parse` below, so opt the OpenAI /
// Codex chains into their strict JSON mode (gated on this flag).
responseFormat: "json",
}),
});
result = fallback.result;
@@ -815,6 +818,8 @@ export const POST = apiHandler((request: NextRequest) =>
user: `${userPrompt}\n\n${buildBriefingGroundingCorrection(ungrounded)}`,
temperature: 0.3,
maxTokens: 1500,
// Parsed with `JSON.parse` below — opt OpenAI / Codex into JSON mode.
responseFormat: "json",
}),
});
const parsedRetry = JSON.parse(retry.result.content);
+3
View File
@@ -195,6 +195,9 @@ async function handleExtract(request: NextRequest): Promise<Response> {
// The reply body is a single small JSON object; 600 is well
// over the empirical max we have seen on a 2k-char input.
maxTokens: 600,
// Parsed with `parseModelJson` (JSON.parse) below — opt the OpenAI /
// Codex chains into their strict JSON-object mode (gated on this flag).
responseFormat: "json",
}),
});
} catch (err) {
@@ -337,4 +337,53 @@ describe("AnthropicClient", () => {
expect(result.finishReason).toBe("tool_calls");
expect(result.cachedInputTokens).toBe(40);
});
it("derives finishReason from tool_use blocks even on an unexpected stop_reason (M-2)", async () => {
// Anthropic normally pairs tool_use blocks with stop_reason "tool_use".
// If it ever returns the blocks under a different (or absent) stop_reason,
// the loop must still see finishReason "tool_calls" so it executes the tool
// round instead of surfacing the empty tool_use-only reply as the answer.
const mockFetch = vi.fn().mockResolvedValue({
ok: true,
json: () =>
Promise.resolve({
content: [
{
type: "tool_use",
id: "toolu_9",
name: "fetch_glucose",
input: { window: "last30days" },
},
],
// Deliberately NOT "tool_use" — the pre-fix code derived finishReason
// from this field alone and would have returned undefined here.
stop_reason: "end_turn",
usage: { input_tokens: 10, output_tokens: 4 },
}),
});
vi.stubGlobal("fetch", mockFetch);
const client = new AnthropicClient({
apiKey: "sk-ant-x",
model: "claude-sonnet-4-6",
});
const result = await client.generateCompletion(
singleUserTurn({
system: "s",
user: "u",
tools: [
{
name: "fetch_glucose",
description: "Fetch glucose readings",
parameters: { type: "object", properties: {} },
},
],
toolChoice: "auto",
}),
);
expect(result.toolCalls).toHaveLength(1);
expect(result.finishReason).toBe("tool_calls");
});
});
@@ -28,6 +28,7 @@ describe("OpenAIClient", () => {
singleUserTurn({
system: "You are a doctor.",
user: "Analyze this data.",
responseFormat: "json",
}),
);
@@ -46,6 +47,66 @@ describe("OpenAIClient", () => {
expect(body).not.toHaveProperty("seed");
});
it("forces JSON mode only when responseFormat is json AND no tools (M-1)", async () => {
const replyContent = (content: string) =>
vi.fn().mockResolvedValue({
ok: true,
json: () =>
Promise.resolve({
choices: [{ message: { content } }],
usage: { total_tokens: 1 },
}),
});
const client = new OpenAIClient({
apiKey: "sk-test",
model: "gpt-4o-mini",
baseUrl: "https://api.openai.com/v1",
});
// 1. Prose Coach path (no responseFormat, no tools) → NO json_object. This
// is the F1 forced-final round contract: JSON mode would corrupt prose.
const proseFetch = replyContent("Here is a plain-prose reply.");
vi.stubGlobal("fetch", proseFetch);
await client.generateCompletion(
singleUserTurn({ system: "coach", user: "how am I doing?" }),
);
expect(JSON.parse(proseFetch.mock.calls[0][1].body)).not.toHaveProperty(
"response_format",
);
// 2. responseFormat:"json" but tools present → still NO json_object (a tool
// round must not be coerced into a JSON object).
const toolFetch = replyContent("{}");
vi.stubGlobal("fetch", toolFetch);
await client.generateCompletion({
system: "s",
messages: [{ role: "user", content: "u" }],
responseFormat: "json",
tools: [
{
name: "t",
description: "d",
parameters: { type: "object", properties: {} },
},
],
});
expect(JSON.parse(toolFetch.mock.calls[0][1].body)).not.toHaveProperty(
"response_format",
);
// 3. responseFormat:"json" with no tools → json_object IS sent (insight /
// extraction callers keep strict JSON).
const jsonFetch = replyContent('{"summary":"x"}');
vi.stubGlobal("fetch", jsonFetch);
await client.generateCompletion(
singleUserTurn({ system: "s", user: "u", responseFormat: "json" }),
);
expect(JSON.parse(jsonFetch.mock.calls[0][1].body).response_format).toEqual(
{ type: "json_object" },
);
});
it("threads a deterministic seed onto the request body when supplied", async () => {
const mockFetch = vi.fn().mockResolvedValue({
ok: true,
+14 -6
View File
@@ -335,14 +335,22 @@ export class AnthropicClient implements AIProvider {
const tokensUsed = inputTokens + outputTokens || null;
const cachedInputTokens = json.usage?.cache_read_input_tokens ?? null;
// The presence of tool_use blocks is authoritative for the tool-loop gate:
// F1's loop continues only when `finishReason === "tool_calls"`. Anthropic
// normally pairs tool_use blocks with `stop_reason: "tool_use"`, but if it
// ever returns the blocks under a different (or absent) stop_reason, deriving
// the reason from `stop_reason` alone would drop the tool request and the
// loop would surface the empty tool_use-only reply as the final answer.
const finishReason: CompletionResult["finishReason"] =
json.stop_reason === "tool_use"
toolCalls && toolCalls.length > 0
? "tool_calls"
: json.stop_reason === "max_tokens"
? "length"
: json.stop_reason === "end_turn"
? "stop"
: undefined;
: json.stop_reason === "tool_use"
? "tool_calls"
: json.stop_reason === "max_tokens"
? "length"
: json.stop_reason === "end_turn"
? "stop"
: undefined;
return {
content,
@@ -63,6 +63,10 @@ vi.mock("@/lib/logging/context", () => ({
}) => {
annotateCalls.push({ name: fields.action?.name, meta: fields.meta });
},
// The snapshot's request-scoped read memoisation (`memoizePerRequest`) reads
// the active event here; no request context in this unit test → null, so the
// memoiser falls through to the factory and behaviour is unchanged.
getEvent: () => null,
}));
vi.mock("./glp1-snapshot", () => ({
@@ -7,6 +7,8 @@ import {
__resetCoachSnapshotCacheForTests,
buildCoachSnapshot,
} from "../snapshot";
import { eventStorage } from "@/lib/logging/context";
import { WideEventBuilder } from "@/lib/logging/event-builder";
vi.mock("@/lib/db", () => ({
prisma: {
@@ -994,3 +996,67 @@ describe("buildCoachSnapshot — reference grounding (W7)", () => {
}
});
});
describe("buildCoachSnapshot request-scoped read sharing (H-1)", () => {
beforeEach(() => {
vi.clearAllMocks();
__resetCoachSnapshotCacheForTests();
prismaMock.measurement.findMany.mockResolvedValue([]);
prismaMock.moodEntry.findMany.mockResolvedValue([]);
prismaMock.medicationIntakeEvent.findMany.mockResolvedValue([]);
prismaMock.medication.findMany.mockResolvedValue([]);
prismaMock.user.findUnique.mockResolvedValue({ coachPrefsJson: null });
resolveModuleMapMock.mockResolvedValue(allModulesEnabled());
featuresMock.mockResolvedValue({
bloodPressure: undefined,
weight: undefined,
pulse: undefined,
mood: undefined,
});
});
it("runs extractFeatures + the prefs read once across distinct-scope tool builds in one request", async () => {
// The F1 coach tools each call buildCoachSnapshot with a DIFFERENT
// single-source scope, so the 60s snapshot LRU (keyed on the source list)
// does NOT share their reads. Memoising the two heavy per-user reads on the
// request-scoped WideEventBuilder cache collapses the fan-out to one of each.
await eventStorage.run(new WideEventBuilder(), async () => {
// Same window (→ same windowDays key), distinct sources (→ distinct LRU
// keys ⇒ two separate snapshot builds, mirroring the real tool fan-out).
await Promise.all([
buildCoachSnapshot("user-h1", {
sources: ["bp"],
window: "last30days",
}),
buildCoachSnapshot("user-h1", {
sources: ["pulse"],
window: "last30days",
}),
]);
});
expect(featuresMock).toHaveBeenCalledTimes(1);
expect(prismaMock.user.findUnique).toHaveBeenCalledTimes(1);
});
it("keeps separate caches per request (no cross-request leakage)", async () => {
// Distinct users so the in-process 60s snapshot LRU (keyed on userId) cannot
// short-circuit the second request — the only thing under test here is that
// the request-scoped feature/prefs cache does NOT survive across requests.
await eventStorage.run(new WideEventBuilder(), async () => {
await buildCoachSnapshot("user-h1a", {
sources: ["bp"],
window: "last30days",
});
});
await eventStorage.run(new WideEventBuilder(), async () => {
await buildCoachSnapshot("user-h1b", {
sources: ["bp"],
window: "last30days",
});
});
expect(featuresMock).toHaveBeenCalledTimes(2);
expect(prismaMock.user.findUnique).toHaveBeenCalledTimes(2);
});
});
+43 -28
View File
@@ -43,6 +43,7 @@ import {
} from "@/lib/insights/derived/sleep-rhythm";
import { compactSections } from "@/lib/ai/prompts/compact-sections";
import { annotate } from "@/lib/logging/context";
import { memoizePerRequest } from "@/lib/request-cache";
import { buildGlp1SnapshotBlock } from "./glp1-snapshot";
import { buildDerivedSnapshotBlock } from "./derived-snapshot";
import { buildCoachMemoryBlock } from "./memory-snapshot";
@@ -670,31 +671,38 @@ async function buildCoachSnapshotImpl(
// single extra round-trip at most. Disabled modules fold into the same
// SYSTEM-side exclusion the user's `excludeMetrics` flow already drives.
const moduleMapPromise = resolveModuleMap(userId);
const prefsRow = await prisma.user.findUnique({
where: { id: userId },
select: {
coachPrefsJson: true,
timezone: true,
locale: true,
// v1.11.5 — needed to collapse a dual-source sleep night to one
// canonical source before reconstructing per-night asleep totals.
sourcePriorityJson: true,
// v1.15 — the cycle snapshot block is gated on the resolved cycle
// toggle (an explicit opt-in/out overrides the gender default). Read
// both columns here so a non-cycle account pays no extra round-trip.
gender: true,
cycleProfile: { select: { cycleTrackingEnabled: true } },
// v1.16.16 — the glucose block converts canonical mg/dL to the user's
// display unit so the Coach reads the same number every other surface
// shows. Read it on this existing prefs hop (no extra round-trip).
glucoseUnit: true,
// v1.18.6 (W7) — the explicit, user-declared diabetes opt-in. Selects
// the tighter ADA glycemic GOAL band for the glucose reference-grounding
// line only; never inferred from a reading, never a diagnosis. Read on
// this existing prefs hop (no extra round-trip).
hasDiabetes: true,
},
});
// v1.20.0 (H-1) — the F1 coach tools each rebuild a single-source snapshot
// with a distinct LRU key, so the 60s snapshot cache does not share this read
// across the fan-out. Memoise it per-request (the select shape is constant, so
// userId is the only key) so up to 6 concurrent tool builds collapse to one
// prefs round-trip instead of starving the shared Prisma pool.
const prefsRow = await memoizePerRequest(`coach-prefs:${userId}`, () =>
prisma.user.findUnique({
where: { id: userId },
select: {
coachPrefsJson: true,
timezone: true,
locale: true,
// v1.11.5 — needed to collapse a dual-source sleep night to one
// canonical source before reconstructing per-night asleep totals.
sourcePriorityJson: true,
// v1.15 — the cycle snapshot block is gated on the resolved cycle
// toggle (an explicit opt-in/out overrides the gender default). Read
// both columns here so a non-cycle account pays no extra round-trip.
gender: true,
cycleProfile: { select: { cycleTrackingEnabled: true } },
// v1.16.16 — the glucose block converts canonical mg/dL to the user's
// display unit so the Coach reads the same number every other surface
// shows. Read it on this existing prefs hop (no extra round-trip).
glucoseUnit: true,
// v1.18.6 (W7) — the explicit, user-declared diabetes opt-in. Selects
// the tighter ADA glycemic GOAL band for the glucose reference-grounding
// line only; never inferred from a reading, never a diagnosis. Read on
// this existing prefs hop (no extra round-trip).
hasDiabetes: true,
},
}),
);
const prefs = parseCoachPrefs(prefsRow?.coachPrefsJson);
// Resolve the UI locale for the rolling-profile narrative recall. The
// narrative rows are keyed by ("de" | "en"); default to "de" (the app
@@ -759,9 +767,16 @@ async function buildCoachSnapshotImpl(
// `userId` + the resolved window/sources), so running them
// concurrently shaves a round-trip off the cold path. No block reads
// `features` before the shared await, so the deferral is safe.
const featuresPromise = extractFeatures(userId, false, {
sinceDays: windowDays,
});
// v1.20.0 (H-1) — `extractFeatures` is the heaviest read on the cold path
// (user findUnique + a windowed measurement findMany + the all-time extremes).
// The F1 tools rebuild distinct-scope snapshots whose LRU keys differ, so they
// do not share this read; memoise it per-request keyed on the only varying
// input (windowDays) so the fan-out runs it once instead of up to 6× against
// the shared pool.
const featuresPromise = memoizePerRequest(
`coach-features:${userId}:${windowDays}`,
() => extractFeatures(userId, false, { sinceDays: windowDays }),
);
// Trim down to the metrics the Coach narrates. extractFeatures
// returns more (sleep, steps, etc.) — the Coach surface keeps the
+14 -5
View File
@@ -49,10 +49,17 @@ export class OpenAIClient implements AIProvider {
// v1.20.0 — tool plumbing. The defs map onto the OpenAI `function` tool
// wire; F1 supplies real defs and consumes the parsed `toolCalls`. No F4
// call site sets `tools`, so the text-only body is unchanged.
const tools =
params.tools && params.tools.length > 0
? buildOpenAITools(params.tools)
: undefined;
const hasTools = !!params.tools && params.tools.length > 0;
const tools = hasTools ? buildOpenAITools(params.tools!) : undefined;
// Only force OpenAI's strict JSON mode when the caller actually consumes a
// JSON object AND no tools are in play — mirrors the Anthropic client's
// `usePrefill` gate. JSON mode coerces `message.content` into a valid JSON
// object, which contradicts the Coach prose contract; the F1 tool loop's
// forced-final round (toolChoice:"none", no tools, no responseFormat) must
// therefore stay out of JSON mode, and every tool round is non-JSON by
// construction. Insight/extraction callers opt in with `responseFormat:"json"`.
const useJsonFormat = params.responseFormat === "json" && !hasTools;
const res = await safeFetch(
url,
@@ -67,7 +74,9 @@ export class OpenAIClient implements AIProvider {
messages,
temperature: params.temperature ?? 0.3,
max_tokens: params.maxTokens ?? 1000,
response_format: { type: "json_object" },
...(useJsonFormat
? { 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).
+5 -4
View File
@@ -246,13 +246,14 @@ export interface CompletionParams {
seed?: number;
/**
* v1.18.7 — opt the structured (JSON) surfaces into the provider's
* strongest JSON-reliability mode. When `"json"`:
* strongest JSON-reliability mode. When `"json"` (and no `tools`):
* - 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.
* - OpenAI / Codex: sends `response_format: { type: "json_object" }`.
* The Coach (prose, NOT JSON) leaves this unset, so no provider coerces
* its reply into a JSON object — including the OpenAI client, which now
* gates `response_format` on this flag rather than pinning it always.
*
* Mutually exclusive with `tools` on Anthropic: the `{`-prefill injects an
* assistant turn that collides with a `tool_use` response, so the Anthropic
+12
View File
@@ -681,6 +681,18 @@ export async function extractFeatures(
? { userId, measuredAt: { gte: sinceCutoff }, deletedAt: null }
: { userId, deletedAt: null },
orderBy: { measuredAt: "asc" },
// Project only the columns every downstream consumer reads (`byType`,
// `summarize`, BP pairing, and `reconstructSleepNights`'s `SleepStageRow`).
// The PULSE / glucose windows are 200k-row-class; pulling every column
// (notes, externalId, …) is pure wasted I/O on the shared Prisma pool.
select: {
type: true,
value: true,
measuredAt: true,
sleepStage: true,
source: true,
deviceType: true,
},
});
// v1.18.11 P1 — when the bulk read is bounded to a recent window, the