From ad541cd5bb4b33251add779b61637f0664ee3990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 27 Sep 2026 12:11:54 +0200 Subject: [PATCH] fix(coach): show a step for metric tables and for a table shown again get_metric_table now gets a step like every other read: the domain is its validated metric, and once the table is in, the window, period and granularity are the ones the table was built with (after the executor's defaults). The count is the readings the table folds, taken from the summary the server computed, so a read past the sixth table still counts. A step whose call produced a table now names it (resultRef), for the projected tables of the older tools as well. show_result gets its own step. It reads nothing new: while it runs it names no domain, and once the stored table is found it takes the source of the read that built it and the name the copy carries in this turn. A table that could not be shown is a failed step with the reason, never an empty read. The property test now drives show_result too and checks resultRef; the coverage test gives each tool the smallest arguments it accepts instead of passing empty arguments to a tool that requires a metric. --- src/lib/ai/coach/turn/__tests__/steps.test.ts | 270 +++++++++++++++++- src/lib/ai/coach/turn/steps.ts | 157 +++++++++- 2 files changed, 415 insertions(+), 12 deletions(-) diff --git a/src/lib/ai/coach/turn/__tests__/steps.test.ts b/src/lib/ai/coach/turn/__tests__/steps.test.ts index dd311d80e..ede739550 100644 --- a/src/lib/ai/coach/turn/__tests__/steps.test.ts +++ b/src/lib/ai/coach/turn/__tests__/steps.test.ts @@ -22,11 +22,14 @@ import { import { coachScopeSourceSchema, coachScopeWindowSchema, + type CoachResultTable, type CoachStep, type CoachStepDomain, } from "@/lib/ai/coach/types"; +import { summariseTable } from "@/lib/ai/coach/results/metric-table-tool"; import { COACH_TOOL_NAMES, + SHOW_RESULT_TOOL_NAME, parseCoachToolArgs, } from "@/lib/ai/coach/tools/definitions"; import type { CoachToolResult } from "@/lib/ai/coach/tools/executor"; @@ -43,6 +46,8 @@ const DOMAINS: CoachStepDomain[] = [ "correlations", "snapshot", ]; +/** Every call a turn can make: the catalogue and `show_result`. */ +const STEP_TOOLS = [...COACH_TOOL_NAMES, SHOW_RESULT_TOOL_NAME] as const; const STATUSES = new Set(["running", "done", "empty", "failed"]); const REASONS = new Set([ "no_data", @@ -132,6 +137,14 @@ function fuzzValue(r: () => number, depth = 0): unknown { "analyte", "value", "label", + "source", + "domain", + "window", + "granularity", + "stats", + "readings", + "total", + "periodsWithReadings", ]; const obj: Record = {}; for (let i = 0; i < 1 + Math.floor(r() * 4); i += 1) { @@ -155,6 +168,8 @@ function fuzzArgs(r: () => number): string { } if (r() < 0.3) args.analyte = PLANTED[0]; if (r() < 0.1) args.period = r() < 0.5 ? "previous" : PLANTED[2]; + if (r() < 0.2) args.granularity = r() < 0.7 ? "week" : PLANTED[2]; + if (r() < 0.3) args.ref = r() < 0.7 ? "m2.r1" : PLANTED[0]; return JSON.stringify(args); } @@ -162,10 +177,38 @@ function fuzzResult(r: () => number): CoachToolResult | undefined { const roll = r(); if (roll < 0.15) return undefined; if (roll < 0.55) { + const ref = ["r1", "r6", "r7", "m1.r1", PLANTED[0]][Math.floor(r() * 5)]; return { present: true, data: fuzzValue(r), ...(r() < 0.3 ? { grounding: PLANTED[1] } : {}), + ...(r() < 0.4 + ? { + resultRef: r() < 0.8 ? ref : "r2", + table: { + ref, + source: { + tool: "get_metric_table", + domain: (r() < 0.7 + ? DOMAINS[Math.floor(r() * DOMAINS.length)] + : PLANTED[0]) as CoachStepDomain, + window: WINDOWS[Math.floor(r() * WINDOWS.length)], + period: "current", + granularity: "day", + }, + shape: "timeSeries", + titleKey: "coach.result.title.byDay", + title: PLANTED[0], + rowCount: 1, + chartKind: null, + displayed: false, + columns: [], + rows: [[PLANTED[0], 128.4]], + truncated: false, + chart: null, + }, + } + : {}), }; } const reasons = [ @@ -179,6 +222,10 @@ function fuzzResult(r: () => number): CoachToolResult | undefined { "analyte_not_found", "no_significant_pattern", "module_disabled", + "unknown_result", + "result_unavailable", + "result_limit", + "use_get_workouts", PLANTED[1], "__proto__", "toString", @@ -227,6 +274,9 @@ function assertClean(step: CoachStep, locale: Locale) { expect(step.count).toBeGreaterThanOrEqual(0); } if (step.reason !== undefined) expect(REASONS.has(step.reason)).toBe(true); + if (step.resultRef !== undefined) { + expect(step.resultRef).toMatch(/^r[1-6]$/); + } const wire = JSON.stringify(step); for (const planted of PLANTED) expect(wire).not.toContain(planted); expect(wire.toLowerCase()).not.toContain("zqx"); @@ -237,7 +287,7 @@ describe("toStep — property: only catalog keys, domains, windows and integers" const r = rng(0x5eed); let produced = 0; for (let i = 0; i < 4000; i += 1) { - const name = COACH_TOOL_NAMES[i % COACH_TOOL_NAMES.length]; + const name = STEP_TOOLS[i % STEP_TOOLS.length]; const locale = locales[Math.floor(r() * locales.length)]; const call = { id: `c${i}`, name, arguments: fuzzArgs(r) }; const step = toStep({ @@ -259,8 +309,15 @@ describe("toStep — property: only catalog keys, domains, windows and integers" }); it("covers every tool with a step", () => { - for (const name of COACH_TOOL_NAMES) { - const args = name === "get_metric_series" ? '{"metric":"bp"}' : "{}"; + // The smallest valid arguments each tool takes: the metric tools need a + // metric, show_result a table name. + const MINIMAL_ARGS: Partial> = { + get_metric_series: '{"metric":"bp"}', + get_metric_table: '{"metric":"bp"}', + show_result: '{"ref":"m1.r1"}', + }; + for (const name of STEP_TOOLS) { + const args = MINIMAL_ARGS[name] ?? "{}"; const step = toStep({ call: { id: "x", name, arguments: args }, index: 0, @@ -426,6 +483,213 @@ describe("toStep — mapping", () => { }); }); +describe("toStep — tables", () => { + function bpWeekTable(ref: string): CoachResultTable { + return { + ref, + source: { + tool: "get_metric_table", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + }, + shape: "timeSeries", + titleKey: "coach.result.title.byWeek", + title: "Blood pressure by week", + rowCount: 3, + chartKind: null, + displayed: false, + columns: [ + { key: "week", kind: "period", labelKey: "k", label: "Week" }, + { key: "systolic", kind: "number", labelKey: "k", label: "Sys" }, + { key: "readings", kind: "count", labelKey: "k", label: "Readings" }, + ], + rows: [ + ["2026-W27", 128.4, 9], + ["2026-W28", null, null], + ["2026-W29", 131, 12], + ], + truncated: false, + chart: null, + }; + } + /** The result the executor hands the settled callback: summary + table. */ + function tableResult(table: CoachResultTable): CoachToolResult { + return { + present: true, + resultRef: table.ref, + data: summariseTable(table), + table, + }; + } + + const tableCall = { + id: "t", + name: "get_metric_table", + arguments: '{"metric":"bp","window":"last90days"}', + }; + const tableArgs = parseCoachToolArgs(tableCall.name, tableCall.arguments); + + it("a metric table is labelled by its metric and window while it runs", () => { + expect( + toStep({ + call: tableCall, + index: 0, + parsedArgs: tableArgs, + locale: "en", + }), + ).toEqual({ + id: "s1", + tool: "get_metric_table", + labelKey: "coach.step.readWindow", + label: "Checking: Blood pressure, last 90 days", + domain: "bp", + window: "last90days", + status: "running", + }); + }); + + it("a settled metric table counts its readings, takes its granularity and names its table", () => { + expect( + toStep({ + call: tableCall, + index: 1, + parsedArgs: tableArgs, + locale: "en", + result: tableResult(bpWeekTable("r2")), + }), + ).toMatchObject({ + id: "s2", + tool: "get_metric_table", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + status: "done", + count: 21, + resultRef: "r2", + }); + }); + + it("past the sixth table the read still counts, with no table to name", () => { + const { + table: _table, + resultRef: _ref, + ...plain + } = tableResult(bpWeekTable("r1")); + const step = toStep({ + call: tableCall, + index: 0, + parsedArgs: tableArgs, + locale: "en", + result: plain, + }); + expect(step).toMatchObject({ status: "done", count: 21 }); + expect(step).not.toHaveProperty("resultRef"); + }); + + it("a projected table of an older tool names its table too", () => { + const table = { + ...bpWeekTable("r3"), + source: { + ...bpWeekTable("r3").source, + tool: "get_workouts" as const, + domain: "workouts" as const, + }, + }; + const step = toStep({ + call: { id: "w", name: "get_workouts", arguments: "{}" }, + index: 0, + parsedArgs: {}, + locale: "en", + result: { + present: true, + data: { totalInWindow: 4 }, + resultRef: "r3", + table, + }, + }); + expect(step).toMatchObject({ count: 4, resultRef: "r3" }); + }); + + const showCall = { + id: "r", + name: "show_result", + arguments: '{"ref":"m2.r1"}', + }; + const showArgs = parseCoachToolArgs(showCall.name, showCall.arguments); + + it("show_result runs without a domain: the name alone says nothing about one", () => { + expect( + toStep({ call: showCall, index: 0, parsedArgs: showArgs, locale: "en" }), + ).toEqual({ + id: "s1", + tool: "show_result", + labelKey: "coach.step.showResult", + label: "Opening an earlier table", + status: "running", + }); + }); + + it("a shown table takes the source of the read that built it, and its new name", () => { + const copy = { + ...bpWeekTable("r1"), + reusedFrom: { messageId: "msg-1", ref: "r1" }, + }; + const step = toStep({ + call: showCall, + index: 0, + parsedArgs: showArgs, + locale: "de", + result: { + ...tableResult(copy), + data: { shownAgain: "m2.r1", ...summariseTable(copy) }, + }, + }); + expect(step).toEqual({ + id: "s1", + tool: "show_result", + labelKey: "coach.step.showResult", + label: "Öffne eine frühere Tabelle", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + status: "done", + count: 21, + resultRef: "r1", + }); + }); + + it("a table that could not be shown is a failed step, never an empty read", () => { + const at = (reason: string) => + toStep({ + call: showCall, + index: 0, + parsedArgs: showArgs, + locale: "en", + result: { present: false, reason }, + }); + expect(at("unknown_result")).toMatchObject({ + status: "failed", + reason: "invalid_arguments", + }); + expect(at("result_unavailable")).toMatchObject({ + status: "failed", + reason: "retrieval_failed", + }); + expect(at("module_disabled")).toMatchObject({ + status: "failed", + reason: "module_disabled", + }); + const limit = at("result_limit"); + expect(limit).toMatchObject({ status: "failed" }); + expect(limit).not.toHaveProperty("reason"); + expect(limit).not.toHaveProperty("domain"); + }); +}); + describe("snapshotStep", () => { it("is one settled step counting the metrics", () => { expect(snapshotStep({ metricCount: 5, locale: "en" })).toEqual({ diff --git a/src/lib/ai/coach/turn/steps.ts b/src/lib/ai/coach/turn/steps.ts index aff8ba98c..9fcad1190 100644 --- a/src/lib/ai/coach/turn/steps.ts +++ b/src/lib/ai/coach/turn/steps.ts @@ -31,9 +31,11 @@ import { } from "@/lib/ai/coach/dialog-keys"; import type { CoachToolResult } from "@/lib/ai/coach/tools/executor"; import { + SHOW_RESULT_TOOL_NAME, isCoachToolName, type CoachToolName, } from "@/lib/ai/coach/tools/definitions"; +import { isTurnResultRef } from "@/lib/ai/coach/results/refs"; /** At most this many steps per turn; later calls run but show no step. */ export const MAX_TURN_STEPS = 12; @@ -42,6 +44,14 @@ const SCOPE_SOURCES: ReadonlySet = new Set( coachScopeSourceSchema.options, ); const WINDOWS: ReadonlySet = new Set(coachScopeWindowSchema.options); +/** Every domain a result table can name as its source. */ +const RESULT_DOMAINS: ReadonlySet = new Set([ + ...coachScopeSourceSchema.options, + "labs", + "illness", + "cycle", + "correlations", +]); const PERIODS: ReadonlySet = new Set([ "current", "previous", @@ -68,6 +78,7 @@ const TOOL_DOMAIN: Readonly> = { get_workouts: "workouts", get_cycle: "cycle", get_correlations: "correlations", + get_metric_table: null, }; /** @@ -88,6 +99,7 @@ const TOOL_WINDOW: Readonly< get_workouts: "fallback", get_cycle: null, get_correlations: null, + get_metric_table: "fallback", }; /** @@ -113,6 +125,12 @@ const MISS: Readonly< status: "failed", reason: "invalid_arguments", }, + use_get_workouts: { status: "failed", reason: "invalid_arguments" }, + // `show_result`: a name the conversation does not hold, a stored table + // that could not be read, a turn already holding six tables. + unknown_result: { status: "failed", reason: "invalid_arguments" }, + result_unavailable: { status: "failed", reason: "retrieval_failed" }, + result_limit: { status: "failed" }, }; function isRecord(value: unknown): value is Record { @@ -135,10 +153,25 @@ function pick( : undefined; } +/** + * The readings a table summary counted: the total of its readings column, + * or, for a table without one, the periods that hold a reading. Both are + * figures `summariseTable` counted on the server. + */ +function tableSummaryCount(data: Record): number | undefined { + const stats = data.stats; + const readings = isRecord(stats) ? stats.readings : undefined; + return ( + (isRecord(readings) ? asCount(readings.total) : undefined) ?? + asCount(data.periodsWithReadings) + ); +} + /** * How many readings or rows a present result covered, when the result says * so in a field the server counted: * - a metric series: the aggregate's coverage count; + * - a metric table: the readings its periods fold; * - workouts: the sessions in the window; * - labs: the biomarkers returned (one row each). * Anything else carries no count rather than a guess. @@ -146,6 +179,8 @@ function pick( function presentCount(tool: CoachToolName, data: unknown): number | undefined { if (!isRecord(data)) return undefined; switch (tool) { + case "get_metric_table": + return tableSummaryCount(data); case "get_metric_series": { const section = data.section; if (!isRecord(section) || !isRecord(section.aggregate)) return undefined; @@ -202,6 +237,9 @@ export function toStep(args: { if (!Number.isSafeInteger(index) || index < 0 || index >= MAX_TURN_STEPS) { return null; } + if (call.name === SHOW_RESULT_TOOL_NAME) { + return reuseStep({ index, result, locale }); + } if (!isCoachToolName(call.name)) return null; const tool = call.name; @@ -213,22 +251,28 @@ export function toStep(args: { // There is nothing true to show, so it gets no step. if (!domain) return null; + // A metric table says what it read (the window, and the period and + // granularity after the executor's defaults); before it settles, the + // validated arguments say what it is about to read. + const source = tool === "get_metric_table" ? tableSource(result) : undefined; const windowRule = TOOL_WINDOW[tool]; const window = windowRule === null ? undefined : windowRule === "fallback" - ? (pick(WINDOWS, parsedArgs?.window) ?? + ? (source?.window ?? + pick(WINDOWS, parsedArgs?.window) ?? pick(WINDOWS, result?.searchedWindow) ?? pick(WINDOWS, args.fallbackWindow)) : windowRule; - const period = pick(PERIODS, parsedArgs?.period); - const granularity = pick( - GRANULARITIES, - parsedArgs?.granularity, - ); + const period = + source?.period ?? pick(PERIODS, parsedArgs?.period); + const granularity = + source?.granularity ?? + pick(GRANULARITIES, parsedArgs?.granularity); const settled = settle(tool, result); + const resultRef = producedRef(result); return { id: `s${index + 1}`, tool, @@ -240,9 +284,106 @@ export function toStep(args: { status: settled.status, ...(settled.count !== undefined ? { count: settled.count } : {}), ...(settled.reason ? { reason: settled.reason } : {}), + ...(resultRef ? { resultRef } : {}), }; } +/** The table a settled call produced for this turn, by its `r` name. */ +function producedRef(result: CoachToolResult | undefined): string | undefined { + const ref = result?.present ? result.table?.ref : undefined; + return ref !== undefined && ref === result?.resultRef && isTurnResultRef(ref) + ? ref + : undefined; +} + +/** + * What a present table result read, each field checked against its enum: + * the table's own source, else the source its model summary repeats. + */ +function tableSource(result: CoachToolResult | undefined): + | { + domain?: CoachStepDomain; + window?: CoachScopeWindow; + period?: CoachResultPeriod; + granularity?: CoachResultGranularity; + } + | undefined { + if (!result?.present) return undefined; + const raw: unknown = + result.table?.source ?? + (isRecord(result.data) ? result.data.source : undefined); + if (!isRecord(raw)) return undefined; + return { + domain: pick(RESULT_DOMAINS, raw.domain), + window: pick(WINDOWS, raw.window), + period: pick(PERIODS, raw.period), + granularity: pick(GRANULARITIES, raw.granularity), + }; +} + +/** + * A `show_result` call: an earlier table of the conversation shown again, + * with no new read. Before it settles the step names no domain (the name + * alone says nothing true about one); once the stored table is found, the + * step takes the domain, window, period and granularity of the read that + * built it, the readings that table counted, and the name the copy carries + * in this turn. + */ +function reuseStep(args: { + index: number; + result: CoachToolResult | undefined; + locale: Locale; +}): CoachStep { + const { index, result, locale } = args; + const { t } = getServerTranslator(locale); + const labelKey = COACH_STEP_LABEL_KEYS.showResult; + const base = { + id: `s${index + 1}`, + tool: SHOW_RESULT_TOOL_NAME, + labelKey, + label: t(labelKey), + } as const; + if (!result) return { ...base, status: "running" }; + const source = tableSource(result); + const resultRef = producedRef(result); + if (!source?.domain || !resultRef) { + // Nothing was shown. A reuse has no "no readings" outcome: the table + // either came back or it did not. + const miss = result.present ? undefined : missFor(result.reason); + return { + ...base, + status: "failed", + ...(miss?.reason ? { reason: miss.reason } : {}), + }; + } + const count = isRecord(result.data) + ? tableSummaryCount(result.data) + : undefined; + return { + ...base, + domain: source.domain, + ...(source.window ? { window: source.window } : {}), + ...(source.period ? { period: source.period } : {}), + ...(source.granularity ? { granularity: source.granularity } : {}), + status: "done", + ...(count !== undefined ? { count } : {}), + resultRef, + }; +} + +function missFor(reason: unknown): { + status: CoachStepStatus; + reason?: CoachStepReason; +} { + return ( + (typeof reason === "string" && + Object.hasOwn(MISS, reason) && + MISS[reason]) || { + status: "empty", + } + ); +} + function settle( tool: CoachToolName, result: CoachToolResult | undefined, @@ -251,9 +392,7 @@ function settle( if (result.present) { return { status: "done", count: presentCount(tool, result.data) }; } - const miss = (typeof result.reason === "string" && - Object.hasOwn(MISS, result.reason) && - MISS[result.reason]) || { status: "empty" as const }; + const miss = missFor(result.reason); // An out-of-window miss knows how much the record holds elsewhere; the // count rides the step for the method line, the row shows the reason. const count =