From 38e788f031400c939b504646a547e08441720f81 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 1 Jul 2026 00:19:44 +0000 Subject: [PATCH] Fix ECharts/Chart.js rendering bugs found in MCP bug-bash 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> --- .../flint-js/src/chartjs/templates/bubble.ts | 21 +++-- .../flint-js/src/echarts/templates/area.ts | 19 ++-- .../flint-js/src/echarts/templates/boxplot.ts | 20 +++- .../flint-js/src/echarts/templates/heatmap.ts | 40 ++++++-- .../flint-js/src/echarts/templates/line.ts | 14 ++- .../src/echarts/templates/streamgraph.ts | 14 ++- .../flint-js/src/echarts/templates/utils.ts | 20 ++++ .../tests/echarts-temporal-dates.test.ts | 77 +++++++++++++++ .../tests/heatmap-label-rounding.test.ts | 69 ++++++++++++++ .../tests/null-values-dropped.test.ts | 93 +++++++++++++++++++ 10 files changed, 353 insertions(+), 34 deletions(-) create mode 100644 packages/flint-js/tests/echarts-temporal-dates.test.ts create mode 100644 packages/flint-js/tests/heatmap-label-rounding.test.ts create mode 100644 packages/flint-js/tests/null-values-dropped.test.ts diff --git a/packages/flint-js/src/chartjs/templates/bubble.ts b/packages/flint-js/src/chartjs/templates/bubble.ts index 2d7a64bd..de566913 100644 --- a/packages/flint-js/src/chartjs/templates/bubble.ts +++ b/packages/flint-js/src/chartjs/templates/bubble.ts @@ -67,17 +67,24 @@ export const cjsBubbleChartDef: ChartTemplateDef = { const palette = getChartJsPalette(ctx, 'color'); // Radius scale spans the whole dataset so groups stay comparable. + // Treat null/empty cells as missing (NaN) rather than coercing to 0. + const num = (raw: any): number => (raw == null || raw === '' ? NaN : Number(raw)); const sizeValues = sizeField - ? table.map((row) => Number(row[sizeField])) + ? table.map((row) => num(row[sizeField])) : []; // Provisional radius range; postProcess refines rMax to canvas size. const radiusScale = makeRadiusScale(sizeValues, 5, 24); const toPoint = (row: any) => { - const v = sizeField ? Number(row[sizeField]) : NaN; + const x = num(row[xField]); + const y = num(row[yField]); + // Drop points with a missing/non-numeric x or y so null values don't + // collapse to phantom bubbles at the origin (0, 0). + if (!isFinite(x) || !isFinite(y)) return null; + const v = sizeField ? num(row[sizeField]) : NaN; return { - x: Number(row[xField]), - y: Number(row[yField]), + x, + y, r: sizeField ? radiusScale(v) : 8, // Raw size value retained so postProcess can rescale to canvas. _v: v, @@ -106,9 +113,11 @@ export const cjsBubbleChartDef: ChartTemplateDef = { if (colorField) { const groups = new Map(); for (const row of table) { + const point = toPoint(row); + if (!point) continue; const key = String(row[colorField] ?? ''); if (!groups.has(key)) groups.set(key, []); - groups.get(key)!.push(toPoint(row)); + groups.get(key)!.push(point); } let colorIdx = 0; for (const [name, data] of groups) { @@ -124,7 +133,7 @@ export const cjsBubbleChartDef: ChartTemplateDef = { config.options.plugins.legend = { display: true }; } else { config.data.datasets.push({ - data: table.map(toPoint), + data: table.map(toPoint).filter((p) => p !== null), backgroundColor: getSeriesBackgroundColor(palette, 0, opacity), borderColor: getSeriesBorderColor(palette, 0), borderWidth: 1, diff --git a/packages/flint-js/src/echarts/templates/area.ts b/packages/flint-js/src/echarts/templates/area.ts index 4ba168ed..b9e0bee8 100644 --- a/packages/flint-js/src/echarts/templates/area.ts +++ b/packages/flint-js/src/echarts/templates/area.ts @@ -10,7 +10,7 @@ */ import { ChartTemplateDef, ChartPropertyDef } from '../../core/types'; -import { extractCategories, groupBy, getCategoryOrder } from './utils'; +import { extractCategories, groupBy, getCategoryOrder, toEChartsTemporal } from './utils'; import { getPaletteForScheme } from '../colormap'; const isDiscrete = (type: string | undefined) => type === 'nominal' || type === 'ordinal'; @@ -49,6 +49,10 @@ export const ecAreaChartDef: ChartTemplateDef = { const xIsTemporal = xCS.type === 'temporal'; const yIsDiscrete = isDiscrete(yCS.type); const isContinuousColor = !!colorField && (colorType === 'quantitative' || colorType === 'temporal'); + + // ECharts' `time` axis can't parse non-ISO date strings; pre-convert temporal + // x-values to epoch-ms so points on a time axis always render. + const xVal = (v: any) => (xIsTemporal ? toEChartsTemporal(v) : v); const categories = xIsDiscrete ? extractCategories(table, xField, getCategoryOrder(ctx, 'x')) : undefined; @@ -133,8 +137,8 @@ export const ecAreaChartDef: ChartTemplateDef = { return String(ax).localeCompare(String(bx)); }); - const pointData = sorted.map((r: any) => [r[xField], r[yField], r[colorField]]); - const lineData = sorted.map((r: any) => [r[xField], r[yField]]); + const pointData = sorted.map((r: any) => [xVal(r[xField]), r[yField], r[colorField]]); + const lineData = sorted.map((r: any) => [xVal(r[xField]), r[yField]]); const nums = sorted .map((r: any) => Number(r[colorField])) @@ -242,7 +246,7 @@ export const ecAreaChartDef: ChartTemplateDef = { ? buildValueAlignedYData(rows, xField, yField, sortedX) : sortedDates ? buildTimeAlignedData(rows, xField, yField, sortedDates) - : rows.map(r => [r[xField], r[yField]]); + : rows.map(r => [xVal(r[xField]), r[yField]]); const series: any = { name, @@ -271,7 +275,7 @@ export const ecAreaChartDef: ChartTemplateDef = { : xIsTemporal ? (() => { const sorted = [...table].sort((a, b) => new Date(a[xField]).getTime() - new Date(b[xField]).getTime()); - return sorted.map(r => [r[xField], r[yField]]); + return sorted.map(r => [xVal(r[xField]), r[yField]]); })() : table.map(r => [r[xField], r[yField]]); @@ -396,11 +400,12 @@ function buildTimeAlignedData( xField: string, yField: string, sortedDates: string[], -): Array<[string, number]> { +): Array<[number | string, number]> { const map = new Map(); for (const row of rows) { const n = Number(row[yField]); map.set(String(row[xField]), Number.isFinite(n) ? n : 0); } - return sortedDates.map(d => [d, map.get(d) ?? 0]); + // Emit epoch-ms for the time axis so non-ISO date strings still plot. + return sortedDates.map(d => [toEChartsTemporal(d) as number | string, map.get(d) ?? 0]); } diff --git a/packages/flint-js/src/echarts/templates/boxplot.ts b/packages/flint-js/src/echarts/templates/boxplot.ts index c7540761..ea14bdf5 100644 --- a/packages/flint-js/src/echarts/templates/boxplot.ts +++ b/packages/flint-js/src/echarts/templates/boxplot.ts @@ -28,6 +28,22 @@ function areCategoriesNumeric(cats: string[]): boolean { }); } +/** + * Extract finite numeric values from rows for a field, dropping null/undefined/empty + * cells instead of coercing them. `Number(null)` is `0`, which would otherwise inject + * spurious zeros (and phantom outliers at 0) for rows with missing measurements. + */ +function numericValues(rows: any[], field: string): number[] { + const out: number[] = []; + for (const r of rows) { + const raw = r[field]; + if (raw == null || raw === '') continue; + const n = Number(raw); + if (isFinite(n)) out.push(n); + } + return out; +} + /** Compute the five-number summary for an array of values. */ function fiveNumberSummary( values: number[], @@ -167,7 +183,7 @@ export const ecBoxplotDef: ChartTemplateDef = { const rows = (catGroups.get(cat) || []).filter( (r: any) => String(r[colorField] ?? '') === colorName, ); - const values = rows.map((r: any) => Number(r[valField])).filter(v => isFinite(v)); + const values = numericValues(rows, valField); boxData.push(fiveNumberSummary(values, whiskerMethod)); if (showOutliers) { @@ -205,7 +221,7 @@ export const ecBoxplotDef: ChartTemplateDef = { for (let i = 0; i < categories.length; i++) { const cat = categories[i]; const rows = catGroups.get(cat) || []; - const values = rows.map((r: any) => Number(r[valField])).filter((v: number) => isFinite(v)); + const values = numericValues(rows, valField); boxData.push(fiveNumberSummary(values, whiskerMethod)); if (showOutliers) { diff --git a/packages/flint-js/src/echarts/templates/heatmap.ts b/packages/flint-js/src/echarts/templates/heatmap.ts index 5dcaf9b6..55785725 100644 --- a/packages/flint-js/src/echarts/templates/heatmap.ts +++ b/packages/flint-js/src/echarts/templates/heatmap.ts @@ -47,6 +47,24 @@ const SCHEME_COLORS: Record = { const DEFAULT_HEATMAP_SCHEME = 'blues'; +/** + * Format a numeric heatmap cell value for display. + * + * Aggregated cell values are often floats with floating-point noise + * (e.g. `30.46666700000002`). Showing them at full precision makes the labels + * long and overlap into an illegible grid, so we round to a small number of + * decimals scaled to the magnitude. `maxDecimals` can force coarser rounding + * when the cell is narrow. + */ +function formatHeatLabel(val: number, maxDecimals = 2): string { + if (!Number.isFinite(val)) return ''; + if (Number.isInteger(val)) return String(val); + const abs = Math.abs(val); + let decimals = abs >= 100 ? 0 : abs >= 10 ? 1 : 2; + decimals = Math.min(decimals, maxDecimals); + return val.toFixed(decimals); +} + function isDivergingHeatmapScheme(scheme: string | undefined): boolean { return scheme === 'blueorange' || scheme === 'redblue'; } @@ -228,15 +246,19 @@ export const ecHeatmapDef: ChartTemplateDef = { } else { const fontSize = Math.max(8, Math.min(12, Math.round(minDim * 0.2))); heatSeries.label.fontSize = fontSize; - // If cell is narrow, truncate displayed value - if (cellW < 50) { - const maxChars = Math.max(2, Math.floor(cellW / (fontSize * 0.6))); - heatSeries.label.formatter = (params: any) => { - const val = params.data[2]; - const s = String(val); - return s.length > maxChars ? s.slice(0, maxChars) : s; - }; - } + // Always round labels (values are often noisy floats). Pick a decimal + // count that fits the cell width; fall back to integer, then hide if + // even that overflows — this prevents overlapping, illegible labels. + const approxCharW = fontSize * 0.62; + const maxChars = Math.max(2, Math.floor(cellW / approxCharW)); + heatSeries.label.formatter = (params: any) => { + const val = Number(params.data?.[2]); + if (!Number.isFinite(val)) return ''; + let s = formatHeatLabel(val); + if (s.length > maxChars) s = formatHeatLabel(val, 0); + if (s.length > maxChars) s = String(Math.round(val)); + return s.length > maxChars ? '' : s; + }; } } diff --git a/packages/flint-js/src/echarts/templates/line.ts b/packages/flint-js/src/echarts/templates/line.ts index 58545456..ba438f57 100644 --- a/packages/flint-js/src/echarts/templates/line.ts +++ b/packages/flint-js/src/echarts/templates/line.ts @@ -10,7 +10,7 @@ */ import { ChartTemplateDef, ChartPropertyDef } from '../../core/types'; -import { extractCategories, groupBy, getCategoryOrder } from './utils'; +import { extractCategories, groupBy, getCategoryOrder, toEChartsTemporal } from './utils'; import { toTypeString } from '../../core/field-semantics'; import { getPaletteForScheme } from '../colormap'; import { makeCartesianPivot } from '../../core/pivot'; @@ -64,6 +64,10 @@ export const ecLineChartDef: ChartTemplateDef = { const yIsDiscrete = isDiscrete(yCS.type); const isContinuousColor = !!colorField && (colorType === 'quantitative' || colorType === 'temporal'); + // ECharts' `time` axis can't parse non-ISO date strings; pre-convert temporal + // x-values to epoch-ms so points on a time axis always render. + const xVal = (v: any) => (xIsTemporal ? toEChartsTemporal(v) : v); + // Build x-axis categories for discrete/temporal axes const categories = xIsDiscrete ? extractCategories(table, xField, getCategoryOrder(ctx, 'x')) : undefined; const yCategories = yIsDiscrete ? extractCategories(table, yField, getCategoryOrder(ctx, 'y')) : undefined; @@ -154,8 +158,8 @@ export const ecLineChartDef: ChartTemplateDef = { return String(ax).localeCompare(String(bx)); }); - const pointData = sorted.map((r: any) => [r[xField], r[yField], r[colorField]]); - const lineData = sorted.map((r: any) => [r[xField], r[yField]]); + const pointData = sorted.map((r: any) => [xVal(r[xField]), r[yField], r[colorField]]); + const lineData = sorted.map((r: any) => [xVal(r[xField]), r[yField]]); // VisualMap domain const nums = sorted @@ -232,7 +236,7 @@ export const ecLineChartDef: ChartTemplateDef = { ? buildCategoryAlignedXYData(rows, xField, yField, yCategories) : xIsDiscrete ? buildCategoryAlignedData(rows, xField, yField, categories!) - : rows.map(r => [r[xField], r[yField]]); + : rows.map(r => [xVal(r[xField]), r[yField]]); const series: any = { name, @@ -258,7 +262,7 @@ export const ecLineChartDef: ChartTemplateDef = { const row = table.find(r => String(r[xField]) === cat); return row ? row[yField] : null; }) - : table.map(r => [r[xField], r[yField]]); + : table.map(r => [xVal(r[xField]), r[yField]]); const series: any = { type: 'line', diff --git a/packages/flint-js/src/echarts/templates/streamgraph.ts b/packages/flint-js/src/echarts/templates/streamgraph.ts index beb81b30..135ac4f2 100644 --- a/packages/flint-js/src/echarts/templates/streamgraph.ts +++ b/packages/flint-js/src/echarts/templates/streamgraph.ts @@ -17,7 +17,7 @@ */ import { ChartTemplateDef } from '../../core/types'; -import { groupBy } from './utils'; +import { groupBy, toEChartsTemporal } from './utils'; export const ecStreamgraphDef: ChartTemplateDef = { chart: 'Streamgraph', @@ -57,7 +57,7 @@ export const ecStreamgraphDef: ChartTemplateDef = { yAxis: { type: 'value', show: false, axisTick: { show: true } }, series: [{ type: 'line', - data: table.map(r => [r[xField], r[yField]]), + data: table.map(r => [xCS.type === 'temporal' ? toEChartsTemporal(r[xField]) : r[xField], r[yField]]), areaStyle: { opacity: 0.85 }, lineStyle: { width: 0.5 }, symbol: 'none', @@ -102,8 +102,8 @@ export const ecStreamgraphDef: ChartTemplateDef = { const key = `${xv}|||${sn}`; const numVal = valMap.get(key); const value = numVal != null && Number.isFinite(numVal) ? numVal : 0; - // Use index for category so ThemeRiver renders; use string for temporal (date string) - riverData.push([xIsTemporal ? xv : i, value, sn]); + // Use index for category so ThemeRiver renders; use epoch-ms for temporal (handles non-ISO dates) + riverData.push([xIsTemporal ? (toEChartsTemporal(xv) as string | number) : i, value, sn]); } } @@ -115,7 +115,11 @@ export const ecStreamgraphDef: ChartTemplateDef = { formatter: (params: any) => { if (!params || params.length === 0) return ''; const xVal = params[0].value[0]; - const displayX = xIsTemporal ? xVal : (xVals[xVal] ?? xVal); + const displayX = xIsTemporal + ? (typeof xVal === 'number' + ? new Date(xVal).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' }) + : xVal) + : (xVals[xVal] ?? xVal); let html = `${displayX}
`; // Sort by value descending const sortedParams = [...params].sort((a, b) => (b.value[1] || 0) - (a.value[1] || 0)); diff --git a/packages/flint-js/src/echarts/templates/utils.ts b/packages/flint-js/src/echarts/templates/utils.ts index 3847c7f4..59dbd537 100644 --- a/packages/flint-js/src/echarts/templates/utils.ts +++ b/packages/flint-js/src/echarts/templates/utils.ts @@ -17,6 +17,26 @@ export function getCategoryOrder(ctx: InstantiateContext, channel: string): stri ?? ctx.channelSemantics?.[channel]?.ordinalSortOrder; } +/** + * Convert a raw value into something an ECharts `time` axis can plot. + * + * ECharts' built-in time parser only understands ISO-8601-like strings, so + * common human-readable dates (e.g. `"Jan 1 2000"` from a CSV) fail to parse + * and the corresponding points are silently dropped, leaving an empty chart. + * JavaScript's `Date` constructor accepts many more formats, so we pre-parse + * temporal values to epoch-milliseconds. Numbers, `Date` instances, and values + * that cannot be parsed are returned unchanged. + * + * Only call this for values destined for a `time` axis. + */ +export function toEChartsTemporal(v: unknown): unknown { + if (v == null) return v; + if (typeof v === 'number') return v; + if (v instanceof Date) return v.getTime(); + const t = new Date(String(v)).getTime(); + return isNaN(t) ? v : t; +} + // Re-export circumference-pressure functions from core (shared with VL backend) export { computeCircumferencePressure, diff --git a/packages/flint-js/tests/echarts-temporal-dates.test.ts b/packages/flint-js/tests/echarts-temporal-dates.test.ts new file mode 100644 index 00000000..bf14845c --- /dev/null +++ b/packages/flint-js/tests/echarts-temporal-dates.test.ts @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { assembleECharts } from '../src'; + +/** + * Regression: ECharts uses a `time` axis for temporal fields, and its internal + * time parser only accepts ISO-8601-like strings. Non-ISO date strings such as + * `"Jan 1 2000"` (e.g. vega-datasets stocks.csv) failed to parse, so every point + * was silently dropped and the chart rendered blank. + * + * The fix pre-converts temporal x-values to epoch milliseconds before handing + * them to ECharts. These tests assert the series carries numeric timestamps for + * human-readable date strings across the temporal-axis templates. + */ + +const HUMAN_DATES = [ + { date: 'Jan 1 2000', price: 10 }, + { date: 'Feb 1 2000', price: 20 }, + { date: 'Mar 1 2000', price: 15 }, +]; + +function assembleTemporal(chartType: string, extra: Record = {}) { + return assembleECharts({ + data: { values: HUMAN_DATES }, + semantic_types: { date: 'temporal', price: 'quantitative' }, + chart_spec: { + chartType, + encodings: { x: 'date', y: 'price', ...extra }, + }, + }) as any; +} + +function firstSeriesData(ec: any): any[] { + const s = (ec.series || []).find((ser: any) => Array.isArray(ser.data) && ser.data.length); + return s?.data ?? []; +} + +describe('ECharts temporal x-axis accepts non-ISO date strings', () => { + for (const chartType of ['Line Chart', 'Area Chart']) { + it(`${chartType}: converts "Jan 1 2000" style strings to epoch-ms`, () => { + const ec = assembleTemporal(chartType); + expect(ec.xAxis.type).toBe('time'); + + const data = firstSeriesData(ec); + expect(data.length).toBe(HUMAN_DATES.length); + + // Every x-coordinate must be a finite number (epoch ms), never a raw string. + const expected = HUMAN_DATES.map((d) => new Date(d.date).getTime()); + const xs = data.map((pt: any) => (Array.isArray(pt) ? pt[0] : pt.value?.[0])); + for (const x of xs) { + expect(typeof x).toBe('number'); + expect(Number.isFinite(x)).toBe(true); + } + expect(xs).toEqual(expected); + }); + } + + it('produces the same timestamps for ISO and human-readable inputs', () => { + const human = firstSeriesData(assembleTemporal('Line Chart')).map((p: any) => p[0]); + const iso = firstSeriesData( + assembleECharts({ + data: { + values: [ + { date: '2000-01-01', price: 10 }, + { date: '2000-02-01', price: 20 }, + { date: '2000-03-01', price: 15 }, + ], + }, + semantic_types: { date: 'temporal', price: 'quantitative' }, + chart_spec: { chartType: 'Line Chart', encodings: { x: 'date', y: 'price' } }, + }) as any, + ).map((p: any) => p[0]); + expect(human).toEqual(iso); + }); +}); diff --git a/packages/flint-js/tests/heatmap-label-rounding.test.ts b/packages/flint-js/tests/heatmap-label-rounding.test.ts new file mode 100644 index 00000000..83732b6c --- /dev/null +++ b/packages/flint-js/tests/heatmap-label-rounding.test.ts @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { assembleECharts } from '../src'; + +/** + * Regression: ECharts heatmap cell labels previously rendered the raw color + * value at full floating-point precision (e.g. `30.46666700000002`) and, when + * cells were narrow, string-SLICED it into gibberish. Noisy aggregates produced + * long, overlapping, illegible labels. + * + * The fix always rounds the label to a sensible number of decimals that fits the + * cell, falling back to an integer, and hides the label only if even that + * overflows. + */ + +function makeHeatmap(size: { width: number; height: number }, n = 5) { + const rows = Array.from({ length: n }, (_, i) => `r${i}`); + const cols = Array.from({ length: n }, (_, i) => `c${i}`); + const values: any[] = []; + let seed = 3; + const rnd = () => ((seed = (seed * 1103515245 + 12345) & 0x7fffffff) / 0x7fffffff); + for (const r of rows) for (const c of cols) values.push({ row: r, col: c, val: rnd() * 30 + 0.46666700000002 }); + return assembleECharts({ + data: { values }, + semantic_types: { row: 'nominal', col: 'nominal', val: 'quantitative' }, + chart_spec: { + chartType: 'Heatmap', + encodings: { x: 'col', y: 'row', color: 'val' }, + baseSize: size, + }, + }) as any; +} + +function heatLabel(ec: any) { + return (ec.series || []).find((s: any) => s.type === 'heatmap')?.label; +} + +describe('ECharts heatmap labels are rounded, never full-precision floats', () => { + it('rounds noisy float values to short, legible strings', () => { + const ec = makeHeatmap({ width: 900, height: 700 }); + const label = heatLabel(ec); + expect(label?.show).toBe(true); + expect(typeof label.formatter).toBe('function'); + + const noisy = label.formatter({ data: [0, 0, 30.46666700000002] }); + expect(noisy).toBe('30.5'); + // The raw, unrounded string must never leak through. + expect(noisy).not.toContain('30.46666'); + + const small = label.formatter({ data: [0, 0, 0.46666700000002] }); + expect(small).toBe('0.47'); + + // Integers stay clean. + expect(label.formatter({ data: [0, 0, 12] })).toBe('12'); + }); + + it('never emits a label longer than a few characters', () => { + const ec = makeHeatmap({ width: 500, height: 400 }, 6); + const label = heatLabel(ec); + if (label?.show && typeof label.formatter === 'function') { + for (const v of [30.46666700000002, 0.46666700000002, 1234.5678, 7]) { + const out = label.formatter({ data: [0, 0, v] }); + expect(out.length).toBeLessThanOrEqual(6); + } + } + }); +}); diff --git a/packages/flint-js/tests/null-values-dropped.test.ts b/packages/flint-js/tests/null-values-dropped.test.ts new file mode 100644 index 00000000..bb9377cb --- /dev/null +++ b/packages/flint-js/tests/null-values-dropped.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { assembleECharts, assembleChartjs } from '../src'; + +/** + * Regression: `Number(null)` is `0`, so rows with a missing (null / empty) + * numeric measurement were silently coerced to 0 instead of being dropped. This + * injected phantom data points at the origin: + * - ECharts boxplot: a spurious value of 0 pulled the whisker/box to 0 and + * added a bogus outlier at 0. + * - Chart.js bubble: a bubble drawn at (0, 0) for every incomplete row. + * + * The fix drops null/empty cells before numeric coercion. + */ + +describe('ECharts boxplot drops null values instead of coercing to 0', () => { + const values = [ + { grp: 'A', v: 10 }, + { grp: 'A', v: 12 }, + { grp: 'A', v: 11 }, + { grp: 'A', v: null }, + { grp: 'A', v: '' }, + { grp: 'A', v: 13 }, + { grp: 'A', v: 9 }, + ]; + + it('computes the five-number summary from real values only', () => { + const ec = assembleECharts({ + data: { values }, + semantic_types: { grp: 'nominal', v: 'quantitative' }, + chart_spec: { chartType: 'Boxplot', encodings: { x: 'grp', y: 'v' } }, + }) as any; + + const box = (ec.series || []).find((s: any) => s.type === 'boxplot'); + expect(box).toBeTruthy(); + const [min, , , , max] = box.data[0]; + // Min must be the real minimum (9), not 0 from a coerced null. + expect(min).toBe(9); + expect(max).toBe(13); + expect(min).toBeGreaterThan(0); + }); + + it('does not create a phantom outlier at 0', () => { + const ec = assembleECharts({ + data: { + values: [ + { grp: 'A', v: 50 }, + { grp: 'A', v: 51 }, + { grp: 'A', v: 52 }, + { grp: 'A', v: 53 }, + { grp: 'A', v: null }, + { grp: 'A', v: 54 }, + ], + }, + semantic_types: { grp: 'nominal', v: 'quantitative' }, + chart_spec: { chartType: 'Boxplot', encodings: { x: 'grp', y: 'v' } }, + }) as any; + + const outliers = (ec.series || []).find((s: any) => s.type === 'scatter'); + const outlierYs: number[] = (outliers?.data ?? []).map((d: any) => d[1]); + expect(outlierYs).not.toContain(0); + }); +}); + +describe('Chart.js bubble drops rows with missing x/y instead of plotting (0,0)', () => { + it('omits null-x and null-y points', () => { + const cj = assembleChartjs({ + data: { + values: [ + { x: 1, y: 2, s: 5 }, + { x: null, y: 3, s: 6 }, + { x: 4, y: null, s: 7 }, + { x: '', y: 8, s: 9 }, + { x: 5, y: 6, s: 8 }, + ], + }, + semantic_types: { x: 'quantitative', y: 'quantitative', s: 'quantitative' }, + chart_spec: { chartType: 'Bubble Chart', encodings: { x: 'x', y: 'y', size: 's' } }, + }) as any; + + const data = cj.data.datasets[0].data; + // Only the two fully-specified rows survive. + expect(data.length).toBe(2); + for (const pt of data) { + expect(Number.isFinite(pt.x)).toBe(true); + expect(Number.isFinite(pt.y)).toBe(true); + } + // No phantom bubble at the origin. + expect(data.some((p: any) => p.x === 0 && p.y === 0)).toBe(false); + }); +});