From 92c58768cd354e77b1af1c565f86f30f37d8fcfa Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 22 Jul 2026 16:32:28 +0200 Subject: [PATCH 1/5] feat: Added KPI variation pills --- apps/backend/src/agents/tools/story.ts | 1 + apps/backend/src/utils/story-html.tsx | 80 ++++++++- .../side-panel/story-chart-embed.tsx | 47 ++++-- apps/frontend/src/components/story-embeds.tsx | 3 +- .../tool-calls/display-chart-edit-dialog.tsx | 37 +++++ .../components/tool-calls/display-chart.tsx | 6 + apps/shared/src/chart-block.ts | 7 +- apps/shared/src/chart-builder.tsx | 154 +++++++++++++++++- apps/shared/src/story-segments.ts | 7 +- apps/shared/src/tools/display-chart.ts | 9 + apps/shared/tests/chart-builder.test.tsx | 125 +++++++++++++- apps/shared/tests/story-segments.test.ts | 48 +++++- apps/shared/tests/story-table-block.test.ts | 21 +++ 13 files changed, 516 insertions(+), 29 deletions(-) diff --git a/apps/backend/src/agents/tools/story.ts b/apps/backend/src/agents/tools/story.ts index 04b906ea2..897abc090 100644 --- a/apps/backend/src/agents/tools/story.ts +++ b/apps/backend/src/agents/tools/story.ts @@ -15,6 +15,7 @@ export default createTool({ 'Use "create" to initialize a new story, "update" to search-and-replace within it (producing a new version),', 'or "replace" to overwrite the entire content (producing a new version).', 'Charts are embedded via .', + 'For kpi_card charts you may add comparison_mode="percentage|variation|absolute" to show a period-over-period change pill; this requires the query to return at least two time-ordered rows (one per period) for the metric, and kpi_card does not need x_axis_key.', 'SQL result tables are embedded via .', 'Use ... to display charts side by side in a responsive grid.', 'Use consecutive ... blocks to organize a story into top-level tabs.', diff --git a/apps/backend/src/utils/story-html.tsx b/apps/backend/src/utils/story-html.tsx index 5781652e9..cd0fcc3f0 100644 --- a/apps/backend/src/utils/story-html.tsx +++ b/apps/backend/src/utils/story-html.tsx @@ -1,4 +1,11 @@ -import { bucketPieData, DEFAULT_COLORS, defaultColorFor, formatCompactNumber, labelize } from '@nao/shared'; +import { + bucketPieData, + computeKpiComparison, + DEFAULT_COLORS, + defaultColorFor, + formatCompactNumber, + labelize, +} from '@nao/shared'; import { type DateFormatSettings, DEFAULT_DATE_FORMAT_SETTINGS, @@ -203,7 +210,15 @@ function ChartLegend({ series }: { series: ParsedChartBlock['series'] }) { } function KpiCards({ chart, rows }: { chart: ParsedChartBlock; rows: Record[] }) { - const firstRow = rows[0] ?? {}; + const sortedRows = [...rows].sort((a, b) => { + const av = a[chart.xAxisKey]; + const bv = b[chart.xAxisKey]; + if (chart.xAxisType === 'date') { + return new Date(String(av)).getTime() - new Date(String(bv)).getTime(); + } + return 0; + }); + const lastRow = sortedRows[sortedRows.length - 1] ?? {}; return (
{chart.series.map((s) => { - const raw = firstRow[s.data_key]; + const raw = lastRow[s.data_key]; const value = typeof raw === 'number' ? formatCompactNumber(raw) : String(raw ?? ''); const label = s.label ?? s.data_key; + const comparison = computeKpiComparison(sortedRows, chart.xAxisKey, s.data_key, chart.comparisonMode); return (
{label}
-
{value}
+
+ {value} +
+ {comparison && + (() => { + const showArrow = comparison.colored && comparison.direction !== 'flat'; + const color = showArrow + ? comparison.direction === 'up' + ? '#16a34a' + : '#dc2626' + : '#6b7280'; + return ( +
+ {showArrow && ( + + + + )} + + {comparison.valueText} + + vs. {comparison.periodLabel} +
+ ); + })()}
); })} diff --git a/apps/frontend/src/components/side-panel/story-chart-embed.tsx b/apps/frontend/src/components/side-panel/story-chart-embed.tsx index bcc17407c..0ec49557b 100644 --- a/apps/frontend/src/components/side-panel/story-chart-embed.tsx +++ b/apps/frontend/src/components/side-panel/story-chart-embed.tsx @@ -21,6 +21,7 @@ interface ChartBlock { yAxisMax?: number; title: string; showDataLabels?: boolean; + comparisonMode?: 'percentage' | 'variation' | 'absolute' | 'none'; rawTag?: string; } @@ -75,7 +76,11 @@ export const StoryChartEmbed = memo(function StoryChartEmbed({ chart }: { chart: const xAxisType = chart.xAxisType === 'number' ? 'number' : ('category' as const); return ( - + ); @@ -94,6 +100,7 @@ export const StoryChartEmbed = memo(function StoryChartEmbed({ chart }: { chart: interface StoryChartEmbedShellProps { chart: ChartBlock; availableColumns: string[]; + dataRowCount?: number; children: React.ReactNode; } @@ -101,7 +108,7 @@ interface StoryChartEmbedShellProps { * Wraps a rendered chart with an "Edit chart" button when the surrounding story * context provides a save handler. */ -export function StoryChartEmbedShell({ chart, availableColumns, children }: StoryChartEmbedShellProps) { +export function StoryChartEmbedShell({ chart, availableColumns, dataRowCount, children }: StoryChartEmbedShellProps) { const edit = useStoryChartEdit(); const [isEditOpen, setIsEditOpen] = useState(false); const canEdit = Boolean(edit && chart.rawTag); @@ -122,41 +129,49 @@ export function StoryChartEmbedShell({ chart, availableColumns, children }: Stor y_axis_max: chart.yAxisMax, title: chart.title, show_data_labels: chart.showDataLabels, + comparison_mode: chart.comparisonMode, }), [chart], ); + const isKpi = chart.chartType == 'kpi_card'; + const editButton = canEdit ? ( + + ) : null; + return (
- {(canEdit || (chart.chartType != 'kpi_card' && chart.title)) && ( + {!isKpi && (canEdit || chart.title) && (
- {chart.chartType != 'kpi_card' && chart.title ? ( + {chart.title ? ( {chart.title} ) : (
)} - {canEdit && ( - - )} + {editButton}
)} -
{children}
+
+ {children} + {isKpi && editButton &&
{editButton}
} +
{canEdit && edit && chart.rawTag && ( edit.saveChart(chart.rawTag!, next)} description={edit.saveDescription} diff --git a/apps/frontend/src/components/story-embeds.tsx b/apps/frontend/src/components/story-embeds.tsx index 504938240..0d98254ff 100644 --- a/apps/frontend/src/components/story-embeds.tsx +++ b/apps/frontend/src/components/story-embeds.tsx @@ -75,7 +75,7 @@ export const StoryChartEmbed = memo(function StoryChartEmbed({ } return ( - + ); diff --git a/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx b/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx index 7de12cf5b..7cbf9cb97 100644 --- a/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx +++ b/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx @@ -32,6 +32,13 @@ const X_AXIS_TYPE_OPTIONS: { value: NonNullable | 'auto' { value: 'number', label: 'Number' }, ]; +const COMPARISON_MODE_OPTIONS: { value: displayChart.ComparisonMode; label: string }[] = [ + { value: 'none', label: 'None' }, + { value: 'percentage', label: 'Percentage' }, + { value: 'variation', label: 'Variation' }, + { value: 'absolute', label: 'Absolute' }, +]; + const Y_AXIS_RANGE_UNSUPPORTED_CHART_TYPES = new Set(['pie', 'kpi_card', 'radar']); /** Maps a 100% stacked type back to its absolute-stacked counterpart, so the type dropdown stays clean. */ @@ -64,6 +71,7 @@ interface ChartConfigEditDialogProps { onSave: (next: displayChart.ChartInput) => Promise; isSaving?: boolean; description?: string; + dataRowCount?: number; } /** Presentational edit dialog for `display_chart` configuration. */ @@ -75,6 +83,7 @@ export function ChartConfigEditDialog({ onSave, isSaving = false, description = 'Tweak the chart parameters.', + dataRowCount, }: ChartConfigEditDialogProps) { const [draft, setDraft] = useState(config); const [yAxisMinText, setYAxisMinText] = useState(toRangeString(config.y_axis_min)); @@ -360,6 +369,31 @@ export function ChartConfigEditDialog({ )}
Options + {draft.chart_type === 'kpi_card' && (dataRowCount ?? 0) >= 2 && ( +
+ Comparison pill + +
+ )}
@@ -380,6 +382,7 @@ export interface ChartDisplayProps { yAxisMin?: number; yAxisMax?: number; showDataLabels?: boolean; + comparisonMode?: displayChart.ComparisonMode; } export const ChartDisplay = memo(function ChartDisplay({ @@ -394,6 +397,7 @@ export const ChartDisplay = memo(function ChartDisplay({ yAxisMin, yAxisMax, showDataLabels, + comparisonMode, }: ChartDisplayProps) { const dateFormat = useDateFormat(); @@ -495,6 +499,7 @@ export const ChartDisplay = memo(function ChartDisplay({ labelFormatter, showGrid, showDataLabels, + comparisonMode, margin: { top: 0, right: 0, bottom: 0, left: 0 }, yAxisMin, yAxisMax, @@ -541,6 +546,7 @@ export const ChartDisplay = memo(function ChartDisplay({ yAxisMin, yAxisMax, showDataLabels, + comparisonMode, legendPayload, handleToggleSeriesVisibility, title, diff --git a/apps/shared/src/chart-block.ts b/apps/shared/src/chart-block.ts index b9638e465..803fe42c6 100644 --- a/apps/shared/src/chart-block.ts +++ b/apps/shared/src/chart-block.ts @@ -20,6 +20,7 @@ export type StoryChartBlockInput = Pick< | 'y_axis_min' | 'y_axis_max' | 'show_data_labels' + | 'comparison_mode' > & { title?: displayChart.ChartInput['title']; }; @@ -33,7 +34,11 @@ export function buildStoryChartBlock(input: StoryChartBlockInput): string { const titleAttr = input.title != null && input.title !== '' ? ` title="${escapeDoubleQuotedStoryAttr(input.title)}"` : ''; const dataLabelsAttr = input.show_data_labels ? ' show_data_labels="true"' : ''; - return ``; + const comparisonModeAttr = + input.comparison_mode && input.comparison_mode !== 'none' + ? ` comparison_mode="${escapeDoubleQuotedStoryAttr(input.comparison_mode)}"` + : ''; + return ``; } export type StoryTableBlockInput = Pick; diff --git a/apps/shared/src/chart-builder.tsx b/apps/shared/src/chart-builder.tsx index b5d052018..4070b6963 100644 --- a/apps/shared/src/chart-builder.tsx +++ b/apps/shared/src/chart-builder.tsx @@ -137,6 +137,108 @@ export function formatPercentShare(value: number, total: number): string { return `${Number.isInteger(rounded) ? rounded : rounded.toFixed(1)}%`; } +export type KpiComparisonDirection = 'up' | 'down' | 'flat'; + +export interface KpiComparison { + valueText: string; + direction: KpiComparisonDirection; + colored: boolean; + periodLabel: string; +} + +export function computeKpiComparison( + data: Record[], + xAxisKey: string, + dataKey: string, + mode: displayChart.ComparisonMode | undefined, +): KpiComparison | null { + if (!mode || mode === 'none' || data.length < 2) { + return null; + } + const currentRow = data[data.length - 1]; + const previousRow = data[data.length - 2]; + const current = toFiniteNumber(currentRow?.[dataKey]); + const previous = toFiniteNumber(previousRow?.[dataKey]); + if (current == null || previous == null) { + return null; + } + const delta = current - previous; + const direction: KpiComparisonDirection = delta > 0 ? 'up' : delta < 0 ? 'down' : 'flat'; + const dateKey = resolveDateKey(currentRow, xAxisKey, dataKey); + const periodLabel = + dateKey == null ? 'previous period' : describePreviousPeriod(previousRow?.[dateKey], currentRow?.[dateKey]); + + if (mode === 'percentage') { + if (previous === 0) { + return null; + } + const pct = (delta / Math.abs(previous)) * 100; + return { valueText: formatPercentMagnitude(Math.abs(pct)), direction, colored: true, periodLabel }; + } + if (mode === 'variation') { + return { valueText: formatCompactNumber(Math.abs(delta)), direction, colored: true, periodLabel }; + } + return { valueText: formatCompactNumber(Math.abs(delta)), direction, colored: false, periodLabel }; +} + +function formatPercentMagnitude(value: number): string { + const rounded = Math.round(value * 10) / 10; + return `${Number.isInteger(rounded) ? rounded : rounded.toFixed(1)}%`; +} + +export function describePreviousPeriod(previousX: unknown, currentX: unknown): string { + const prev = parseDateMs(previousX); + const curr = parseDateMs(currentX); + if (prev == null || curr == null) { + return 'previous period'; + } + const gapDays = Math.round((curr - prev) / 86_400_000); + if (gapDays === 1) { + return 'yesterday'; + } + if (gapDays >= 6 && gapDays <= 8) { + return 'last week'; + } + if (gapDays >= 26 && gapDays <= 33) { + return 'last month'; + } + if (gapDays >= 80 && gapDays <= 100) { + return 'last quarter'; + } + if (gapDays >= 330 && gapDays <= 400) { + return 'last year'; + } + return 'previous period'; +} + +function resolveDateKey(row: Record | undefined, xAxisKey: string, dataKey: string): string | null { + if (xAxisKey && parseDateMs(row?.[xAxisKey]) != null) { + return xAxisKey; + } + if (!row) { + return null; + } + for (const key of Object.keys(row)) { + if (key !== dataKey && parseDateMs(row[key]) != null) { + return key; + } + } + return null; +} + +function parseDateMs(value: unknown): number | null { + if (typeof value !== 'string') { + return null; + } + const trimmed = value.trim(); + if (!/^\d{4}-\d{2}(?:-\d{2})?(?:[ T].*)?$/.test(trimmed)) { + return null; + } + const normalized = trimmed.length === 7 ? `${trimmed}-01` : trimmed.replace(' ', 'T'); + const ms = new Date(normalized).getTime(); + return Number.isNaN(ms) ? null : ms; +} + export function formatDataLabel(value: unknown): string { const number = toFiniteNumber(value); return number == null ? '' : formatCompactNumber(number); @@ -165,6 +267,7 @@ export interface BuildChartProps { /** Chart background color, used as the separator between stacked segments. Pass a concrete color on surfaces where CSS vars do not resolve (backend PNG/HTML export). */ backgroundColor?: string; showDataLabels?: boolean; + comparisonMode?: displayChart.ComparisonMode; } /** @@ -356,7 +459,12 @@ function buildKpiCard(props: ResolvedProps) { return ( {series.map((s) => ( - + ))} ); @@ -366,7 +474,15 @@ function KpiCardContainer({ children }: { children: React.ReactNode }) { return
{children}
; } -function KpiCard({ value, displayName }: { value: unknown; displayName: string }) { +function KpiCard({ + value, + displayName, + comparison, +}: { + value: unknown; + displayName: string; + comparison: KpiComparison | null; +}) { let formattedValue = ''; if (typeof value === 'number') { @@ -375,14 +491,46 @@ function KpiCard({ value, displayName }: { value: unknown; displayName: string } formattedValue = value; } + const showArrowAndColor = comparison != null && comparison.colored && comparison.direction !== 'flat'; + const pillColorClass = showArrowAndColor + ? comparison.direction === 'up' + ? 'text-green-600' + : 'text-red-600' + : 'text-muted-foreground'; + return (
{displayName}
-
{formattedValue}
+
{formattedValue}
+ {comparison && ( +
+ {showArrowAndColor && } + {comparison.valueText} + vs. {comparison.periodLabel} +
+ )}
); } +function KpiTrendArrow({ direction }: { direction: KpiComparisonDirection }) { + return ( + + ); +} + function renderValueYAxis(isPercent = false) { return ( ` tag this block was parsed from, when available. */ rawTag?: string; } @@ -62,7 +63,8 @@ export function parseChartAttributes(attrString: string): Record export function parseChartBlock(attrString: string): ParsedChartBlock | null { const attrs = parseChartAttributes(attrString); - if (!attrs.query_id || !attrs.chart_type || !attrs.x_axis_key) { + const requiresXAxisKey = attrs.chart_type !== 'kpi_card'; + if (!attrs.query_id || !attrs.chart_type || (requiresXAxisKey && !attrs.x_axis_key)) { return null; } @@ -86,13 +88,14 @@ export function parseChartBlock(attrString: string): ParsedChartBlock | null { return { queryId: attrs.query_id, chartType: attrs.chart_type, - xAxisKey: attrs.x_axis_key, + xAxisKey: attrs.x_axis_key ?? '', xAxisType: attrs.x_axis_type || null, series, yAxisMin, yAxisMax, title: attrs.title || '', showDataLabels: attrs.show_data_labels === 'true', + comparisonMode: (attrs.comparison_mode as ParsedChartBlock['comparisonMode']) || undefined, }; } diff --git a/apps/shared/src/tools/display-chart.ts b/apps/shared/src/tools/display-chart.ts index ace26fec6..43b6a93c7 100644 --- a/apps/shared/src/tools/display-chart.ts +++ b/apps/shared/src/tools/display-chart.ts @@ -17,6 +17,9 @@ export const ChartTypeEnum = z.enum([ export const XAxisTypeEnum = z.enum(['date', 'number', 'category']); +export const ComparisonModeEnum = z.enum(['percentage', 'variation', 'absolute', 'none']); +export type ComparisonMode = z.infer; + export const SeriesConfigSchema = z.object({ data_key: z.string().describe('Column name from SQL result to plot.'), color: z.string().describe('CSS color (defaults to theme colors).').optional(), @@ -111,6 +114,9 @@ export const ChartInputSchema = z 'Show the numeric value of each data point directly on the chart. Set to true when the user asks to display values/data labels on the chart.', ) .optional(), + comparison_mode: ComparisonModeEnum.describe( + 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', + ).optional(), title: z .string() .describe( @@ -152,6 +158,9 @@ const BaseInputSchema = z.object({ .min(1) .describe('Columns to plot as data series. Required for charts and omitted for tables.') .optional(), + comparison_mode: ComparisonModeEnum.describe( + 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', + ).optional(), title: z.string().describe('A concise, descriptive title for the visualization. Required for charts.').optional(), conditional_formats: ColumnConditionalFormatsSchema.describe( 'Conditional formatting rules for table columns. Only used when chart_type is "table".', diff --git a/apps/shared/tests/chart-builder.test.tsx b/apps/shared/tests/chart-builder.test.tsx index 7eff853b5..018a776c9 100644 --- a/apps/shared/tests/chart-builder.test.tsx +++ b/apps/shared/tests/chart-builder.test.tsx @@ -1,7 +1,7 @@ import type { ReactElement } from 'react'; import { describe, expect, it } from 'vitest'; -import { buildChart } from '../src/chart-builder'; +import { buildChart, computeKpiComparison, describePreviousPeriod } from '../src/chart-builder'; describe('buildChart', () => { it('uses stack totals for stacked bar fallback bounds', () => { @@ -65,6 +65,129 @@ describe('buildChart', () => { }); }); +describe('computeKpiComparison', () => { + const rows = [ + { period: '2023-01-01', value: 100 }, + { period: '2024-01-01', value: 120 }, + ]; + + it('returns null without a previous row', () => { + expect(computeKpiComparison([rows[0]], 'period', 'value', 'percentage')).toBeNull(); + }); + + it('returns null when comparison is disabled', () => { + expect(computeKpiComparison(rows, 'period', 'value', 'none')).toBeNull(); + expect(computeKpiComparison(rows, 'period', 'value', undefined)).toBeNull(); + }); + + it('computes a percentage increase', () => { + expect(computeKpiComparison(rows, 'period', 'value', 'percentage')).toEqual({ + valueText: '20%', + direction: 'up', + colored: true, + periodLabel: 'last year', + }); + }); + + it('infers the period label from a date column when xAxisKey is empty', () => { + expect( + computeKpiComparison( + [ + { month: '2024-01-01', revenue: 100 }, + { month: '2024-02-01', revenue: 150 }, + ], + '', + 'revenue', + 'percentage', + ), + ).toMatchObject({ valueText: '50%', direction: 'up', periodLabel: 'last month' }); + }); + + it('computes a percentage decrease as an unsigned magnitude', () => { + expect( + computeKpiComparison( + [ + { period: '2023-01-01', value: 100 }, + { period: '2024-01-01', value: 80 }, + ], + 'period', + 'value', + 'percentage', + ), + ).toMatchObject({ valueText: '20%', direction: 'down', colored: true }); + }); + + it('returns null for percentage change from zero', () => { + expect( + computeKpiComparison( + [ + { period: '2023-01-01', value: 0 }, + { period: '2024-01-01', value: 20 }, + ], + 'period', + 'value', + 'percentage', + ), + ).toBeNull(); + }); + + it('computes variation as a colored absolute delta', () => { + expect(computeKpiComparison(rows, 'period', 'value', 'variation')).toMatchObject({ + valueText: '20', + direction: 'up', + colored: true, + }); + }); + + it('computes absolute mode without color', () => { + expect(computeKpiComparison(rows, 'period', 'value', 'absolute')).toMatchObject({ + valueText: '20', + direction: 'up', + colored: false, + }); + }); + + it('marks an unchanged value as flat', () => { + expect( + computeKpiComparison( + [ + { period: '2023-01-01', value: 100 }, + { period: '2024-01-01', value: 100 }, + ], + 'period', + 'value', + 'percentage', + ), + ).toMatchObject({ valueText: '0%', direction: 'flat', colored: true }); + }); +}); + +describe('describePreviousPeriod', () => { + it('describes yearly date gaps', () => { + expect(describePreviousPeriod('2023-01-01', '2024-01-01')).toBe('last year'); + }); + + it('describes monthly date gaps', () => { + expect(describePreviousPeriod('2024-01-01', '2024-02-01')).toBe('last month'); + }); + + it('describes month-only date gaps', () => { + expect(describePreviousPeriod('2024-01', '2024-02')).toBe('last month'); + }); + + it('handles space-separated datetimes', () => { + expect(describePreviousPeriod('2023-01-01 00:00:00', '2024-01-01 00:00:00')).toBe('last year'); + }); + + it('describes weekly date gaps', () => { + expect(describePreviousPeriod('2024-01-01', '2024-01-08')).toBe('last week'); + }); + + it('falls back for non-date categories', () => { + expect(describePreviousPeriod('Q1', 'Q2')).toBe('previous period'); + }); +}); + function getYAxis(chart: ReactElement): ReactElement | undefined { return flattenChildren(chart.props.children).find((child) => child.type.displayName === 'YAxis'); } diff --git a/apps/shared/tests/story-segments.test.ts b/apps/shared/tests/story-segments.test.ts index fb0f5ca5d..814884dc2 100644 --- a/apps/shared/tests/story-segments.test.ts +++ b/apps/shared/tests/story-segments.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import { buildStoryChartBlock } from '../src/chart-block'; -import { splitCodeIntoSegments } from '../src/story-segments'; +import { parseChartBlock, splitCodeIntoSegments } from '../src/story-segments'; function chartOf(code: string) { const segment = splitCodeIntoSegments(code).find((s) => s.type === 'chart'); @@ -66,6 +66,52 @@ describe('splitCodeIntoSegments chart series', () => { }); }); +describe('parseChartBlock x-axis requirements', () => { + it('allows KPI cards without an x-axis key but rejects other chart types', () => { + const kpiCard = parseChartBlock( + 'query_id="query_bd642c80" chart_type="kpi_card" title="Total Revenue ($)" series=\'[{"data_key":"total_revenue"}]\'', + ); + + expect(kpiCard).not.toBeNull(); + expect(kpiCard?.xAxisKey).toBe(''); + expect(kpiCard?.series).toEqual([{ data_key: 'total_revenue' }]); + expect( + parseChartBlock('query_id="query_bd642c80" chart_type="bar" series=\'[{"data_key":"total_revenue"}]\''), + ).toBeNull(); + }); +}); + +describe('KPI comparison mode story blocks', () => { + it('round-trips a comparison mode', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'kpi_card', + x_axis_key: 'year', + x_axis_type: 'date', + series: [{ data_key: 'revenue' }], + title: 'Revenue', + comparison_mode: 'percentage', + }); + + expect(code).toContain('comparison_mode="percentage"'); + expect(chartOf(code)?.comparisonMode).toBe('percentage'); + }); + + it('does not emit the none comparison mode', () => { + const code = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'kpi_card', + x_axis_key: 'year', + x_axis_type: 'date', + series: [{ data_key: 'revenue' }], + title: 'Revenue', + comparison_mode: 'none', + }); + + expect(code).not.toContain('comparison_mode='); + }); +}); + describe('splitCodeIntoSegments slash-in-attribute handling', () => { it('parses a chart whose title contains a slash', () => { const code = diff --git a/apps/shared/tests/story-table-block.test.ts b/apps/shared/tests/story-table-block.test.ts index 41e1ccc2e..3b7016e4f 100644 --- a/apps/shared/tests/story-table-block.test.ts +++ b/apps/shared/tests/story-table-block.test.ts @@ -150,6 +150,27 @@ describe('displayChart.InputSchema table variant', () => { expect(result.success).toBe(true); }); + it('retains KPI comparison mode in chart schemas', () => { + const input = { + query_id: 'query_1', + chart_type: 'kpi_card' as const, + x_axis_key: 'year', + x_axis_type: 'date' as const, + series: [{ data_key: 'sales' }], + title: 'Annual sales', + comparison_mode: 'percentage' as const, + }; + + expect(displayChart.ChartInputSchema.safeParse(input)).toMatchObject({ + success: true, + data: { comparison_mode: 'percentage' }, + }); + expect(displayChart.InputSchema.safeParse(input)).toMatchObject({ + success: true, + data: { comparison_mode: 'percentage' }, + }); + }); + it('rejects a chart config missing chart fields', () => { const result = displayChart.InputSchema.safeParse({ query_id: 'query_1', From fada8374c2007f78e72031fdbb186ca1435ce845 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 22 Jul 2026 17:24:09 +0200 Subject: [PATCH 2/5] fix: Chart loading screen bug fixed --- .../tool-calls/display-chart-edit-dialog.tsx | 72 +++++++------- .../components/tool-calls/display-chart.tsx | 18 +++- apps/frontend/src/lib/charts.utils.test.ts | 8 +- apps/frontend/src/lib/charts.utils.ts | 5 +- apps/shared/src/chart-block.ts | 5 +- apps/shared/src/tools/display-chart.ts | 96 +++++++++++-------- apps/shared/tests/story-table-block.test.ts | 34 ++++++- 7 files changed, 157 insertions(+), 81 deletions(-) diff --git a/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx b/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx index 7cbf9cb97..1928c479f 100644 --- a/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx +++ b/apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx @@ -109,7 +109,11 @@ export function ChartConfigEditDialog({ const handleSubmit = async (event: React.FormEvent) => { event.preventDefault(); - const parsed = displayChart.ChartInputSchema.safeParse(draft); + const normalized: displayChart.ChartInput = + draft.chart_type === 'kpi_card' + ? { ...draft, x_axis_key: draft.x_axis_key || '', x_axis_type: draft.x_axis_type ?? null } + : draft; + const parsed = displayChart.ChartInputSchema.safeParse(normalized); if (!parsed.success) { setError(parsed.error.issues[0]?.message ?? 'Invalid chart configuration.'); return; @@ -220,29 +224,31 @@ export function ChartConfigEditDialog({
-
- X-axis type - -
+ {draft.chart_type !== 'kpi_card' && ( +
+ X-axis type + +
+ )}
{displayChart.isStackedChartType(draft.chart_type) && ( @@ -270,14 +276,16 @@ export function ChartConfigEditDialog({
)} -
- X-axis column - setDraft((prev) => ({ ...prev, x_axis_key: value }))} - /> -
+ {draft.chart_type !== 'kpi_card' && ( +
+ X-axis column + setDraft((prev) => ({ ...prev, x_axis_key: value }))} + /> +
+ )}
diff --git a/apps/frontend/src/components/tool-calls/display-chart.tsx b/apps/frontend/src/components/tool-calls/display-chart.tsx index bdaf288b7..5e56c99c4 100644 --- a/apps/frontend/src/components/tool-calls/display-chart.tsx +++ b/apps/frontend/src/components/tool-calls/display-chart.tsx @@ -34,6 +34,7 @@ import { import { useDateFormat } from '@/hooks/use-date-format'; import { useChatId } from '@/hooks/use-chat-id'; import { useSidePanel } from '@/contexts/side-panel'; +import { useToolCallContext } from '@/contexts/tool-call'; import { StoryViewer } from '@/components/side-panel/story-viewer'; import { cn } from '@/lib/utils'; import { downloadCsv, tableToCsv } from '@/lib/table-export'; @@ -51,6 +52,7 @@ export const DisplayChartToolCall = ({ const chatId = useChatId(); const queryClient = useQueryClient(); const { open: openSidePanel, currentStorySlug, isVisible } = useSidePanel(); + const { isSettled } = useToolCallContext(); const config = state !== 'input-streaming' ? input : undefined; const chartConfig = config?.chart_type === 'table' ? undefined : config; const tableConfig = config?.chart_type === 'table' ? config : undefined; @@ -156,6 +158,12 @@ export const DisplayChartToolCall = ({ } if (!chartConfig) { + // Only show the loader while the input is genuinely still streaming. An + // orphaned partial tool call stuck in `input-streaming` after the message + // settled would otherwise render an endless loader. + if (state !== 'input-streaming' || isSettled) { + return null; + } return (
@@ -229,20 +237,24 @@ export const DisplayChartToolCall = ({ } }; + const isKpiChartView = viewMode === 'chart' && chartConfig.chart_type === 'kpi_card'; + return (
{chartConfig.chart_type != 'kpi_card' ? ( diff --git a/apps/frontend/src/lib/charts.utils.test.ts b/apps/frontend/src/lib/charts.utils.test.ts index aba0bd568..4dfc1978e 100644 --- a/apps/frontend/src/lib/charts.utils.test.ts +++ b/apps/frontend/src/lib/charts.utils.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { resolvePieTooltipLabel } from './charts.utils'; +import { resolveDataKey, resolvePieTooltipLabel } from './charts.utils'; + +describe('resolveDataKey', () => { + it('returns an empty string when the key is undefined (kpi cards have no x-axis)', () => { + expect(resolveDataKey([{ revenue: 100 }], undefined)).toBe(''); + }); +}); describe('resolvePieTooltipLabel', () => { it('returns the slice category name from the payload', () => { diff --git a/apps/frontend/src/lib/charts.utils.ts b/apps/frontend/src/lib/charts.utils.ts index abded0f20..9563577e8 100644 --- a/apps/frontend/src/lib/charts.utils.ts +++ b/apps/frontend/src/lib/charts.utils.ts @@ -102,7 +102,10 @@ export function resolvePieTooltipLabel(payload?: readonly { name?: unknown }[]): } /** Resolves a config key to the matching key in the data, ignoring case. Falls back to the original key. */ -export function resolveDataKey(data: Record[], key: string): string { +export function resolveDataKey(data: Record[], key: string | undefined): string { + if (key === undefined) { + return ''; + } const row = data[0]; if (!row || key in row) { return key; diff --git a/apps/shared/src/chart-block.ts b/apps/shared/src/chart-block.ts index 803fe42c6..df9dc18fc 100644 --- a/apps/shared/src/chart-block.ts +++ b/apps/shared/src/chart-block.ts @@ -14,7 +14,6 @@ export type StoryChartBlockInput = Pick< displayChart.ChartInput, | 'query_id' | 'chart_type' - | 'x_axis_key' | 'x_axis_type' | 'series' | 'y_axis_min' @@ -23,9 +22,11 @@ export type StoryChartBlockInput = Pick< | 'comparison_mode' > & { title?: displayChart.ChartInput['title']; + x_axis_key?: displayChart.ChartInput['x_axis_key']; }; export function buildStoryChartBlock(input: StoryChartBlockInput): string { + const xAxisKeyAttr = input.x_axis_key ? ` x_axis_key="${escapeDoubleQuotedStoryAttr(input.x_axis_key)}"` : ''; const xAxisTypeAttr = input.x_axis_type != null ? ` x_axis_type="${escapeDoubleQuotedStoryAttr(input.x_axis_type)}"` : ''; const yMinAttr = input.y_axis_min !== undefined ? ` y_axis_min="${input.y_axis_min}"` : ''; @@ -38,7 +39,7 @@ export function buildStoryChartBlock(input: StoryChartBlockInput): string { input.comparison_mode && input.comparison_mode !== 'none' ? ` comparison_mode="${escapeDoubleQuotedStoryAttr(input.comparison_mode)}"` : ''; - return ``; + return ``; } export type StoryTableBlockInput = Pick; diff --git a/apps/shared/src/tools/display-chart.ts b/apps/shared/src/tools/display-chart.ts index 43b6a93c7..1857d73b6 100644 --- a/apps/shared/src/tools/display-chart.ts +++ b/apps/shared/src/tools/display-chart.ts @@ -89,47 +89,56 @@ export const ColumnConditionalFormatsSchema = z .record(z.string(), ConditionalFormatRuleSchema) .describe('Map of column name to the conditional-formatting rule applied to that column.'); -export const ChartInputSchema = z - .object({ - query_id: z.string().describe("The id of a previous `execute_sql` tool call's output to get data from."), - chart_type: ChartTypeEnum.describe('Type of chart to display.'), - x_axis_key: z.string().describe('Column name for X-axis/category labels.'), - x_axis_type: XAxisTypeEnum.nullable().describe( - 'Use "date" only when x-axis values parse as JS Date (YYYY-MM-DD). Use "category" for quarter_ending, fiscal periods, or labels. Use "number" for numeric x-axis.', +const ChartInputObjectSchema = z.object({ + query_id: z.string().describe("The id of a previous `execute_sql` tool call's output to get data from."), + chart_type: ChartTypeEnum.describe('Type of chart to display.'), + x_axis_key: z.string().describe('Column name for X-axis/category labels.'), + x_axis_type: XAxisTypeEnum.nullable().describe( + 'Use "date" only when x-axis values parse as JS Date (YYYY-MM-DD). Use "category" for quarter_ending, fiscal periods, or labels. Use "number" for numeric x-axis.', + ), + series: z + .array(SeriesConfigSchema) + .min(1) + .describe('Columns to plot as data series (at least one series required).'), + y_axis_min: z + .number() + .describe( + 'Fixes the Y-axis lower bound. Leave unset to auto-scale for readability (line and scatter charts do not force a zero baseline).', + ) + .optional(), + y_axis_max: z.number().describe('Fixes the Y-axis upper bound. Leave unset to auto-scale.').optional(), + show_data_labels: z + .boolean() + .describe( + 'Show the numeric value of each data point directly on the chart. Set to true when the user asks to display values/data labels on the chart.', + ) + .optional(), + comparison_mode: ComparisonModeEnum.describe( + 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', + ).optional(), + title: z + .string() + .describe( + 'A concise and descriptive title of what the chart shows. Do not include the type of chart in the title or other chart configurations.', ), - series: z - .array(SeriesConfigSchema) - .min(1) - .describe('Columns to plot as data series (at least one series required).'), - y_axis_min: z - .number() - .describe( - 'Fixes the Y-axis lower bound. Leave unset to auto-scale for readability (line and scatter charts do not force a zero baseline).', - ) - .optional(), - y_axis_max: z.number().describe('Fixes the Y-axis upper bound. Leave unset to auto-scale.').optional(), - show_data_labels: z - .boolean() - .describe( - 'Show the numeric value of each data point directly on the chart. Set to true when the user asks to display values/data labels on the chart.', - ) - .optional(), - comparison_mode: ComparisonModeEnum.describe( - 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', - ).optional(), - title: z - .string() - .describe( - 'A concise and descriptive title of what the chart shows. Do not include the type of chart in the title or other chart configurations.', - ), - }) - .refine( - (input) => - input.y_axis_min === undefined || input.y_axis_max === undefined || input.y_axis_min < input.y_axis_max, - { - message: 'The Y-axis minimum must be less than the maximum.', - }, - ); +}); + +const yAxisBoundsValid = (input: { y_axis_min?: number; y_axis_max?: number }) => + input.y_axis_min === undefined || input.y_axis_max === undefined || input.y_axis_min < input.y_axis_max; + +const Y_AXIS_BOUNDS_MESSAGE = { message: 'The Y-axis minimum must be less than the maximum.' }; + +export const ChartInputSchema = ChartInputObjectSchema.refine(yAxisBoundsValid, Y_AXIS_BOUNDS_MESSAGE); + +/** KPI cards render a single headline number and have no axes, so they may omit the x-axis fields. */ +const KpiCardInputSchema = ChartInputObjectSchema.extend({ + x_axis_key: z.string().describe('Column name for X-axis/category labels.').optional(), + x_axis_type: XAxisTypeEnum.nullable() + .describe( + 'Use "date" only when x-axis values parse as JS Date (YYYY-MM-DD). Use "category" for quarter_ending, fiscal periods, or labels. Use "number" for numeric x-axis.', + ) + .optional(), +}).refine(yAxisBoundsValid, Y_AXIS_BOUNDS_MESSAGE); export const TableInputSchema = z.object({ query_id: z.string().describe("The id of a previous `execute_sql` tool call's output to get data from."), @@ -168,7 +177,12 @@ const BaseInputSchema = z.object({ }); export const InputSchema = BaseInputSchema.superRefine((input, context) => { - const result = input.chart_type === 'table' ? TableInputSchema.safeParse(input) : ChartInputSchema.safeParse(input); + const result = + input.chart_type === 'table' + ? TableInputSchema.safeParse(input) + : input.chart_type === 'kpi_card' + ? KpiCardInputSchema.safeParse(input) + : ChartInputSchema.safeParse(input); if (result.success) { return; } diff --git a/apps/shared/tests/story-table-block.test.ts b/apps/shared/tests/story-table-block.test.ts index 3b7016e4f..c992f7ced 100644 --- a/apps/shared/tests/story-table-block.test.ts +++ b/apps/shared/tests/story-table-block.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { buildStoryTableBlock } from '../src/chart-block'; +import { buildStoryChartBlock, buildStoryTableBlock } from '../src/chart-block'; import type { ColumnConditionalFormats } from '../src/conditional-formatting'; import { injectTableFormatting, parseTableBlock, splitCodeIntoSegments } from '../src/story-segments'; import { displayChart } from '../src/tools'; @@ -150,6 +150,38 @@ describe('displayChart.InputSchema table variant', () => { expect(result.success).toBe(true); }); + it('accepts a kpi_card input without x-axis fields', () => { + const result = displayChart.InputSchema.safeParse({ + query_id: 'q1', + chart_type: 'kpi_card', + series: [{ data_key: 'revenue' }], + title: 'Revenue', + }); + expect(result.success).toBe(true); + }); + + it('still requires x_axis_key for line charts', () => { + const result = displayChart.InputSchema.safeParse({ + query_id: 'q1', + chart_type: 'line', + series: [{ data_key: 'revenue' }], + title: 'Revenue', + }); + expect(result.success).toBe(false); + }); + + it('omits the x_axis_key attribute for kpi cards without an axis', () => { + const block = buildStoryChartBlock({ + query_id: 'q1', + chart_type: 'kpi_card', + x_axis_type: null, + series: [{ data_key: 'revenue' }], + title: 'Revenue', + }); + expect(block).not.toContain('x_axis_key='); + expect(block).toContain('chart_type="kpi_card"'); + }); + it('retains KPI comparison mode in chart schemas', () => { const input = { query_id: 'query_1', From c167c5207c982b4cba2ad6f4672961b67ea51f97 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 22 Jul 2026 15:57:12 +0000 Subject: [PATCH 3/5] Fixed Cubic complaints --- .../backend/src/components/generate-chart.tsx | 2 + .../side-panel/story-chart-embed.tsx | 12 +- apps/frontend/src/components/story-embeds.tsx | 2 +- .../tool-calls/display-chart-edit-dialog.tsx | 33 ++++- .../components/tool-calls/display-chart.tsx | 2 +- apps/shared/src/story-validation.ts | 5 +- apps/shared/tests/chart-builder.test.tsx | 4 + package-lock.json | 140 ++++++++---------- 8 files changed, 108 insertions(+), 92 deletions(-) diff --git a/apps/backend/src/components/generate-chart.tsx b/apps/backend/src/components/generate-chart.tsx index b58d47a6b..2b07708a0 100644 --- a/apps/backend/src/components/generate-chart.tsx +++ b/apps/backend/src/components/generate-chart.tsx @@ -23,6 +23,7 @@ export interface RenderChartInput { | 'y_axis_max' | 'title' | 'show_data_labels' + | 'comparison_mode' >; data: Record[]; width?: number; @@ -87,6 +88,7 @@ export function renderChartToSvg(input: RenderChartInput): string { backgroundColor: '#ffffff', yAxisMin: config.y_axis_min, yAxisMax: config.y_axis_max, + comparisonMode: config.comparison_mode, }); const html = renderToString(React.cloneElement(chart, { width: chartWidth, height })); diff --git a/apps/frontend/src/components/side-panel/story-chart-embed.tsx b/apps/frontend/src/components/side-panel/story-chart-embed.tsx index 0ec49557b..fcdf2d01d 100644 --- a/apps/frontend/src/components/side-panel/story-chart-embed.tsx +++ b/apps/frontend/src/components/side-panel/story-chart-embed.tsx @@ -76,11 +76,7 @@ export const StoryChartEmbed = memo(function StoryChartEmbed({ chart }: { chart: const xAxisType = chart.xAxisType === 'number' ? 'number' : ('category' as const); return ( - + []; children: React.ReactNode; } @@ -108,7 +104,7 @@ interface StoryChartEmbedShellProps { * Wraps a rendered chart with an "Edit chart" button when the surrounding story * context provides a save handler. */ -export function StoryChartEmbedShell({ chart, availableColumns, dataRowCount, children }: StoryChartEmbedShellProps) { +export function StoryChartEmbedShell({ chart, availableColumns, data, children }: StoryChartEmbedShellProps) { const edit = useStoryChartEdit(); const [isEditOpen, setIsEditOpen] = useState(false); const canEdit = Boolean(edit && chart.rawTag); @@ -171,7 +167,7 @@ export function StoryChartEmbedShell({ chart, availableColumns, dataRowCount, ch onOpenChange={setIsEditOpen} config={config} availableColumns={availableColumns} - dataRowCount={dataRowCount} + data={data} isSaving={edit.isSaving} onSave={(next) => edit.saveChart(chart.rawTag!, next)} description={edit.saveDescription} diff --git a/apps/frontend/src/components/story-embeds.tsx b/apps/frontend/src/components/story-embeds.tsx index 0d98254ff..c19d14ff9 100644 --- a/apps/frontend/src/components/story-embeds.tsx +++ b/apps/frontend/src/components/story-embeds.tsx @@ -75,7 +75,7 @@ export const StoryChartEmbed = memo(function StoryChartEmbed({ } return ( - + Promise; isSaving?: boolean; description?: string; - dataRowCount?: number; + data?: Record[]; } /** Presentational edit dialog for `display_chart` configuration. */ @@ -83,13 +84,17 @@ export function ChartConfigEditDialog({ onSave, isSaving = false, description = 'Tweak the chart parameters.', - dataRowCount, + data, }: ChartConfigEditDialogProps) { const [draft, setDraft] = useState(config); const [yAxisMinText, setYAxisMinText] = useState(toRangeString(config.y_axis_min)); const [yAxisMaxText, setYAxisMaxText] = useState(toRangeString(config.y_axis_max)); const [error, setError] = useState(null); const supportsYAxisRange = !Y_AXIS_RANGE_UNSUPPORTED_CHART_TYPES.has(draft.chart_type); + const canShowComparisonPill = useMemo( + () => draft.chart_type === 'kpi_card' && hasRenderableKpiComparison(data, draft.x_axis_key, draft.series), + [draft.chart_type, draft.x_axis_key, draft.series, data], + ); useEffect(() => { if (open) { @@ -377,7 +382,7 @@ export function ChartConfigEditDialog({ )}
Options - {draft.chart_type === 'kpi_card' && (dataRowCount ?? 0) >= 2 && ( + {canShowComparisonPill && (
Comparison pill setDraft((prev) => ({ ...prev, @@ -445,7 +451,7 @@ interface DisplayChartEditDialogProps { open: boolean; onOpenChange: (open: boolean) => void; toolCallId: string; - config: displayChart.ChartInput; + config: EditableChartInput; availableColumns: string[]; dataRowCount?: number; } @@ -470,7 +476,7 @@ export function DisplayChartEditDialog({ }), ); - const handleSave = async (next: displayChart.ChartInput) => { + const handleSave = async (next: EditableChartInput) => { const previousMessages = messages; setMessages(applyChartConfigToMessages(previousMessages, toolCallId, next)); try { @@ -547,7 +553,7 @@ function normalizeHexColor(color?: string): string { function applyChartConfigToMessages( messages: UIMessage[], toolCallId: string, - config: displayChart.ChartInput, + config: EditableChartInput, ): UIMessage[] { return messages.map((message) => { let changed = false; diff --git a/apps/frontend/src/components/tool-calls/display-chart.tsx b/apps/frontend/src/components/tool-calls/display-chart.tsx index 5e56c99c4..278cf9f6b 100644 --- a/apps/frontend/src/components/tool-calls/display-chart.tsx +++ b/apps/frontend/src/components/tool-calls/display-chart.tsx @@ -368,14 +368,14 @@ export const DisplayChartToolCall = ({ )}
diff --git a/apps/frontend/src/contexts/story-chart-edit.tsx b/apps/frontend/src/contexts/story-chart-edit.tsx index 5b8d8a7ba..aac1f53e2 100644 --- a/apps/frontend/src/contexts/story-chart-edit.tsx +++ b/apps/frontend/src/contexts/story-chart-edit.tsx @@ -3,13 +3,15 @@ import { buildStoryChartBlock } from '@nao/shared'; import { useStoryBlockEdit } from './use-story-block-edit'; import type { displayChart } from '@nao/shared/tools'; +type EditableChartInput = displayChart.ChartInput | displayChart.KpiCardInput; + export interface StoryChartEditHandlers { /** * Persists a new chart config by replacing `rawTag` (the original `` tag) * in the story's markdown and saving a new version. * Returns a promise that rejects if the save fails. */ - saveChart: (rawTag: string, config: displayChart.ChartInput) => Promise; + saveChart: (rawTag: string, config: EditableChartInput) => Promise; /** Whether a save is currently in flight. */ isSaving: boolean; /** Human-readable hint describing how the edit is persisted, shown in the edit dialog. */ @@ -66,7 +68,7 @@ export function StoryChartEditProvider({ const { replaceBlock, isSaving } = useStoryBlockEdit({ chatId, storySlug, storyTitle, storyCode }); const saveChart = useCallback( - (rawTag: string, config: displayChart.ChartInput) => replaceBlock(rawTag, buildStoryChartBlock(config)), + (rawTag: string, config: EditableChartInput) => replaceBlock(rawTag, buildStoryChartBlock(config)), [replaceBlock], ); diff --git a/apps/shared/src/chart-block.ts b/apps/shared/src/chart-block.ts index df9dc18fc..c57153e26 100644 --- a/apps/shared/src/chart-block.ts +++ b/apps/shared/src/chart-block.ts @@ -11,7 +11,7 @@ export function escapeSingleQuotedStoryAttr(value: string): string { } export type StoryChartBlockInput = Pick< - displayChart.ChartInput, + displayChart.KpiCardInput, | 'query_id' | 'chart_type' | 'x_axis_type' @@ -21,8 +21,8 @@ export type StoryChartBlockInput = Pick< | 'show_data_labels' | 'comparison_mode' > & { - title?: displayChart.ChartInput['title']; - x_axis_key?: displayChart.ChartInput['x_axis_key']; + title?: displayChart.KpiCardInput['title']; + x_axis_key?: displayChart.KpiCardInput['x_axis_key']; }; export function buildStoryChartBlock(input: StoryChartBlockInput): string { diff --git a/apps/shared/src/tools/display-chart.ts b/apps/shared/src/tools/display-chart.ts index 1857d73b6..0d2487dc3 100644 --- a/apps/shared/src/tools/display-chart.ts +++ b/apps/shared/src/tools/display-chart.ts @@ -20,6 +20,9 @@ export const XAxisTypeEnum = z.enum(['date', 'number', 'category']); export const ComparisonModeEnum = z.enum(['percentage', 'variation', 'absolute', 'none']); export type ComparisonMode = z.infer; +const COMPARISON_MODE_DESCRIPTION = + 'KPI cards only: shows a change pill comparing the latest value to the previous period ("percentage", "variation", "absolute", or "none" to hide). Requires the query to return 2+ time-ordered rows (oldest → newest).'; + export const SeriesConfigSchema = z.object({ data_key: z.string().describe('Column name from SQL result to plot.'), color: z.string().describe('CSS color (defaults to theme colors).').optional(), @@ -113,9 +116,6 @@ const ChartInputObjectSchema = z.object({ 'Show the numeric value of each data point directly on the chart. Set to true when the user asks to display values/data labels on the chart.', ) .optional(), - comparison_mode: ComparisonModeEnum.describe( - 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', - ).optional(), title: z .string() .describe( @@ -138,6 +138,7 @@ const KpiCardInputSchema = ChartInputObjectSchema.extend({ 'Use "date" only when x-axis values parse as JS Date (YYYY-MM-DD). Use "category" for quarter_ending, fiscal periods, or labels. Use "number" for numeric x-axis.', ) .optional(), + comparison_mode: ComparisonModeEnum.describe(COMPARISON_MODE_DESCRIPTION).optional(), }).refine(yAxisBoundsValid, Y_AXIS_BOUNDS_MESSAGE); export const TableInputSchema = z.object({ @@ -148,8 +149,9 @@ export const TableInputSchema = z.object({ }); export type ChartInput = z.infer; +export type KpiCardInput = z.infer; export type TableInput = z.infer; -export type Input = ChartInput | TableInput; +export type Input = ChartInput | KpiCardInput | TableInput; const DisplayTypeEnum = z.enum([...ChartTypeEnum.options, 'table']); @@ -167,9 +169,7 @@ const BaseInputSchema = z.object({ .min(1) .describe('Columns to plot as data series. Required for charts and omitted for tables.') .optional(), - comparison_mode: ComparisonModeEnum.describe( - 'KPI cards only. Shows a small pill under the number with the change vs the previous period — the latest row compared to the row immediately before it. Modes: "percentage" = percent change (arrow + color); "variation" = absolute change (arrow + color); "absolute" = magnitude only (no arrow/color); "none" = no pill. When a KPI metric is worth tracking over time (revenue, orders, active users, conversion, etc.), write the SQL to return that metric across at least two consecutive, time-ordered periods (one row per period, ordered oldest→newest) and set this to "percentage" so the card shows the latest period and its change vs the prior one; use "variation"/"absolute" when a raw change reads better than a percent. Omit or use "none" for all-time totals, static rates, or metrics with no meaningful previous period. A pill only renders when the query returns 2+ rows; a single-row (single number) query shows no pill.', - ).optional(), + comparison_mode: ComparisonModeEnum.describe(COMPARISON_MODE_DESCRIPTION).optional(), title: z.string().describe('A concise, descriptive title for the visualization. Required for charts.').optional(), conditional_formats: ColumnConditionalFormatsSchema.describe( 'Conditional formatting rules for table columns. Only used when chart_type is "table".', diff --git a/apps/shared/tests/story-table-block.test.ts b/apps/shared/tests/story-table-block.test.ts index c992f7ced..c1c8c91e0 100644 --- a/apps/shared/tests/story-table-block.test.ts +++ b/apps/shared/tests/story-table-block.test.ts @@ -182,7 +182,7 @@ describe('displayChart.InputSchema table variant', () => { expect(block).toContain('chart_type="kpi_card"'); }); - it('retains KPI comparison mode in chart schemas', () => { + it('retains KPI comparison mode only in the dispatched KPI schema', () => { const input = { query_id: 'query_1', chart_type: 'kpi_card' as const, @@ -193,10 +193,11 @@ describe('displayChart.InputSchema table variant', () => { comparison_mode: 'percentage' as const, }; - expect(displayChart.ChartInputSchema.safeParse(input)).toMatchObject({ - success: true, - data: { comparison_mode: 'percentage' }, - }); + const chartResult = displayChart.ChartInputSchema.safeParse(input); + expect(chartResult.success).toBe(true); + if (chartResult.success) { + expect(chartResult.data).not.toHaveProperty('comparison_mode'); + } expect(displayChart.InputSchema.safeParse(input)).toMatchObject({ success: true, data: { comparison_mode: 'percentage' },