Fix ECharts/Chart.js rendering bugs found in MCP bug-bash#42
Open
Chenglong-MS wants to merge 1 commit into
Open
Fix ECharts/Chart.js rendering bugs found in MCP bug-bash#42Chenglong-MS wants to merge 1 commit into
Chenglong-MS wants to merge 1 commit into
Conversation
Fixes four rendering bugs surfaced while exercising flint-chart-mcp as an external user, with regression tests for each. - ECharts temporal x-axis (line/area/streamgraph): the `time` axis parser only accepts ISO-8601 strings, so common human-readable dates like "Jan 1 2000" (e.g. vega-datasets stocks.csv) silently dropped every point and the chart rendered blank. Pre-convert temporal x-values to epoch-ms via a shared `toEChartsTemporal()` helper. - ECharts heatmap labels: cell labels showed raw full-precision floats (e.g. 30.46666700000002) and string-sliced them when narrow, producing overlapping, illegible text. Always round to a width-appropriate number of decimals, fall back to an integer, and hide only if it still overflows. - Null -> 0 coercion (ECharts boxplot, Chart.js bubble): `Number(null)` is 0, so rows with a missing measurement injected phantom values at 0 (a bogus boxplot whisker/outlier at 0, and bubbles piled at the origin). Drop null/empty cells before numeric coercion instead. Verified: 371 flint-js tests pass, typecheck clean, site build succeeds, and the four scenarios were re-rendered and visually confirmed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Fixes four rendering bugs surfaced while exercising
flint-chart-mcpas an external user in a bug-bash, each with a regression test.timeaxis parser only accepts ISO-8601 strings, so human-readable dates like"Jan 1 2000"(e.g. vega-datasetsstocks.csv) silently dropped every point and the chart rendered blank.toEChartsTemporal()helper.30.46666700000002) and string-sliced them when narrow, producing overlapping, illegible text.Number(null) === 0, so rows with a missing measurement injected a spurious value at 0 (bogus whisker / outlier at 0).(0, 0).Testing
npm run test -w packages/flint-js- 371 tests pass (3 new regression test files added).npm run typecheck -w packages/flint-js- clean.npm run build -w site- succeeds (galleries unaffected).Notes / out of scope
canvasSizehard-ceiling overflow observed in the bug-bash is layout-level and intentional incompute-layout; any actual PNG overflow lives in the MCP renderer and carries high gallery-regression risk, so it is left for a separate, focused change.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com