From b40de3a4d9b09f28b32f2450d03acce9339d1099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 27 Sep 2026 12:11:55 +0200 Subject: [PATCH] fix(coach): count a table shown again once, and say it is from before The method line merged a repeated read by keeping the larger count. With show_result that could let an earlier answer's count stand over the read this turn just made. A fresh read of the same source, window and period now always wins, whichever order the steps ran in, and the two are never added. An entry that rests only on an earlier table says so ("from an earlier answer"), including a reused table no step accounts for. The new catalog string is in all seven locales. --- messages/de.json | 1 + messages/en.json | 1 + messages/es.json | 1 + messages/fr.json | 1 + messages/it.json | 1 + messages/ko.json | 1 + messages/pl.json | 1 + src/lib/ai/coach/__tests__/method.test.ts | 94 ++++++++++++++++++ src/lib/ai/coach/dialog-keys.ts | 1 + src/lib/ai/coach/method.ts | 116 +++++++++++++++------- 10 files changed, 181 insertions(+), 37 deletions(-) diff --git a/messages/de.json b/messages/de.json index c958a3007..c852210fe 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9602,6 +9602,7 @@ "label": "Wie das ermittelt wurde", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "aus einer früheren Antwort", "readingsOne": "1 Messwert", "readingsFew": "{count} Messwerte", "readingsOther": "{count} Messwerte", diff --git a/messages/en.json b/messages/en.json index aaf1bacfd..2854b82c8 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9602,6 +9602,7 @@ "label": "How this was worked out", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "from an earlier answer", "readingsOne": "1 reading", "readingsFew": "{count} readings", "readingsOther": "{count} readings", diff --git a/messages/es.json b/messages/es.json index 72afbd64a..e00c2b17f 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9602,6 +9602,7 @@ "label": "Cómo se obtuvo", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "de una respuesta anterior", "readingsOne": "1 medición", "readingsFew": "{count} mediciones", "readingsOther": "{count} mediciones", diff --git a/messages/fr.json b/messages/fr.json index e5f9712bc..f04e29e3f 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9602,6 +9602,7 @@ "label": "Comment cela a été calculé", "entry": "{domain}, {window} : {detail}", "entryNoWindow": "{domain} : {detail}", + "reused": "d'une réponse précédente", "readingsOne": "1 mesure", "readingsFew": "{count} mesures", "readingsOther": "{count} mesures", diff --git a/messages/it.json b/messages/it.json index 3d8b61622..475b53945 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9602,6 +9602,7 @@ "label": "Come è stato ricavato", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "da una risposta precedente", "readingsOne": "1 misurazione", "readingsFew": "{count} misurazioni", "readingsOther": "{count} misurazioni", diff --git a/messages/ko.json b/messages/ko.json index d64520a7a..a8baf3d87 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -9602,6 +9602,7 @@ "label": "산출 방법", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "이전 답변에서", "readingsOne": "측정값 1개", "readingsFew": "측정값 {count}개", "readingsOther": "측정값 {count}개", diff --git a/messages/pl.json b/messages/pl.json index c0513eea3..e9e5a9d6e 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9602,6 +9602,7 @@ "label": "Jak to ustalono", "entry": "{domain}, {window}: {detail}", "entryNoWindow": "{domain}: {detail}", + "reused": "z wcześniejszej odpowiedzi", "readingsOne": "1 pomiar", "readingsFew": "{count} pomiary", "readingsOther": "{count} pomiarów", diff --git a/src/lib/ai/coach/__tests__/method.test.ts b/src/lib/ai/coach/__tests__/method.test.ts index a70461557..765e75a71 100644 --- a/src/lib/ai/coach/__tests__/method.test.ts +++ b/src/lib/ai/coach/__tests__/method.test.ts @@ -210,6 +210,100 @@ describe("buildMethod", () => { ]); }); + describe("a table shown again (show_result)", () => { + const reusedStep = (id: string, ref: string, count: number) => + step({ + id, + tool: "show_result", + labelKey: "coach.step.showResult", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + count, + resultRef: ref, + }); + const reusedTable = (ref: string) => + table(ref, "bp", { + source: { + tool: "get_metric_table", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + }, + reusedFrom: { messageId: "msg-1", ref: "r1" }, + }); + + it("says the figures come from an earlier answer", () => { + const method = buildMethod({ + steps: [reusedStep("s1", "r1", 142)], + results: [reusedTable("r1")], + locale: "en", + }); + expect(method?.text).toBe( + "Blood pressure, last 90 days: 142 readings, weekly averages, from an earlier answer", + ); + expect(method?.entries).toEqual([ + { + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + count: 142, + aggregation: "mean", + }, + ]); + }); + + it("a re-read of the same source is one entry with the fresh count, never a sum and never marked as earlier", () => { + for (const order of ["reuse-first", "read-first"] as const) { + const read = step({ + id: "s2", + tool: "get_metric_table", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + count: 150, + resultRef: "r2", + }); + const reuse = reusedStep("s1", "r1", 142); + const method = buildMethod({ + steps: order === "reuse-first" ? [reuse, read] : [read, reuse], + results: [ + reusedTable("r1"), + table("r2", "bp", { + source: { + tool: "get_metric_table", + domain: "bp", + window: "last90days", + period: "current", + granularity: "week", + }, + }), + ], + locale: "en", + }); + expect(method?.text, order).toBe( + "Blood pressure, last 90 days: 150 readings, weekly averages", + ); + expect(method?.entries, order).toHaveLength(1); + } + }); + + it("a reused table no step accounts for is marked as well", () => { + const method = buildMethod({ + steps: [], + results: [reusedTable("r1")], + locale: "de", + }); + expect(method?.text).toBe( + "Blutdruck, letzte 90 Tage: Wochendurchschnitte, aus einer früheren Antwort", + ); + }); + }); + it("describes a table no step accounts for", () => { const method = buildMethod({ steps: [], diff --git a/src/lib/ai/coach/dialog-keys.ts b/src/lib/ai/coach/dialog-keys.ts index a04dec646..6c4860441 100644 --- a/src/lib/ai/coach/dialog-keys.ts +++ b/src/lib/ai/coach/dialog-keys.ts @@ -180,6 +180,7 @@ export const COACH_METHOD_KEYS = { label: "coach.method.label", entry: "coach.method.entry", entryNoWindow: "coach.method.entryNoWindow", + reused: "coach.method.reused", } as const; /** "N readings" in the method line, by plural tier. */ diff --git a/src/lib/ai/coach/method.ts b/src/lib/ai/coach/method.ts index a6d685da0..f2ef3d41b 100644 --- a/src/lib/ai/coach/method.ts +++ b/src/lib/ai/coach/method.ts @@ -81,40 +81,66 @@ function entryKey(e: CoachMethodEntry): string { ); } -/** The entries, in the order the turn read them, one per read. */ -export function methodEntries( +/** + * The entries, in the order the turn read them, one per read, and which of + * them rest only on a table an earlier answer read (`show_result`): those + * figures were not read again this turn, and the line says so. + */ +function collectEntries( steps: readonly CoachStep[], results: readonly CoachResultMeta[], -): CoachMethodEntry[] { +): { entries: CoachMethodEntry[]; reused: ReadonlySet } { const byRef = new Map(results.map((r) => [r.ref, r])); const usedRefs = new Set(); const entries = new Map(); + const reusedKeys = new Set(); - const add = (entry: CoachMethodEntry) => { + const add = (entry: CoachMethodEntry, reuse = false) => { const key = entryKey(entry); const existing = entries.get(key); if (!existing) { entries.set(key, entry); + if (reuse) reusedKeys.add(key); return; } - // The same read twice (a retry, a show_result of this turn's table): - // one entry, the fullest one. A found read outranks an absent one. - if (existing.absent && !entry.absent) { - entries.set(key, entry); - return; - } - if (!entry.absent) { + if (entry.absent) return; + // A read of this turn and an earlier table of the same source, window + // and period: one entry. The fresh read is what the answer rests on, + // so its count stands and the earlier one is never added to it. + const existingReused = reusedKeys.has(key); + if (existingReused && !reuse) { + reusedKeys.delete(key); entries.set(key, { - ...existing, - ...(entry.count !== undefined && - (existing.count === undefined || entry.count > existing.count) - ? { count: entry.count } - : {}), - ...(existing.aggregation === undefined && entry.aggregation - ? { aggregation: entry.aggregation } + ...entry, + ...(entry.aggregation === undefined && existing.aggregation + ? { aggregation: existing.aggregation } : {}), }); + return; } + if (reuse && !existingReused && !existing.absent) { + if (existing.aggregation === undefined && entry.aggregation) { + entries.set(key, { ...existing, aggregation: entry.aggregation }); + } + return; + } + // The same read twice (a retry): one entry, the fullest one. A found + // read outranks an absent one. + if (existing.absent) { + entries.set(key, entry); + if (reuse) reusedKeys.add(key); + return; + } + entries.set(key, { + ...existing, + ...(entry.count !== undefined && + (existing.count === undefined || entry.count > existing.count) + ? { count: entry.count } + : {}), + ...(existing.aggregation === undefined && entry.aggregation + ? { aggregation: entry.aggregation } + : {}), + }); }; for (const step of steps) { @@ -140,35 +166,47 @@ export function methodEntries( // not the same as nothing being there. The step list says which. if (step.status !== "done") continue; const aggregation = result ? aggregationFor(result) : undefined; - add({ - ...base, - ...(typeof step.count === "number" && step.count >= 0 - ? { count: Math.floor(step.count) } - : {}), - ...(aggregation ? { aggregation } : {}), - }); + add( + { + ...base, + ...(typeof step.count === "number" && step.count >= 0 + ? { count: Math.floor(step.count) } + : {}), + ...(aggregation ? { aggregation } : {}), + }, + step.tool === "show_result", + ); } // A table no step accounts for still says how it was built. for (const result of results) { if (usedRefs.has(result.ref)) continue; const aggregation = aggregationFor(result); - add({ - domain: result.source.domain, - window: result.source.window, - period: result.source.period, - ...(result.source.granularity - ? { granularity: result.source.granularity } - : {}), - ...(aggregation ? { aggregation } : {}), - }); + add( + { + domain: result.source.domain, + window: result.source.window, + period: result.source.period, + ...(result.source.granularity + ? { granularity: result.source.granularity } + : {}), + ...(aggregation ? { aggregation } : {}), + }, + result.reusedFrom !== undefined, + ); } - return [...entries.values()]; + const reused = new Set(); + for (const key of reusedKeys) { + const entry = entries.get(key); + if (entry) reused.add(entry); + } + return { entries: [...entries.values()], reused }; } function renderEntry( entry: CoachMethodEntry, + reused: boolean, t: (key: string, params?: Record) => string, locale: Locale, ): string | null { @@ -188,6 +226,9 @@ function renderEntry( } else if (entry.aggregation) { details.push(t(COACH_METHOD_AGGREGATION_KEYS[entry.aggregation])); } + if (reused && details.length > 0) { + details.push(t(COACH_METHOD_KEYS.reused)); + } } // Nothing to say about how it was worked out: the step list already // names the source. @@ -214,8 +255,9 @@ export function buildMethod(args: { const { locale } = args; const { t } = getServerTranslator(locale); const rendered: Array<{ entry: CoachMethodEntry; text: string }> = []; - for (const entry of methodEntries(args.steps, args.results)) { - const text = renderEntry(entry, t, locale); + const { entries, reused } = collectEntries(args.steps, args.results); + for (const entry of entries) { + const text = renderEntry(entry, reused.has(entry), t, locale); if (text) rendered.push({ entry, text }); if (rendered.length === METHOD_MAX_ENTRIES) break; }