Skip to content

fix: preserve numeric and text types in CSV charts - #10869

Open
Light2Dark wants to merge 4 commits into
codex/fix-dotted-chart-columnsfrom
codex/parse-chart-csv-column-types
Open

Light2Dark wants to merge 4 commits into
codex/fix-dotted-chart-columnsfrom
codex/parse-chart-csv-column-types

Conversation

@Light2Dark

@Light2Dark Light2Dark commented Sep 16, 2026

Copy link
Copy Markdown
Member

This pull request was authored by a coding agent.

📝 Summary

  • Stacked on fix: render dotted chart columns when data falls back to CSV #10856; this PR targets codex/fix-dotted-chart-columns.
  • Use backend column metadata to parse CSV charts so numeric inf, +inf, and -inf remain distinct from missing values, while text such as inf and 001 keeps its original representation.
  • Retain full-CSV automatic inference when backend metadata is absent. Sample-inferred types only guide the chart controls.
  • Avoid unnecessary refetches for equivalent schemas while reparsing when column types change.
  • Preserve chart date parsing and the table loader's existing BigInt and number-like string handling.

Screenshot

  • User-provided screenshot from the parent PR showing dotted user.name labels rendering correctly. This image illustrates the parent fix.

Chart with dotted user.name column showing alice and bob

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions.
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Written by GPT-6 on Codex

@vercel

vercel Bot commented Sep 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
marimo-docs Ready Ready Preview Sep 16, 2026 5:32pm UTC

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for ./frontend

Status Category Percentage Covered / Total
🔵 Lines 58.24% 22181 / 38085
🔵 Statements 58.16% 22567 / 38796
🔵 Functions 51.65% 5038 / 9753
🔵 Branches 51.8% 11602 / 22396
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
frontend/src/components/data-table/charts/charts.tsx 52.05% 53.26% 42.42% 53.19% 115-123, 128, 132-149, 153-168, 182-195, 199-221, 235, 250-251, 271-275, 278, 344, 349, 354, 356, 394, 397-410, 498-499, 522, 561-562, 568, 570, 576
frontend/src/plugins/impl/vega/loader.ts 93.33% 89.61% 100% 93.1% 45, 48, 51, 58, 76, 106, 117, 121-122, 202
frontend/src/plugins/impl/vega/utils.ts 70.83% 82.14% 60% 70.83% 33-43, 66
Generated in workflow #21523 for commit 3fc0b55 by the Vitest Coverage Report Action

@Light2Dark Light2Dark added the bug Something isn't working label Sep 16, 2026

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

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.

Review completed against the latest diff

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread frontend/src/plugins/impl/vega/loader.ts
Comment thread frontend/src/components/data-table/charts/charts.tsx Outdated
Comment thread frontend/src/components/data-table/charts/charts.tsx

Copilot AI left a comment

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.

🟡 Changes recommended

Schema-less lazy tables incorrectly use sample-derived explicit parsing instead of full-CSV automatic inference.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds schema-aware CSV parsing for chart data while preserving text values and supporting infinities.

Changes:

  • Adds numeric infinity and opt-in date parsing.
  • Passes table column types into chart CSV loading.
  • Expands loader and ChartPanel tests.
File summaries
File Description
frontend/src/plugins/impl/vega/utils.ts Adds configurable date parsing.
frontend/src/plugins/impl/vega/loader.ts Parses numeric infinity values.
frontend/src/plugins/impl/vega/__tests__/vega.test.ts Tests typed CSV parsing.
frontend/src/plugins/impl/vega/__tests__/utils.test.ts Tests date parsing options.
frontend/src/components/data-table/charts/charts.tsx Applies field metadata to CSV charts.
frontend/src/components/data-table/charts/__tests__/chart-panel.test.tsx Tests chart loading and schema changes.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/src/components/data-table/charts/charts.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants