fix: make display_chart series color optional to prevent chart loading failure#458
Open
leoncuhk wants to merge 1 commit intogetnao:mainfrom
Open
fix: make display_chart series color optional to prevent chart loading failure#458leoncuhk wants to merge 1 commit intogetnao:mainfrom
leoncuhk wants to merge 1 commit intogetnao:mainfrom
Conversation
…g failure LLMs frequently omit the `color` field in `display_chart` tool calls, causing Zod validation to reject the input with "Required" error. The chart gets stuck in "Loading chart" state permanently. - Make `SeriesConfigSchema.color` optional with default `'auto'` - Frontend already had fallback logic (`s.color || Colors[...]`) but it was never reached because validation failed first - Handle `'auto'` sentinel in frontend color resolution Author: Leon <leoncuhk@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes display_chart tool-call validation failures when LLMs omit series[].color, preventing the UI from getting stuck in a perpetual “Loading chart” state by allowing a theme-based fallback color to be used.
Changes:
- Update the shared Zod schema to accept missing
series[].colorby making it optional and defaulting it to'auto'. - Update the frontend chart rendering to treat
'auto'as “use theme colors” (for both chart config colors and legend payload colors).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/shared/src/tools/display-chart.ts | Makes SeriesConfigSchema.color optional and defaults it to 'auto' to avoid tool input validation failures. |
| apps/frontend/src/components/tool-calls/display-chart.tsx | Adds explicit handling of the 'auto' sentinel so the chart/legend fall back to theme colors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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).'), | ||
| color: z.string().describe('CSS color (defaults to theme colors).').optional().default('auto'), |
| 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).'), | ||
| color: z.string().describe('CSS color (defaults to theme colors).').optional().default('auto'), |
Comment on lines
268
to
270
| label: s.label || labelize(s.data_key), | ||
| color: s.color || Colors[idx % Colors.length], | ||
| color: (s.color && s.color !== 'auto') ? s.color : Colors[idx % Colors.length], | ||
| }; |
Comment on lines
286
to
289
| value: s.label || labelize(s.data_key), | ||
| dataKey: s.data_key, | ||
| color: s.color || Colors[idx % Colors.length], | ||
| color: (s.color && s.color !== 'auto') ? s.color : Colors[idx % Colors.length], | ||
| isHidden: hiddenSeriesKeys.has(s.data_key), |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Contributor
🚀 Preview Deployment
Preview will be automatically removed when this PR is closed. |
Bl3f
reviewed
Mar 17, 2026
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
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.
Problem
LLMs frequently omit the
colorfield when calling thedisplay_charttool — theseriesarray isgenerated as
[{"data_key": "...", "label": "..."}]without acolorproperty.Since
SeriesConfigSchema.coloris currently required, Zod validation rejects the input with:Invalid input for tool display_chart: Type validation failed
"series", 0, "color" — "expected": "string", "received": "undefined"
The chart component gets stuck in "Loading chart" state permanently because the tool state never
transitions from
input-streamingtoresult.Root Cause
The
colorfield description says "defaults to theme colors" but the schema doesn't actually providea default — it's a required string. The frontend already has fallback logic (
s.color || Colors[idx % Colors.length]) but it's never reached because validation fails first.Fix
apps/shared/src/tools/display-chart.ts: Makecoloroptional with.optional().default('auto')apps/frontend/src/components/tool-calls/display-chart.tsx: Handle'auto'sentinel value,falling back to theme colors (2 locations: chart config + legend payload)
Author: Leon leoncuhk@gmail.com