fix(coach): keep the chart on "as a table" so the chart is offered back

"As a table" used to drop the table's chart, so the reply offered no way
back to it. The copied table now keeps its chart and carries view: table,
which the panel opens on the table (the chart one tap away on the toggle)
and the chips read to offer "as a chart". show_result with view table
does the same. The field is optional on the result metadata; a client
that ignores it shows the chart first as before.
This commit is contained in:
Marc-André Bombeck committed 2026-09-27 13:36:48 +02:00
1 parent 5e9c4fe8d8
commit 21a2e2fcef
14 files changed
+95 -21

No files matched your search

+10
View File
@@ -50775,6 +50775,11 @@ components:
- messageId
- ref
additionalProperties: false
view:
description: "`table` when the answer asked for the table view of a table that has a chart; the chart stays for the
toggle. Absent: the chart shows first when there is one."
type: string
const: table
required:
- ref
- source
@@ -51074,6 +51079,11 @@ components:
- messageId
- ref
additionalProperties: false
view:
description: "`table` when the answer asked for the table view of a table that has a chart; the chart stays for the
toggle. Absent: the chart shows first when there is one."
type: string
const: table
columns:
type: array
items:
@@ -205,11 +205,18 @@ describe("a reuse chip", () => {
"result",
).map((f) => f.result);
expect(result.rows).toEqual(world.storedTables[0].rows);
expect(result.chart).toBeNull();
// The table shows first; the chart stays, so the chart is offered back.
expect(result.view).toBe("table");
expect(result.chart).toEqual(world.storedTables[0].chart);
expect(result.reusedFrom).toEqual({
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"]);
expect(m.appendMessage).toHaveBeenCalledWith(
expect.objectContaining({ role: "assistant", providerType: "reuse" }),
);
@@ -108,6 +108,16 @@ describe("CoachResults", () => {
expect(tableButton).toContain('aria-pressed="false"');
});
it("opens an 'as a table' answer on the table, with the chart one tap away", () => {
const html = results([table({ view: "table" })], "displayed");
expect(html).toContain('data-slot="coach-result-table"');
expect(html).not.toContain('data-slot="coach-result-chart"');
const chartButton = html.match(
/<button[^>]*data-slot="coach-result-view-chart"[^>]*>/,
)?.[0];
expect(chartButton).toContain('aria-pressed="false"');
});
it("opens a table without a chart as a table, with no toggle", () => {
const html = results(
[table({ chart: null, chartKind: null })],
@@ -247,8 +247,9 @@ function CoachResultView({
const titleId = useId();
const reusedLine = useReusedLine(result);
const hasChart = result.chart !== null;
// "As a table" keeps the chart for the toggle but shows the table first.
const [view, setView] = useState<ResultView>(
hasChart && chartFirst ? "chart" : "table",
hasChart && chartFirst && result.view !== "table" ? "chart" : "table",
);
if (!hasChart) {
return <CoachResultTable result={result} method={reusedLine} />;
@@ -700,7 +700,8 @@ describe.each(["de", "en"] as const)("in %s", (lang) => {
const [table] = resultsOf(frames);
// The person still sees the title on their own table.
expect(table.title).toBe(INJECTED_TITLE);
expect(table.chart).toBeNull();
// Asked for as a table: the table shows first, its chart kept beside it.
expect(table.view).toBe("table");
// Were the same text to ride the transcript, the fence check fails.
const leaked = await run({
@@ -152,6 +152,15 @@ 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 the other view of a table that has a chart, as a reuse chip", () => {
const shown = derive({
results: [table("bp", { chartKind: "line", displayed: true })],
+4 -1
View File
@@ -167,9 +167,12 @@ export function deriveFollowUps(args: {
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: meta.displayed ? "as_table" : "as_chart",
kind: showsChart ? "as_table" : "as_chart",
anchor,
origin: "server",
locale,
@@ -376,7 +376,7 @@ describe("show_result", () => {
});
});
it("shows the table without a chart for view table", async () => {
it("shows the table first for view table, the chart kept beside it", async () => {
readMessageResults.mockResolvedValue([STORED]);
const result = await executeCoachTool({
userId: "u1",
@@ -384,10 +384,12 @@ describe("show_result", () => {
rawArguments: JSON.stringify({ ref: "m2.r1", view: "table" }),
turn: turn(),
});
// The table shows first; the chart the server picks stays beside it.
expect(result.table).toMatchObject({
rows: STORED.rows,
chart: null,
chartKind: null,
view: "table",
chart: { kind: "line", x: "day", series: ["value"] },
chartKind: "line",
});
});
+6
View File
@@ -167,6 +167,12 @@ const coachResultMetaShape = {
.object({ messageId: messageIdSchema, ref: resultRefSchema })
.optional()
.describe("Set when the table was copied from an earlier message."),
view: z
.literal("table")
.optional()
.describe(
"`table` when the answer asked for the table view of a table that has a chart; the chart stays for the toggle. Absent: the chart shows first when there is one.",
),
};
export const coachResultMetaSchema = z.object(coachResultMetaShape).meta({
+11 -2
View File
@@ -893,11 +893,20 @@ async function getMetricTable(
* then reads the summary of what is shown.
*/
function withResultView(
table: CoachResultTable,
shown: CoachResultTable,
view: "table" | "chart" | undefined,
locale: Locale,
): CoachResultTable {
if (view === "table") return { ...table, chart: null, chartKind: null };
// The view is this showing's alone, never the stored copy's.
const { view: _stored, ...table } = shown;
if (view === "table") {
// The table shows first; the chart stays for the toggle and the chip
// back to it.
const chart = deriveChartSpec(table);
return chart
? { ...table, chart, chartKind: chart.kind, view: "table" }
: { ...table, chart: null, chartKind: null };
}
if (view === "chart") {
const { t } = getServerTranslator(locale);
const distribution = buildDistributionTable(table, {
@@ -327,16 +327,21 @@ describe("a reuse chip", () => {
expect(annotated("coach.followUp.reused")).toHaveLength(1);
});
it("drops the chart on 'as a table'", async () => {
it("shows the table first on 'as a table' and offers the chart back", async () => {
const asTable = { ...AS_CHART, kind: "as_table" as const };
m.findMany.mockResolvedValue(latestReply([asTable]));
m.readMessageResults.mockResolvedValue([STORED]);
const out = await frames(await runCoachTurn(input()));
const result = out.find((f) => f.type === "result");
expect(result?.type === "result" && result.result.chart).toBeNull();
expect(result?.type === "result" && result.result.rows).toEqual(
STORED.rows,
);
expect(result?.type === "result" && result.result).toMatchObject({
view: "table",
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"]);
});
it("goes to the model with the chip's hint when the table is withheld", async () => {
+1
View File
@@ -56,6 +56,7 @@ export function toResultMeta(table: CoachResultTable): CoachResultMeta {
chartKind: table.chartKind,
displayed: table.displayed,
...(table.reusedFrom ? { reusedFrom: table.reusedFrom } : {}),
...(table.view ? { view: table.view } : {}),
};
}
+11 -7
View File
@@ -6,8 +6,9 @@
* and persists an assistant message with `providerType: "reuse"`.
*
* The table is copied as it was stored: the same rows, the same columns,
* marked with where it came from. Only the view changes: "as a chart" keeps
* the stored chart, "as a table" drops it so the table is what shows.
* marked with where it came from. Only the view changes: "as a chart" shows
* the stored chart, "as a table" shows the table first and keeps the chart
* for the toggle and the chip back to it.
*
* Answers null, before anything is written, when the stored table cannot be
* served (withheld, unreadable, or no chart to show). The pipeline then runs
@@ -59,14 +60,17 @@ async function copyStoredTable(args: {
);
const entry = entries?.find((candidate) => candidate.ref === ref);
if (!entry || "withheld" in entry) return null;
const chart = followUp.kind === "as_chart" ? entry.chart : null;
if (followUp.kind === "as_chart" && chart === null) return null;
if (followUp.kind === "as_chart" && entry.chart === null) return null;
// Only the view changes. "As a table" keeps the chart beside the table, so
// the reply can offer the chart again.
const { view: _view, ...stored } = entry;
return {
...entry,
...stored,
ref: "r1",
displayed: true,
chart,
chartKind: chart?.kind ?? null,
...(followUp.kind === "as_table" && entry.chart !== null
? { view: "table" as const }
: {}),
reusedFrom: { messageId: sourceMessageId, ref },
};
}
+6
View File
@@ -387,6 +387,12 @@ export interface CoachResultMeta {
displayed: boolean;
/** Set when the table was copied from an earlier message of the thread. */
reusedFrom?: { messageId: string; ref: string };
/**
* v1.39.4 — `table` when the answer asked for the table view of a table
* that has a chart ("as a table"). The chart stays, so the other view is
* one tap away. Absent: the chart shows first when there is one.
*/
view?: "table";
}
/** Encrypted at rest; on the wire only to the owner. */