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.
This commit is contained in:
Marc-André Bombeck committed 2026-09-27 12:11:55 +02:00
1 parent ad541cd5bb
commit b40de3a4d9
10 files changed
+181 -37

No files matched your search

+1
View File
@@ -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",
+1
View File
@@ -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",
+1
View File
@@ -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",
+1
View File
@@ -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",
+1
View File
@@ -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",
+1
View File
@@ -9602,6 +9602,7 @@
"label": "산출 방법",
"entry": "{domain}, {window}: {detail}",
"entryNoWindow": "{domain}: {detail}",
"reused": "이전 답변에서",
"readingsOne": "측정값 1개",
"readingsFew": "측정값 {count}개",
"readingsOther": "측정값 {count}개",
+1
View File
@@ -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",
+94
View File
@@ -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: [],
+1
View File
@@ -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. */
+79 -37
View File
@@ -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<CoachMethodEntry> } {
const byRef = new Map(results.map((r) => [r.ref, r]));
const usedRefs = new Set<string>();
const entries = new Map<string, CoachMethodEntry>();
const reusedKeys = new Set<string>();
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<CoachMethodEntry>();
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, string | number>) => 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;
}