fix(coach): stop offering a view chip the result panel already toggles

A table the answer displays with a chart renders its own chart/table
toggle, and the "Show as a chart" / "Show as a table" chips under it
repeated that control. The server no longer derives a view chip for such
a table; a table the answer only used, folded under "Data used", still
gets "Show as a chart", since that brings it into the answer. A reuse
turn shows its table with the toggle and so offers no view chip either.

The rule is one predicate in a leaf module, applied where chips are
derived and where a stored reply is read back, so a reply stored before
the change offers the same chips as a new one on every client.
This commit is contained in:
Marc-André Bombeck committed 2026-10-05 17:41:20 +02:00
1 parent 0c3de5bdfb
commit b0ac267af9
8 files changed
+158 -63

No files matched your search

@@ -212,17 +212,17 @@ describe("a reuse chip", () => {
messageId: h.LAST_ASSISTANT_ID,
ref: "r1",
});
const [chips] = framesOf<{ followUps: CoachFollowUp[] }>(
run.frames,
"followUps",
);
expect(chips.followUps.map((c) => c.kind)).toEqual(["as_chart"]);
// The result renders with its own chart/table toggle, so no chip
// offers the chart again.
expect(
framesOf<{ followUps: CoachFollowUp[] }>(run.frames, "followUps"),
).toEqual([]);
expect(m.appendMessage).toHaveBeenCalledWith(
expect.objectContaining({ role: "assistant", providerType: "reuse" }),
);
});
it("answers 'as a chart' with the stored chart and offers the table view", async () => {
it("answers 'as a chart' with the stored chart and leaves the table to its toggle", async () => {
conversationWithChips();
const run = await tap("f3");
expect(providerCalls).toHaveLength(0);
@@ -231,11 +231,9 @@ describe("a reuse chip", () => {
"result",
).map((f) => f.result);
expect(result.chart).toEqual(world.storedTables[0].chart);
const [chips] = framesOf<{ followUps: CoachFollowUp[] }>(
run.frames,
"followUps",
);
expect(chips.followUps.map((c) => c.kind)).toEqual(["as_table"]);
expect(
framesOf<{ followUps: CoachFollowUp[] }>(run.frames, "followUps"),
).toEqual([]);
});
it("still passes the rate limit", async () => {
@@ -201,6 +201,60 @@ describe("appendMessage — the dialog fields", () => {
});
});
it("reads a stored view chip back only where the answer's panel has no toggle for it", async () => {
// Stored before the rule: "as a table" for the shown chart (r1), whose
// panel already toggles, and "as a chart" for a table only used (r2).
const view = (id: string, kind: "as_chart" | "as_table", ref: string) => ({
...FULL.followUps![0],
id,
kind,
labelKey: `coach.followUp.${kind === "as_chart" ? "asChart" : "asTable"}`,
label: kind === "as_chart" ? "Show as a chart" : "Show as a table",
anchor: { ref, domain: "bp" as const, window: "last30days" as const },
reuse: true,
});
const used = { ...SLEEP_META, displayed: false };
stubEchoCreate(
JSON.stringify({
windows: [],
metrics: [],
results: [META, used],
followUps: [
view("f0", "as_table", "r1"),
FULL.followUps![0],
view("f3", "as_chart", "r2"),
],
}),
);
const out = await appendMessage({
conversationId: "c1",
role: "assistant",
content: "reply",
metricSource: { windows: [], metrics: [] },
});
expect(out.metricSource?.followUps?.map((c) => [c.id, c.kind])).toEqual([
["f1", "previous_period"],
["f3", "as_chart"],
]);
// Only the redundant chip stored: the list reads back absent, not empty.
stubEchoCreate(
JSON.stringify({
windows: [],
metrics: [],
results: [META],
followUps: [view("f1", "as_table", "r1")],
}),
);
const bare = await appendMessage({
conversationId: "c1",
role: "assistant",
content: "reply",
metricSource: { windows: [], metrics: [] },
});
expect(bare.metricSource).not.toHaveProperty("followUps");
});
it("strips a key the stored blob carries that the contract does not know", async () => {
stubEchoCreate(
JSON.stringify({
@@ -165,20 +165,26 @@ describe("deriveFollowUps — the rules", () => {
expect(all).toEqual([]);
});
it("offers the chart back for a table shown as a table", () => {
const chips = derive({
results: [
table("bp", { chartKind: "line", displayed: true, view: "table" }),
],
});
expect(chips[0]).toMatchObject({ kind: "as_chart", reuse: true });
it("offers no view chip for a table the answer shows with its chart", () => {
// The result panel renders its own chart/table toggle; a chip asking for
// either view would repeat it, whichever view the panel opens on.
for (const view of [undefined, "table" as const]) {
const chips = derive({
results: [
table("bp", {
chartKind: "line",
displayed: true,
...(view ? { view } : {}),
}),
],
});
expect(chips.some((c) => c.reuse)).toBe(false);
}
});
it("offers the other view of a table that has a chart, as a reuse chip", () => {
const shown = derive({
results: [table("bp", { chartKind: "line", displayed: true })],
});
expect(shown[0]).toMatchObject({ kind: "as_table", reuse: true });
it("offers the chart of a table the answer only used, as a reuse chip", () => {
// Folded under "Data used" as a table, its toggle is out of sight; the
// chip brings it into the answer.
const tucked = derive({
results: [table("bp", { chartKind: "line", displayed: false })],
});
@@ -404,6 +410,11 @@ describe("deriveFollowUps — property: every chip's domain was read this turn",
}
if (!chip.reuse) {
expect(held.has(domain!), `seed ${seed}`).toBe(true);
} else {
// A view chip never repeats the toggle of a table the answer
// shows with its chart.
const meta = results.find((m) => m.ref === chip.anchor?.ref);
expect(meta?.displayed && meta.chartKind, `seed ${seed}`).toBeFalsy();
}
}
}
+2 -6
View File
@@ -22,6 +22,8 @@ import {
} from "@/lib/ai/coach/dialog-keys";
import { METRIC_TABLE_EXCLUDED_SOURCES } from "@/lib/ai/coach/results/metric-table-tool";
import { REUSE_FOLLOW_UP_KINDS } from "./view-chip";
/** At most this many chips under one reply. */
export const MAX_FOLLOW_UPS = 3;
@@ -36,12 +38,6 @@ export const FOLLOW_UP_KINDS: readonly CoachFollowUpKind[] = [
"continue",
];
/** Kinds answered from a table the conversation already holds. */
export const REUSE_FOLLOW_UP_KINDS: ReadonlySet<CoachFollowUpKind> = new Set([
"as_chart",
"as_table",
]);
/**
* Kinds the model may propose in a `---FOLLOWUPS---` block. `continue` is
* the server's alone: it follows from the loop hitting its round cap, which
+13 -14
View File
@@ -6,8 +6,10 @@
*
* A chip is offered only when what it asks for is grounded in this turn:
*
* - `as_chart` / `as_table`: a table this turn produced (or copied) that
* has a chart; the chip names the view it is not shown in yet.
* - `as_chart`: a table this turn produced that has a chart and that the
* answer only used, so it sits folded under "Data used" as a table. A
* table the answer displays with a chart carries its own chart/table
* toggle, so it gets no view chip (`isRedundantViewChip`).
* - `previous_period`: a time series table of the current period, for a
* metric the table tool reads and the record holds.
* - `year_ago`: the same, over a window of at most 90 days, when the
@@ -59,6 +61,7 @@ import {
widerWindow,
} from "./catalog";
import type { FollowUpProposal } from "./parse-sentinel";
import { isRedundantViewChip } from "./view-chip";
import {
exceedsReach,
fitsReach,
@@ -195,18 +198,14 @@ export function deriveFollowUps(args: {
...(source.granularity ? { granularity: source.granularity } : {}),
period: source.period,
};
if (meta.chartKind !== null) {
// The other view of what the answer shows: a table shown as a table
// offers its chart, a shown chart offers its table.
const showsChart = meta.displayed && meta.view !== "table";
candidates.push(
buildFollowUp({
kind: showsChart ? "as_table" : "as_chart",
anchor,
origin: "server",
locale,
}),
);
const viewChip = buildFollowUp({
kind: "as_chart",
anchor,
origin: "server",
locale,
});
if (meta.chartKind !== null && !isRedundantViewChip(viewChip, [meta])) {
candidates.push(viewChip);
}
}
+42
View File
@@ -0,0 +1,42 @@
/**
* The view chips ("as a chart", "as a table") and when one would repeat a
* control the answer already shows.
*
* A leaf module on purpose: the stored-reply reader (`persistence.ts`) uses
* it, and that reader sits on the fenced document chat's module graph, which
* must not reach the Coach tool registry the rest of the chip catalog
* imports.
*/
import type {
CoachFollowUp,
CoachFollowUpKind,
CoachResultMeta,
} from "@/lib/ai/coach/types";
/** Kinds answered from a table the conversation already holds. */
export const REUSE_FOLLOW_UP_KINDS: ReadonlySet<CoachFollowUpKind> = new Set([
"as_chart",
"as_table",
]);
/**
* True when a view chip ("as a chart", "as a table") would repeat a control
* the answer already shows: a table the answer displays with a chart renders
* its own chart/table toggle right under the prose, so the chip adds
* nothing. A view chip still counts for a table the answer only used (it
* sits folded under "Data used", and the chip brings it into the answer)
* and every other kind is never redundant this way. One definition for the
* chips a turn derives and the chips a stored reply is read back with, so a
* reply stored before the rule offers the same as a new one, on every
* client.
*/
export function isRedundantViewChip(
chip: Pick<CoachFollowUp, "kind" | "anchor">,
results: readonly Pick<CoachResultMeta, "ref" | "displayed" | "chartKind">[],
): boolean {
if (!REUSE_FOLLOW_UP_KINDS.has(chip.kind)) return false;
const ref = chip.anchor?.ref;
return results.some(
(meta) => meta.ref === ref && meta.displayed && meta.chartKind !== null,
);
}
+7 -2
View File
@@ -23,6 +23,7 @@ import {
coachStepSchema,
} from "./stream-events";
import { COACH_CONVERSATION_TITLE_MAX } from "./types";
import { isRedundantViewChip } from "./follow-ups/view-chip";
import { RESULTS_MAX_BYTES, fitResultsToStorage } from "./results/refs";
import {
isCheckupIntervalId,
@@ -323,7 +324,11 @@ function provenanceFromJson(raw: string | null): CoachProvenance | null {
const steps = restoreSteps(parsed.steps);
const method = restoreMethod(parsed.method);
const results = restoreResultMetas(parsed.results);
const followUps = restoreFollowUps(parsed.followUps);
// A reply stored before view chips were withheld for a table the answer
// already shows with its own toggle reads back without them.
const followUps = restoreFollowUps(parsed.followUps)?.filter(
(chip) => !isRedundantViewChip(chip, results ?? []),
);
const clarification = restoreClarification(parsed.clarification);
const forcedFinal = parsed.forcedFinal === true;
const continuationOf =
@@ -345,7 +350,7 @@ function provenanceFromJson(raw: string | null): CoachProvenance | null {
...(steps ? { steps } : {}),
...(method ? { method } : {}),
...(results ? { results } : {}),
...(followUps ? { followUps } : {}),
...(followUps && followUps.length > 0 ? { followUps } : {}),
...(clarification ? { clarification } : {}),
...(forcedFinal ? { forcedFinal: true as const } : {}),
...(continuationOf ? { continuationOf } : {}),
@@ -290,14 +290,7 @@ describe("a reuse chip", () => {
const order = out
.map((f) => f.type)
.filter((type, i, all) => type !== "token" || all[i - 1] !== "token");
expect(order).toEqual([
"step",
"token",
"provenance",
"result",
"followUps",
"done",
]);
expect(order).toEqual(["step", "token", "provenance", "result", "done"]);
expect(
out.flatMap((f) => (f.type === "token" ? [f.token] : [])).join(""),
).toBe("Here are the same figures from my last answer, unchanged.");
@@ -310,11 +303,9 @@ describe("a reuse chip", () => {
chart: STORED.chart,
reusedFrom: { messageId: "m-last", ref: "r1" },
});
const followUps = out.find((f) => f.type === "followUps");
// Shown as a chart now, so the other view is what is offered.
expect(
followUps?.type === "followUps" && followUps.followUps.map((c) => c.kind),
).toEqual(["as_table"]);
// The table is shown with its chart, and the panel carries its own
// chart/table toggle, so no chip offers the other view again.
expect(out.some((f) => f.type === "followUps")).toBe(false);
const done = out.at(-1);
expect(done).toMatchObject({
type: "done",
@@ -327,7 +318,7 @@ describe("a reuse chip", () => {
expect(annotated("coach.followUp.reused")).toHaveLength(1);
});
it("shows the table first on 'as a table' and offers the chart back", async () => {
it("shows the table first on 'as a table', leaving the chart to the panel's toggle", async () => {
const asTable = { ...AS_CHART, kind: "as_table" as const };
m.findMany.mockResolvedValue(latestReply([asTable]));
m.readMessageResults.mockResolvedValue([STORED]);
@@ -338,10 +329,9 @@ describe("a reuse chip", () => {
chart: STORED.chart,
rows: STORED.rows,
});
const chips = out.find((f) => f.type === "followUps");
expect(
chips?.type === "followUps" && chips.followUps.map((c) => c.kind),
).toEqual(["as_chart"]);
// The chart stays on the result, one tap away on its toggle; a chip
// for it would repeat that toggle.
expect(out.some((f) => f.type === "followUps")).toBe(false);
});
it("goes to the model with the chip's hint when the table is withheld", async () => {