From 203068d5bacc0084ac81c309481159ac4fd3da2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 27 Sep 2026 13:06:02 +0200 Subject: [PATCH] fix(coach): keep a billed reply when its tables fail, and fit tables once Tables are now projected outside the provider's failure path: a projection defect costs the turn its tables and is annotated, instead of refunding tokens already billed and dropping the reply. The at-rest ceiling for a message's tables is applied once, when the dialog is assembled, before anything streams. A table trimmed only at write time used to show live and read back as withheld after a reload. --- src/lib/ai/coach/persistence.ts | 23 +++-------- src/lib/ai/coach/results/project.ts | 28 +++++++++++++ .../turn/__tests__/dialog-frames.test.ts | 35 +++++++++++++++++ .../turn/__tests__/model-results.test.ts | 39 +++++++++++++++++++ src/lib/ai/coach/turn/model.ts | 30 ++++++++++++-- src/lib/ai/coach/turn/provenance.ts | 10 ++++- 6 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 src/lib/ai/coach/turn/__tests__/model-results.test.ts diff --git a/src/lib/ai/coach/persistence.ts b/src/lib/ai/coach/persistence.ts index f5a1966cd..2822bacd9 100644 --- a/src/lib/ai/coach/persistence.ts +++ b/src/lib/ai/coach/persistence.ts @@ -23,6 +23,7 @@ import { coachStepSchema, } from "./stream-events"; import { COACH_CONVERSATION_TITLE_MAX } from "./types"; +import { RESULTS_MAX_BYTES, fitResultsToStorage } from "./results/project"; import { isCheckupIntervalId, isSuggestedActionType, @@ -392,31 +393,19 @@ export interface AppendMessageParams { results?: CoachResultTable[]; } -/** - * v1.39.4 — the at-rest ceiling for one message's tables, as JSON before - * encryption. Tables arrive trimmed to 400 rows; a message whose tables still - * exceed this keeps the leading tables that fit, and a reader is told the - * rest are unavailable rather than handed a partial table. - */ -export const RESULTS_MAX_BYTES = 128 * 1024; +export { RESULTS_MAX_BYTES }; /** * Serialise a turn's tables for the ciphertext column, or null when there are - * none. Drops whole tables from the end until the JSON fits the ceiling. + * none. The turn already fitted them (`fitResultsToStorage`, before it + * streamed them); fitting again here only guards a caller that did not. */ function resultsToBytes( results: CoachResultTable[] | undefined, ): Uint8Array | null { if (!results || results.length === 0) return null; - const kept = results.slice(0, MAX_RESULTS_PER_MESSAGE); - while (kept.length > 0) { - const json = JSON.stringify(kept); - if (Buffer.byteLength(json, "utf8") <= RESULTS_MAX_BYTES) { - return encryptToBytes(json); - } - kept.pop(); - } - return null; + const kept = fitResultsToStorage(results.slice(0, MAX_RESULTS_PER_MESSAGE)); + return kept.length > 0 ? encryptToBytes(JSON.stringify(kept)) : null; } /** diff --git a/src/lib/ai/coach/results/project.ts b/src/lib/ai/coach/results/project.ts index bbf1b56f2..5435780aa 100644 --- a/src/lib/ai/coach/results/project.ts +++ b/src/lib/ai/coach/results/project.ts @@ -22,6 +22,34 @@ import { MAX_RESULTS_PER_TURN, isTurnResultRef } from "./refs"; /** At most this many rows reach a table; a year of days fits. */ export const RESULT_TABLE_MAX_ROWS = 400; +/** + * The at-rest ceiling for one message's tables, as JSON before encryption. + * Tables arrive trimmed to 400 rows; a message whose tables still exceed + * this keeps the leading tables that fit. + */ +export const RESULTS_MAX_BYTES = 128 * 1024; + +/** + * The tables a message keeps: the leading ones whose JSON fits + * `RESULTS_MAX_BYTES`, whole tables dropped from the end. Applied once, + * before the turn streams them, so the tables the person sees live are the + * tables a reload reads back, and the metadata never names a table the + * ciphertext does not hold. + */ +export function fitResultsToStorage( + results: readonly CoachResultTable[], +): CoachResultTable[] { + const kept = results.slice(0, MAX_RESULTS_PER_TURN); + while ( + kept.length > 0 && + new TextEncoder().encode(JSON.stringify(kept)).byteLength > + RESULTS_MAX_BYTES + ) { + kept.pop(); + } + return kept; +} + /** One tool call of this turn, as the loop settled it. */ export interface SettledToolCall { name: string; diff --git a/src/lib/ai/coach/turn/__tests__/dialog-frames.test.ts b/src/lib/ai/coach/turn/__tests__/dialog-frames.test.ts index bcd5f85ed..c3499db95 100644 --- a/src/lib/ai/coach/turn/__tests__/dialog-frames.test.ts +++ b/src/lib/ai/coach/turn/__tests__/dialog-frames.test.ts @@ -21,6 +21,7 @@ import { DEFAULT_COACH_PREFS } from "@/lib/validations/coach-prefs"; import { emitReply } from "../sse"; import { assembleTurnDialog, buildTurnProvenance } from "../provenance"; +import { RESULTS_MAX_BYTES } from "@/lib/ai/coach/results/project"; import type { ModelOutcome } from "../model"; import type { GuardedReply } from "../reply-guards"; @@ -170,6 +171,40 @@ describe("assembleTurnDialog", () => { expect(JSON.stringify(provenance)).not.toContain("128"); }); + it("streams only the tables the message can keep, so a reload shows the same ones", () => { + // A year of days with long period keys: two such tables exceed the + // at-rest ceiling together, one fits. + const big = (ref: string): CoachResultTable => ({ + ...TABLE, + ref, + rowCount: 400, + rows: Array.from({ length: 400 }, (_, i) => [ + `2026-01-01-${"x".repeat(160)}-${i}`, + 120 + (i % 30), + ]), + }); + const tables = [big("r1"), big("r2")]; + expect(JSON.stringify(tables).length).toBeGreaterThan(RESULTS_MAX_BYTES); + const dialog = assembleTurnDialog({ + model: { ...model, results: tables }, + reply: reply(false), + prefs: DEFAULT_COACH_PREFS, + locale: "en", + }); + expect(dialog.results.map((t) => t.ref)).toEqual(["r1"]); + const provenance = buildTurnProvenance({ + snapshotProvenance: { windows: [], metrics: [] }, + reply: reply(false), + suggestion: null, + action: null, + toolTrace: [], + steps: [], + dialog, + forcedFinal: false, + }); + expect(provenance.results?.map((r) => r.ref)).toEqual(["r1"]); + }); + it("drops tables, chips and clarification on a blocked turn", () => { const dialog = assembleTurnDialog({ model, diff --git a/src/lib/ai/coach/turn/__tests__/model-results.test.ts b/src/lib/ai/coach/turn/__tests__/model-results.test.ts new file mode 100644 index 000000000..08d5cbbf4 --- /dev/null +++ b/src/lib/ai/coach/turn/__tests__/model-results.test.ts @@ -0,0 +1,39 @@ +/** + * v1.39.4 — the tables of a settled turn are built outside the provider's + * failure path: a projection defect costs the turn its tables, never the + * billed reply. + */ +import { describe, expect, it, vi } from "vitest"; + +const m = vi.hoisted(() => ({ + projectResults: vi.fn(), + annotate: vi.fn(), +})); + +vi.mock("@/lib/db", () => ({ prisma: {} })); +vi.mock("@/lib/logging/context", () => ({ annotate: m.annotate })); +vi.mock("@/lib/ai/coach/persistence", () => ({ appendMessage: vi.fn() })); +vi.mock("@/lib/ai/coach/results/project", async (importOriginal) => ({ + ...(await importOriginal()), + projectResults: m.projectResults, +})); + +import { buildTurnResults } from "../model"; + +describe("buildTurnResults", () => { + it("returns no tables and records why when the projection throws", () => { + m.projectResults.mockImplementation(() => { + throw new TypeError("rows is not iterable"); + }); + expect(buildTurnResults([], "en")).toEqual([]); + expect(m.annotate).toHaveBeenCalledWith({ + action: { name: "coach.results.project_failed" }, + meta: { error: "TypeError", calls: 0 }, + }); + }); + + it("passes the projected tables through with their chart", () => { + m.projectResults.mockReturnValue([]); + expect(buildTurnResults([], "en")).toEqual([]); + }); +}); diff --git a/src/lib/ai/coach/turn/model.ts b/src/lib/ai/coach/turn/model.ts index af8603f07..166c77687 100644 --- a/src/lib/ai/coach/turn/model.ts +++ b/src/lib/ai/coach/turn/model.ts @@ -152,6 +152,30 @@ function withChart(table: CoachResultTable): CoachResultTable { return { ...table, chart, chartKind: chart?.kind ?? null }; } +/** + * The tables a settled turn shows. Built outside the provider's failure + * path: the model has already answered and its tokens are billed, so a + * projection defect must not refund them or drop the reply. It costs the + * turn its tables, which the annotation records, and nothing else. + */ +export function buildTurnResults( + calls: Parameters[0]["calls"], + locale: Locale, +): CoachResultTable[] { + try { + return projectResults({ calls, locale }).map(withChart); + } catch (err) { + annotate({ + action: { name: "coach.results.project_failed" }, + meta: { + error: err instanceof Error ? err.name : "unknown", + calls: calls.length, + }, + }); + return []; + } +} + /** v1.39.4 — server-authored lines appended to the system prompt. */ function appendBlocks(base: string, blocks: string[]): string { const extra = blocks.filter((block) => block.length > 0); @@ -305,10 +329,10 @@ export async function runTurnModel(args: { totalTokens: loop.totalTokens, cachedTokens: loop.cachedTokens, steps: steps.list(), - results: projectResults({ - calls: settled.filter((call) => call !== undefined), + results: buildTurnResults( + settled.filter((call) => call !== undefined), locale, - }).map(withChart), + ), forcedFinal: loop.forcedFinal === true, inventory: inventory.entries, correlations: correlationPartners( diff --git a/src/lib/ai/coach/turn/provenance.ts b/src/lib/ai/coach/turn/provenance.ts index eb68aaead..0f223a469 100644 --- a/src/lib/ai/coach/turn/provenance.ts +++ b/src/lib/ai/coach/turn/provenance.ts @@ -28,6 +28,7 @@ import { import { numberFollowUps } from "@/lib/ai/coach/follow-ups/catalog"; import { buildContinueFollowUp } from "@/lib/ai/coach/follow-ups/continue"; import type { CoachPrefs } from "@/lib/validations/coach-prefs"; +import { fitResultsToStorage } from "@/lib/ai/coach/results/project"; import type { ModelOutcome } from "./model"; import type { GuardedReply } from "./reply-guards"; @@ -76,10 +77,15 @@ export function assembleTurnDialog(args: { const { model, reply, prefs, locale } = args; const blocked = reply.outboundBlocked; const referenced = new Set(reply.referencedResults); + // Fitted to the at-rest ceiling here, before anything streams, so the + // tables shown live, the metadata and the ciphertext name the same set: a + // table trimmed only at write time showed live and read back withheld. const results = blocked ? [] - : model.results.map((table) => - referenced.has(table.ref) ? { ...table, displayed: true } : table, + : fitResultsToStorage( + model.results.map((table) => + referenced.has(table.ref) ? { ...table, displayed: true } : table, + ), ); const metas = results.map(toResultMeta); const method = buildMethod({ steps: model.steps, results: metas, locale });