feat(charts): add dual-axis combo chart mode#943
Closed
Bl3f wants to merge 2 commits into
Closed
Conversation
Adds a 'combo' chart type that mixes bar/line/area series across two independently configurable Y-axes. - schema: combo chart_type, per-series series_type + y_axis, top-level y_axes (label + domain) config - renderer: buildComboChart using recharts ComposedChart with optional right YAxis, per-series geometry, and axis labels/domains - propagate y_axes/series fields through story blocks, parsing, validation, MCP embed, PNG/SVG export and story HTML - edit dialog: combo option, per-series type/axis selectors, axis labels - system prompt guidance for combo/dual-axis - tests for combo SVG rendering and story round-trip
Contributor
🧹 Preview RemovedThe preview deployment for this PR has been cleaned up. |
Dual-axis combo charts plot series on different scales, so summing them into a 'Total' row is meaningless. Skip the total in both the React tooltip and the exported story HTML tooltip when chart_type is combo.
Bl3f
marked this pull request as ready for review
July 2, 2026 14:45
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 20 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/shared/src/story-segments.ts">
<violation number="1" location="apps/shared/src/story-segments.ts:3">
P3: Parsed combo story series can omit `color`, but `ParsedChartSeries.color` is typed as required. This makes downstream code believe a parsed value is always a string even though generated combo blocks and the display-chart schema allow it to be absent; `color?: string` would match runtime behavior.</violation>
<violation number="2" location="apps/shared/src/story-segments.ts:82">
P2: Malformed `y_axes` story attributes can now reach chart rendering as valid axis config because the parser only checks that the JSON is an object before casting. Consider validating `left/right.domain` as either `'auto'` or numeric `{ min, max }` before returning `yAxes`, so edited stories fail safely instead of passing undefined bounds into Recharts.</violation>
</file>
<file name="apps/shared/src/tools/display-chart.ts">
<violation number="1" location="apps/shared/src/tools/display-chart.ts:37">
P2: Manual axis ranges can validate even when `min` is greater than `max`, which can produce inverted or broken Y-axis scaling at render time. Adding a schema refinement for `min <= max` would reject invalid domain configs early.</violation>
</file>
<file name="apps/shared/src/story-validation.ts">
<violation number="1" location="apps/shared/src/story-validation.ts:23">
P2: Story validation doesn't check combo-specific series fields (`series_type`, `y_axis`). When `chart_type="combo"`, the validator could reject invalid `series_type` values (must be "bar"|"line"|"area") and `y_axis` values (must be "left"|"right"). Without this, story authors get no editor feedback for typos or invalid combo config — the chart would silently apply defaults or potentially misrender.</violation>
</file>
<file name="apps/shared/src/chart-builder.tsx">
<violation number="1" location="apps/shared/src/chart-builder.tsx:319">
P2: Area fills can bleed across multiple combo charts because gradient IDs are only index-based and not chart-scoped. Consider generating per-chart-unique gradient IDs before wiring `id`/`fill` so each chart’s SVG defs remain isolated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| xAxisKey: attrs.x_axis_key, | ||
| xAxisType: attrs.x_axis_type || null, | ||
| series, | ||
| yAxes: attrs.y_axes ? (tryParseJsonObject(attrs.y_axes) as ParsedYAxesConfig | undefined) : undefined, |
Contributor
There was a problem hiding this comment.
P2: Malformed y_axes story attributes can now reach chart rendering as valid axis config because the parser only checks that the JSON is an object before casting. Consider validating left/right.domain as either 'auto' or numeric { min, max } before returning yAxes, so edited stories fail safely instead of passing undefined bounds into Recharts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/story-segments.ts, line 82:
<comment>Malformed `y_axes` story attributes can now reach chart rendering as valid axis config because the parser only checks that the JSON is an object before casting. Consider validating `left/right.domain` as either `'auto'` or numeric `{ min, max }` before returning `yAxes`, so edited stories fail safely instead of passing undefined bounds into Recharts.</comment>
<file context>
@@ -60,10 +79,20 @@ export function parseChartBlock(attrString: string): ParsedChartBlock | null {
xAxisKey: attrs.x_axis_key,
xAxisType: attrs.x_axis_type || null,
series,
+ yAxes: attrs.y_axes ? (tryParseJsonObject(attrs.y_axes) as ParsedYAxesConfig | undefined) : undefined,
title: attrs.title || '',
};
</file context>
Comment on lines
+37
to
+40
| z.object({ | ||
| min: z.number().describe('Minimum value of the axis scale.'), | ||
| max: z.number().describe('Maximum value of the axis scale.'), | ||
| }), |
Contributor
There was a problem hiding this comment.
P2: Manual axis ranges can validate even when min is greater than max, which can produce inverted or broken Y-axis scaling at render time. Adding a schema refinement for min <= max would reject invalid domain configs early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/tools/display-chart.ts, line 37:
<comment>Manual axis ranges can validate even when `min` is greater than `max`, which can produce inverted or broken Y-axis scaling at render time. Adding a schema refinement for `min <= max` would reject invalid domain configs early.</comment>
<file context>
@@ -10,16 +10,49 @@ export const ChartTypeEnum = z.enum([
+export const YAxisDomainSchema = z
+ .union([
+ z.literal('auto'),
+ z.object({
+ min: z.number().describe('Minimum value of the axis scale.'),
+ max: z.number().describe('Maximum value of the axis scale.'),
</file context>
Suggested change
| z.object({ | |
| min: z.number().describe('Minimum value of the axis scale.'), | |
| max: z.number().describe('Maximum value of the axis scale.'), | |
| }), | |
| z.object({ | |
| min: z.number().describe('Minimum value of the axis scale.'), | |
| max: z.number().describe('Maximum value of the axis scale.'), | |
| }).refine((domain) => domain.min <= domain.max, { | |
| message: 'Axis domain requires min <= max.', | |
| }), |
| 'kpi_card', | ||
| 'scatter', | ||
| 'radar', | ||
| 'combo', |
Contributor
There was a problem hiding this comment.
P2: Story validation doesn't check combo-specific series fields (series_type, y_axis). When chart_type="combo", the validator could reject invalid series_type values (must be "bar"|"line"|"area") and y_axis values (must be "left"|"right"). Without this, story authors get no editor feedback for typos or invalid combo config — the chart would silently apply defaults or potentially misrender.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/story-validation.ts, line 23:
<comment>Story validation doesn't check combo-specific series fields (`series_type`, `y_axis`). When `chart_type="combo"`, the validator could reject invalid `series_type` values (must be "bar"|"line"|"area") and `y_axis` values (must be "left"|"right"). Without this, story authors get no editor feedback for typos or invalid combo config — the chart would silently apply defaults or potentially misrender.</comment>
<file context>
@@ -20,6 +20,7 @@ const VALID_CHART_TYPES = new Set([
'kpi_card',
'scatter',
'radar',
+ 'combo',
]);
</file context>
| <defs> | ||
| {series.map((s, i) => | ||
| s.series_type === 'area' ? ( | ||
| <linearGradient key={s.data_key} id={`grad-combo-${i}`} x1='0' y1='0' x2='0' y2='1'> |
Contributor
There was a problem hiding this comment.
P2: Area fills can bleed across multiple combo charts because gradient IDs are only index-based and not chart-scoped. Consider generating per-chart-unique gradient IDs before wiring id/fill so each chart’s SVG defs remain isolated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/chart-builder.tsx, line 319:
<comment>Area fills can bleed across multiple combo charts because gradient IDs are only index-based and not chart-scoped. Consider generating per-chart-unique gradient IDs before wiring `id`/`fill` so each chart’s SVG defs remain isolated.</comment>
<file context>
@@ -286,6 +292,139 @@ function buildAreaChart(props: ResolvedProps) {
+ <defs>
+ {series.map((s, i) =>
+ s.series_type === 'area' ? (
+ <linearGradient key={s.data_key} id={`grad-combo-${i}`} x1='0' y1='0' x2='0' y2='1'>
+ <stop offset='0%' stopColor={colorFor(s.data_key, i)} stopOpacity={0.25} />
+ <stop offset='100%' stopColor={colorFor(s.data_key, i)} stopOpacity={0} />
</file context>
| @@ -1,9 +1,28 @@ | |||
| export interface ParsedChartSeries { | |||
| data_key: string; | |||
| color: string; | |||
Contributor
There was a problem hiding this comment.
P3: Parsed combo story series can omit color, but ParsedChartSeries.color is typed as required. This makes downstream code believe a parsed value is always a string even though generated combo blocks and the display-chart schema allow it to be absent; color?: string would match runtime behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/story-segments.ts, line 3:
<comment>Parsed combo story series can omit `color`, but `ParsedChartSeries.color` is typed as required. This makes downstream code believe a parsed value is always a string even though generated combo blocks and the display-chart schema allow it to be absent; `color?: string` would match runtime behavior.</comment>
<file context>
@@ -1,9 +1,28 @@
+export interface ParsedChartSeries {
+ data_key: string;
+ color: string;
+ label?: string;
+ series_type?: 'bar' | 'line' | 'area';
</file context>
socallmebertille
requested changes
Jul 3, 2026
socallmebertille
left a comment
socallmebertille
left a comment
Contributor
There was a problem hiding this comment.
I think this PR adds real value and it LGTM for the most part :
- I just left a tiny comment below
- and could you also take a look at the Cubic comments that seem valid to me
Comment on lines
+328
to
+337
| <YAxis | ||
| yAxisId='left' | ||
| tick={AXIS_TICK} | ||
| tickLine={false} | ||
| axisLine={false} | ||
| minTickGap={12} | ||
| tickFormatter={formatYAxisTick} | ||
| domain={resolveAxisDomain(yAxes?.left?.domain)} | ||
| label={axisLabel(yAxes?.left?.label, 'left')} | ||
| /> |
Contributor
There was a problem hiding this comment.
The left YAxis is always rendered, so if all series are assigned to the right axis we still draw an empty left axis. So, you should make the left axis conditional too, symmetric to the right one OR add a guard in the edit dialog preventing the user from moving the last remaining left-axis series to the right (keeps at least one series on the left).
Contributor
Author
|
cf. #1241 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements dual-axis charts with mixed bar + line (+ area) series, closing #873.
Adds a new
combochart type todisplay_chartin a backward-compatible way. Existing single-axis charts are unchanged; the work is concentrated in the shared schema and the singlebuildChartrenderer, so chat, stories, PNG export, and MCP embeds all stay in sync.Walkthrough
dual_axis_combo_chart_tooltip_demo.mp4
Combo chart: dark-teal bars (Revenue, left axis) and an orange line (conversion rate, right axis), each with its own labeled Y-axis; tooltips show both series with no misleading cross-axis "Total".
Dual-axis combo chart
How this maps to the issue's expected behavior
y_axis: "left" | "right"(defaultleft). A rightYAxisis rendered whenever any series usesright.chart_type: "combo"; each series setsseries_type: "bar" | "line" | "area".y_axison eachseries[]entry.y_axes.left/y_axes.rightwithlabelanddomain("auto"or{ min, max }).Example tool input
{ "query_id": "query_…", "chart_type": "combo", "x_axis_key": "month", "x_axis_type": "date", "title": "Monthly revenue vs order count", "y_axes": { "left": { "label": "Revenue (USD)" }, "right": { "label": "Orders" } }, "series": [ { "data_key": "total_revenue", "series_type": "bar", "y_axis": "left" }, { "data_key": "order_count", "series_type": "line", "y_axis": "right" } ] }Changes
apps/shared/src/tools/display-chart.ts):combochart type, per-seriesseries_type+y_axis, top-levely_axes(label + domain).apps/shared/src/chart-builder.tsx):buildComboChartusing rechartsComposedChartwith an optional rightYAxis, per-series Bar/Line/Area geometry, and configurable axis labels/domains.y_axesand the new series fields through story<chart>blocks, MCP embed config, PNG/SVG export, and story HTML export.Testing
npm run lint— tsc + eslint across all workspaces (pass)npm run -w @nao/shared test— 74 pass (incl. combo story round-trip)npm run -w @nao/backend test -- generate-chart-combo— 5 passTo show artifacts inline, enable in settings.