From bd3aa3ddb174226f831f91553fa4bc758a816d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 27 Sep 2026 13:18:26 +0200 Subject: [PATCH] fix(coach): hold an excluded metric's earlier tables back from the model show_result now passes the same scope gate a fresh read does before it decrypts a stored table: a metric the person has since excluded from the Coach, or one outside the conversation's scope, answers unavailable_in_scope and nothing is sent. The list of earlier tables in the context drops those metrics too, so the model is not told they exist. The exclusion (the person's own list plus switched-off modules, and no adherence when medications are excluded) is the snapshot builder's, now one exported helper both paths use. --- .../__tests__/executor-results.test.ts | 88 ++++++++++++++++++- src/lib/ai/coach/snapshot.ts | 80 ++++++++++------- src/lib/ai/coach/tools/executor.ts | 72 ++++++++++++++- src/lib/ai/coach/tools/index.ts | 1 + src/lib/ai/coach/turn/model.ts | 14 ++- 5 files changed, 218 insertions(+), 37 deletions(-) diff --git a/src/lib/ai/coach/results/__tests__/executor-results.test.ts b/src/lib/ai/coach/results/__tests__/executor-results.test.ts index 93e0f742f..7890a7c97 100644 --- a/src/lib/ai/coach/results/__tests__/executor-results.test.ts +++ b/src/lib/ai/coach/results/__tests__/executor-results.test.ts @@ -11,7 +11,8 @@ import type { CoachResultTable } from "@/lib/ai/coach/types"; const buildCoachSnapshot = vi.fn<(userId: string, scope?: unknown) => Promise>(); -vi.mock("@/lib/ai/coach/snapshot", () => ({ +vi.mock("@/lib/ai/coach/snapshot", async (importOriginal) => ({ + ...(await importOriginal()), buildCoachSnapshot: (userId: string, scope?: unknown) => buildCoachSnapshot(userId, scope), })); @@ -43,6 +44,7 @@ vi.mock("@/lib/modules/gate", () => ({ })); import { + admittedPriorResults, executeCoachTool, type CoachToolTurnContext, } from "@/lib/ai/coach/tools/executor"; @@ -311,6 +313,28 @@ describe("get_metric_table", () => { }); describe("show_result", () => { + it("sends nothing of a metric the person has since excluded", async () => { + // The snapshot gate no longer admits pulse: the stored table is not + // even decrypted. + buildCoachSnapshot.mockResolvedValue(snapshot({ scope: { sources: [] } })); + readMessageResults.mockResolvedValue([STORED]); + const result = await executeCoachTool({ + userId: "u1", + name: "show_result", + rawArguments: JSON.stringify({ ref: "m2.r1" }), + turn: turn(), + }); + expect(result).toEqual({ + present: false, + reason: "unavailable_in_scope", + }); + expect(buildCoachSnapshot).toHaveBeenCalledWith("u1", { + sources: ["pulse"], + window: "last7days", + }); + expect(readMessageResults).not.toHaveBeenCalled(); + }); + it("copies a stored table of this conversation under a new name", async () => { readMessageResults.mockResolvedValue([STORED]); const result = await executeCoachTool({ @@ -497,3 +521,65 @@ describe("projections of the older tools", () => { expect((result.data as { totalInWindow: number }).totalInWindow).toBe(6); }); }); + +describe("admittedPriorResults", () => { + const meta = (ref: string, domain: string) => ({ + ...turn().priorResults[0].results[0], + ref, + source: { ...STORED.source, domain } as CoachResultTable["source"], + }); + const prior = [ + { + messageId: "m-a2", + turnIndex: 2, + results: [meta("r1", "pulse"), meta("r2", "steps"), meta("r3", "labs")], + }, + { messageId: "m-a4", turnIndex: 4, results: [meta("r1", "steps")] }, + ]; + + it("drops a metric the person excluded and keeps what no exclusion names", async () => { + const out = await admittedPriorResults({ + userId: "u1", + prefs: { excludeMetrics: ["steps"] }, + scope: undefined, + prior, + }); + expect( + out.map((t) => [t.turnIndex, t.results.map((r) => r.source.domain)]), + ).toEqual([[2, ["pulse", "labs"]]]); + }); + + it("keeps to the conversation's scope", async () => { + const out = await admittedPriorResults({ + userId: "u1", + prefs: { excludeMetrics: [] }, + scope: { sources: ["steps"] }, + prior, + }); + expect( + out.map((t) => [t.turnIndex, t.results.map((r) => r.source.domain)]), + ).toEqual([ + [2, ["steps", "labs"]], + [4, ["steps"]], + ]); + }); + + it("drops a metric whose module is switched off", async () => { + resolveModuleMap.mockResolvedValue({ mood: false }); + const out = await admittedPriorResults({ + userId: "u1", + prefs: { excludeMetrics: [] }, + scope: undefined, + prior: [ + { + messageId: "m-a2", + turnIndex: 2, + results: [meta("r1", "mood"), meta("r2", "pulse")], + }, + ], + }); + expect(out.flatMap((t) => t.results.map((r) => r.source.domain))).toEqual([ + "pulse", + ]); + }); +}); diff --git a/src/lib/ai/coach/snapshot.ts b/src/lib/ai/coach/snapshot.ts index 824f1ea87..51d046718 100644 --- a/src/lib/ai/coach/snapshot.ts +++ b/src/lib/ai/coach/snapshot.ts @@ -21,6 +21,7 @@ import { parseCoachPrefs, type CoachExcludeMetric, type CoachDataCluster, + type CoachPrefs, } from "@/lib/validations/coach-prefs"; import { DEFAULT_TIMEZONE } from "@/lib/tz/resolver"; import { locales, defaultLocale, type Locale } from "@/lib/i18n/config"; @@ -298,6 +299,51 @@ function blockFailed(block: string): (err: unknown) => null { }; } +/** + * What the Coach may not read for this person: their own `excludeMetrics` + * plus every source a switched-off module owns (`moduleMap[key] === false`; + * the gate has already resolved every delegation, so the map is + * authoritative). + */ +export function coachExclusions( + prefs: Pick, + moduleMap: Readonly>, +): ReadonlySet { + const excluded = new Set(prefs.excludeMetrics); + for (const [key, srcs] of Object.entries(MODULE_EXCLUDED_SOURCES)) { + if (moduleMap[key as ModuleKey] === false) { + for (const src of srcs ?? []) { + // Every entry in MODULE_EXCLUDED_SOURCES is a CoachScopeSource that + // also exists in the CoachExcludeMetric enum overlap the + // source-narrowing check reads. + excluded.add(src as unknown as CoachExcludeMetric); + } + } + } + return excluded; +} + +/** + * The sources of a scope the Coach may read: the scope minus the + * exclusions, and no medication compliance when medications are excluded + * (excluding medications means no medication data at all). The one gate + * every read passes, the snapshot's own and the table tools' alike. + */ +export function admitCoachSources( + scoped: Iterable, + excluded: ReadonlySet, +): Set { + const sources = new Set(); + for (const src of scoped) { + // The `excludeMetrics` enum is a superset of `CoachScopeSource` + // (medications + anthropometrics live on the exclude-only side); the + // runtime `has` check only catches the overlapping members. + if (!excluded.has(src as unknown as CoachExcludeMetric)) sources.add(src); + } + if (excluded.has("medications")) sources.delete("compliance"); + return sources; +} + async function buildCoachSnapshotImpl( userId: string, scope?: CoachScope, @@ -380,42 +426,14 @@ async function buildCoachSnapshotImpl( // before any row is read — the model never sees a disabled domain. const moduleMap = await moduleMapPromise; const recoveryDisabled = moduleMap.recovery === false; - const excluded = new Set(prefs.excludeMetrics); - for (const [key, srcs] of Object.entries(MODULE_EXCLUDED_SOURCES)) { - if (moduleMap[key as ModuleKey] === false) { - for (const src of srcs ?? []) { - // Every entry in MODULE_EXCLUDED_SOURCES is a CoachScopeSource that - // also exists in the CoachExcludeMetric enum overlap the - // source-narrowing loop checks against; the cast mirrors the one the - // loop already uses below. - excluded.add(src as unknown as CoachExcludeMetric); - } - } - } + const excluded = coachExclusions(prefs, moduleMap); // v1.4.36 W3 T2 — `medications` and `anthropometrics` are // exclude-only toggles (not in `CoachScopeSource`); they gate the // GLP-1 weeklyContext / compliance branch and the anthropometrics - // block respectively. The mapping below treats them as additive to - // the existing source-level exclusions. + // block respectively. const excludesMedications = excluded.has("medications"); const excludesAnthropometrics = excluded.has("anthropometrics"); - const sources = new Set(); - for (const src of scopedSources) { - // The `excludeMetrics` enum is a superset of `CoachScopeSource` now - // (medications + anthropometrics live on the exclude-only side); - // the cast was safe before v1.4.36 because the enums matched 1:1, - // and the runtime `excluded.has` check still only catches the - // overlapping members. - if (!excluded.has(src as unknown as CoachExcludeMetric)) { - sources.add(src); - } - } - // `medications` exclusion also drops the compliance source so the - // intake-event branch below short-circuits — keeps the contract - // consistent (excluding medications == no medication data at all). - if (excludesMedications) { - sources.delete("compliance"); - } + const sources = admitCoachSources(scopedSources, excluded); const windowDays = windowToDays(window); // v1.11.3 — kick the feature extraction off as a promise now and await diff --git a/src/lib/ai/coach/tools/executor.ts b/src/lib/ai/coach/tools/executor.ts index f1bc29818..3e85d7192 100644 --- a/src/lib/ai/coach/tools/executor.ts +++ b/src/lib/ai/coach/tools/executor.ts @@ -45,14 +45,21 @@ import { annotate } from "@/lib/logging/context"; import type { Locale } from "@/lib/i18n/config"; import { resolveUserTimezone } from "@/lib/tz/resolver"; import { resolveModuleMap } from "@/lib/modules/gate"; -import { buildCoachSnapshot } from "@/lib/ai/coach/snapshot"; +import { + admitCoachSources, + buildCoachSnapshot, + coachExclusions, +} from "@/lib/ai/coach/snapshot"; +import type { CoachPrefs } from "@/lib/validations/coach-prefs"; import { readMessageResults } from "@/lib/ai/coach/persistence"; import type { CoachResultTable, CoachScope, CoachScopeSource, CoachScopeWindow, + CoachStepDomain, } from "@/lib/ai/coach/types"; +import { COACH_SOURCE_MEASUREMENT_TYPES } from "@/lib/ai/coach/source-measurement-types"; import { METRIC_TABLE_EXCLUDED_SOURCES, compareWithCurrent, @@ -264,7 +271,7 @@ export async function executeCoachTool(args: { try { const result = name === SHOW_RESULT_TOOL_NAME - ? await showResult(userId, parsedArgs, turn) + ? await showResult(userId, parsedArgs, sharedScope, turn) : await dispatch( name as CoachToolName, userId, @@ -742,7 +749,7 @@ async function getCorrelations( * rows the builder does not, so it asks the builder's gate first rather * than keeping a second copy of it. */ -function scopeAdmits( +export function scopeAdmits( sections: Record, metric: CoachScopeSource, ): boolean { @@ -750,6 +757,50 @@ function scopeAdmits( return Array.isArray(scope?.sources) && scope.sources.includes(metric); } +/** + * The earlier tables a turn may name and show again: those whose metric the + * same gate would read now. With a conversation scope that is the scope's + * sources less the person's exclusions and switched-off modules; without + * one, every metric not excluded. The exclusion is the snapshot's own + * (`coachExclusions`, `admitCoachSources`), so a metric the person excluded + * after a table was stored is neither listed for the model nor sent to it. + * Tables of other domains (labs) answer to their module when read. + */ +export async function admittedPriorResults(args: { + userId: string; + prefs: Pick; + scope: CoachScope | undefined; + prior: readonly PriorResultTurn[]; +}): Promise { + if (args.prior.length === 0) return []; + const excluded = coachExclusions( + args.prefs, + await resolveModuleMap(args.userId), + ); + const scoped = + args.scope?.sources && args.scope.sources.length > 0 + ? admitCoachSources(args.scope.sources, excluded) + : null; + const admits = (domain: CoachStepDomain) => + !isCoachScopeSource(domain) || + (scoped + ? scoped.has(domain) + : admitCoachSources([domain], excluded).size > 0); + return args.prior + .map((turn) => ({ + ...turn, + results: turn.results.filter((meta) => admits(meta.source.domain)), + })) + .filter((turn) => turn.results.length > 0); +} + +/** True for a step domain the snapshot scope gates (a metric, not labs). */ +export function isCoachScopeSource( + domain: CoachStepDomain, +): domain is CoachScopeSource { + return Object.hasOwn(COACH_SOURCE_MEASUREMENT_TYPES, domain); +} + async function getMetricTable( userId: string, rawArgs: unknown, @@ -878,6 +929,7 @@ function withResultView( async function showResult( userId: string, rawArgs: unknown, + sharedScope: CoachScope | undefined, turn: CoachToolTurnContext | undefined, ): Promise { const parsed = showResultArgsSchema.safeParse(rawArgs); @@ -893,6 +945,20 @@ async function showResult( return { present: false, reason: "unknown_result" }; } + // The same gate a fresh read passes, before anything is decrypted: a + // metric the person has since excluded from the Coach, or left out of + // this conversation's scope, is not sent to the model again either. + const domain = target.meta.source.domain; + if (isCoachScopeSource(domain)) { + const gate = await buildCoachSnapshot( + userId, + sharedScope ?? { sources: [domain], window: target.meta.source.window }, + ); + if (!scopeAdmits(gate.sections, domain)) { + return { present: false, reason: "unavailable_in_scope" }; + } + } + const modules = await resolveModuleMap(userId); const entries = await readMessageResults( userId, diff --git a/src/lib/ai/coach/tools/index.ts b/src/lib/ai/coach/tools/index.ts index acee46e47..bad2a2c37 100644 --- a/src/lib/ai/coach/tools/index.ts +++ b/src/lib/ai/coach/tools/index.ts @@ -8,6 +8,7 @@ */ export { COACH_TOOL_DEFS, parseCoachToolArgs } from "./definitions"; export type { CoachToolTrace } from "./executor"; +export { admittedPriorResults } from "./executor"; export { buildCoachDataInventory, renderDataInventory, diff --git a/src/lib/ai/coach/turn/model.ts b/src/lib/ai/coach/turn/model.ts index 166c77687..4f9a130c9 100644 --- a/src/lib/ai/coach/turn/model.ts +++ b/src/lib/ai/coach/turn/model.ts @@ -22,6 +22,7 @@ import { } from "@/lib/ai/coach/chat-request-builder"; import { COACH_TOOL_DEFS, + admittedPriorResults, buildCoachDataInventory, renderDataInventory, renderFocusHint, @@ -217,6 +218,15 @@ export async function runTurnModel(args: { // build reuses the snapshot we already computed (60s LRU), so the tools // that fire this turn share its reads. const inventory = await buildCoachDataInventory(userId, effectiveScope); + // Earlier tables whose metric the person has since excluded (or that + // fall outside this conversation's scope) are neither named for the + // model nor reachable through show_result. + const priorResults = await admittedPriorResults({ + userId, + prefs: ctx.coachPrefs, + scope: effectiveScope, + prior: args.priorResults ?? [], + }); const toolRequest = buildCoachToolRequest({ systemPrompt: ctx.systemPrompt, toolModeAddendum: appendBlocks(buildToolModeAddendum(locale), [ @@ -226,7 +236,7 @@ export async function runTurnModel(args: { focusHint: renderFocusHint(effectiveScope?.sources), workoutEvidence, dataInventory: renderDataInventory(inventory), - priorResults: renderPriorResultRefs(args.priorResults ?? []), + priorResults: renderPriorResultRefs(priorResults), guidedBlock: turnContext.guidedBlock, transcript: turnContext.transcript, languageName: LANGUAGE_NAMES[locale], @@ -259,7 +269,7 @@ export async function runTurnModel(args: { turn: { conversationId, locale, - priorResults: args.priorResults ?? [], + priorResults, refs: createResultRefAllocator(), }, // v1.39.4 — live steps: a `running` step as each call starts, its