Skip to content

feat : display_chart locale-aware dates and state row filters#741

Open
nehaaprasaad wants to merge 1 commit intogetnao:mainfrom
nehaaprasaad:fix/dat-writ-amer-lst
Open

feat : display_chart locale-aware dates and state row filters#741
nehaaprasaad wants to merge 1 commit intogetnao:mainfrom
nehaaprasaad:fix/dat-writ-amer-lst

Conversation

@nehaaprasaad
Copy link
Copy Markdown
Contributor

@nehaaprasaad nehaaprasaad commented May 7, 2026

summary

  • Improves display_chart: locale-aware ISO dates and optional row filters.

Changes

  • Default date labels follow viewer locale; optional date_locale to force one.
  • Optional filter_state_types / filter_state_names (AND); case-insensitive column keys.
  • Same behaviour in UI, PNG download, messaging chart images, and stories.
  • Clear message when filters or date range leave no rows.

Test plan

  • npm run lint

fix : #734

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🚀 Preview Deployment

URL https://pr-741-9092bd9.preview.getnao.io
Commit 9092bd9

⚠️ No LLM API keys configured - you'll see the API key setup flow when trying to chat.


Preview will be automatically removed when this PR is closed.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 9 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:72">
P2: Unvalidated `date_locale` can propagate an invalid locale into `toLocaleDateString`, which may throw at render time.</violation>
</file>

<file name="apps/frontend/src/components/tool-calls/display-chart.tsx">

<violation number="1" location="apps/frontend/src/components/tool-calls/display-chart.tsx:349">
P2: Unvalidated `date_locale` is forwarded into shared date formatting, which can throw on invalid locale strings and break chart rendering.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


const filterStateTypes = tryParseStringArray(attrs.filter_state_types);
const filterStateNames = tryParseStringArray(attrs.filter_state_names);
const dateLocale = attrs.date_locale?.trim() || undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Unvalidated date_locale can propagate an invalid locale into toLocaleDateString, which may throw at render time.

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 72:

<comment>Unvalidated `date_locale` can propagate an invalid locale into `toLocaleDateString`, which may throw at render time.</comment>

<file context>
@@ -52,13 +67,20 @@ export function parseChartBlock(attrString: string): ParsedChartBlock | null {
 
+	const filterStateTypes = tryParseStringArray(attrs.filter_state_types);
+	const filterStateNames = tryParseStringArray(attrs.filter_state_names);
+	const dateLocale = attrs.date_locale?.trim() || undefined;
+
 	return {
</file context>

series: visibleSeries,
colorFor,
labelFormatter: xAxisLabelFormatter,
dateLocale: chartDateLocale,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Unvalidated date_locale is forwarded into shared date formatting, which can throw on invalid locale strings and break chart rendering.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart.tsx, line 349:

<comment>Unvalidated `date_locale` is forwarded into shared date formatting, which can throw on invalid locale strings and break chart rendering.</comment>

<file context>
@@ -314,6 +346,7 @@ export const ChartDisplay = memo(function ChartDisplay({
 				series: visibleSeries,
 				colorFor,
 				labelFormatter: xAxisLabelFormatter,
+				dateLocale: chartDateLocale,
 				showGrid,
 				margin: { top: 0, right: 0, bottom: 0, left: 0 },
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dates are written the American way by default (should be a param at least)

1 participant