fix(charts): reject long-format results in bar and line (#1400)

CHANGELOG.md

@@ -16,6 +16,7 @@ Chart-experience release: chart authoring, editing, rule-based styling and click
 
 ### Fixed
 
+- A long-format result — `category, series, value`, what `GROUP BY a, b` naturally produces — rendered a bar or line chart that looked plausible and was wrong. `resolveValueKeys` treats every non-label column as a value series, so the `series` column became a series of its own whose cells all coerced to null: 12 bars with duplicated x labels, a ghost legend swatch with no bars, and **no stacking at all** despite `stackMode` being set. Line was worse — a single revenue line drawn across duplicated x values, interleaving delivered -> shipped -> delivered, which reads as a violently spiky time series but is pure artifact. Nothing marked either as an error, and the seeded reference dashboard itself demonstrated two `stackMode` options with this exact query shape, which is how easy it is to fall into. Bar and line now reject a result whose plotted value columns contain no numeric values, naming the offending column and showing the pivot that fixes it. A column that is entirely null stays legal — that is a sparse series, what a LEFT JOIN produces. `validate` now receives the same column mapping as the transform, so a result whose text columns the user already mapped away is not wrongly rejected. The 11 affected reference tiles are rewritten to wide format, so the stacking options finally demonstrate stacking (#1400)
 - `numberFormat: "percent"` appended a `%` sign and scaled nothing, so a KPI computing `401 / 2000` rendered **`0.2%`** where the true share was **`20.05%`** — wrong by a factor of 100, rendered cleanly, with nothing in the UI to distinguish it from a real value. `0.2%` and `20.05%` are both believable share figures. The convention was never stated anywhere and the two option surfaces documented **opposite** contracts: the table's option was labelled `Percent (12%)`, asserting input was already 0–100, while the seeded reference tile was authored with ratio semantics — the clearest evidence the contract was not discoverable. `percent` now takes a **ratio** and scales it, via `Intl.NumberFormat`'s own `style: "percent"`, which also brings locale grouping (`12.5` → `1,250%`) that the hand-rolled branch never had. With no explicit decimal places it caps at 2 rather than Intl's default of 0, since rounding `20.05%` to `20%` is the same class of silent precision loss. **Breaking** for anyone who worked around the old behaviour by pre-multiplying by 100 in their query. Both option descriptions, both option labels, the single-value docs page and the demo tile now state the expected input range (#1396)
 - A client-side `groupBy` reported `count` as the number of **rows** in the group while `sum`, `avg`, `min` and `max` all skipped nulls, so the three contradicted each other: a group of `[100, 200, null]` returned `count: 3, sum: 300, avg: 150`, and `sum / count` came to 100. A reader saw "3 revenue values totalling 300" with no way to know only two rows had revenue at all — and because `count` is typically the denominator someone divides by, the error propagated into everything derived from it. Nulls in an aggregated column are not an edge case; they are what a `LEFT JOIN` produces. `count` now counts non-null values, matching SQL's `COUNT(col)` — the output key is `revenue_count`, named for the column, so the column form is the only one it could have meant. The existing test used sample data with no nulls and passed on both the wrong and the right behaviour; the new invariant test (`sum / count === avg` for every group) fails on the old code and cannot be silently re-broken (#1414)
 - Overlays ignored `prefers-reduced-motion`. All ten Radix primitives (dialog, alert-dialog, sheet, popover, tooltip, toast, select, dropdown-menu, context-menu, navigation-menu) animated unconditionally, even though spinner, progress and skeleton had honoured the preference since the component audit. Per-component `motion-reduce:animate-none` cannot fix this: the overlays animate through data-attribute variants like `data-[state=open]:animate-in`, which compile to `.class[data-state=open]` — specificity (0,2,0) — while the `motion-reduce:` utility is (0,1,0) and a media query adds no specificity, so the guard loses the cascade. The reset now lives once in `design-tokens.css`, the only stylesheet both packages import. It also fixed a CI flake nobody had connected to accessibility: Radix keeps an overlay mounted until its exit animation reports `animationend`, and when an NVL/WebGL widget mounted alongside the close that animation stalled, leaving `data-state="closed"` dialogs visible past a 5s budget — every "flaky graph chart" failure on `dev` was this, never a graph assertion. The E2E suite now runs as a reduced-motion user, so the accessibility branch is exercised in CI rather than merely declared (#1458)

app/src/components/__tests__/card-container-states.test.tsx

@@ -98,6 +98,28 @@ vi.mock("@/lib/query/data-transforms", () => ({
   applyTransforms: (d: unknown) => d,
 }));
 
+// `chart-helpers` registers *lightweight stub* plugins when the full plugin
+// modules haven't loaded, and a stub carries no `validate` — so without this
+// the validation path is unreachable from jsdom and #1400 could not be tested
+// at this layer at all. Attach the real bar validator; importing the plugin
+// module itself would drag ECharts into jsdom for no benefit.
+vi.mock("@/lib/plugin/chart-helpers", async () => {
+  const actual = await vi.importActual<
+    typeof import("@/lib/plugin/chart-helpers")
+  >("@/lib/plugin/chart-helpers");
+  const { validateBarData } = await vi.importActual<
+    typeof import("@/plugins/bar/transform")
+  >("@/plugins/bar/transform");
+  return {
+    ...actual,
+    getChartConfig: (type: string) => {
+      const config = actual.getChartConfig(type);
+      if (!config || type !== "bar") return config;
+      return { ...config, validate: validateBarData };
+    },
+  };
+});
+
 /* ---------- import under test ---------- */
 import { CardContainer } from "../card-container";
 import type { DashboardWidget } from "@/lib/db/schema";
@@ -462,4 +484,63 @@ describe("CardContainer", () => {
 
     expect(screen.queryByText(/Showing first .* rows/)).toBeNull();
   });
+
+  // ----- Long-format rejection (#1400) -----
+
+  describe("long-format results (#1400)", () => {
+    const longFormat = [
+      { category: "Apparel", series: "delivered", revenue: 100 },
+      { category: "Apparel", series: "shipped", revenue: 50 },
+      { category: "Home", series: "delivered", revenue: 80 },
+    ];
+
+    it("renders an explicit error state instead of a silently wrong chart", () => {
+      mockUseWidgetQuery.mockReturnValue({
+        isPending: false,
+        fetchStatus: "idle",
+        isError: false,
+        data: { data: longFormat, resultId: "r1" },
+        missingParams: [],
+      });
+
+      render(<CardContainer widget={makeWidget({ chartType: "bar" })} />);
+
+      expect(screen.queryByTestId("chart-renderer")).toBeNull();
+      expect(screen.getByText("Incompatible data format")).toBeDefined();
+    });
+
+    it("names the offending column in the error", () => {
+      mockUseWidgetQuery.mockReturnValue({
+        isPending: false,
+        fetchStatus: "idle",
+        isError: false,
+        data: { data: longFormat, resultId: "r1" },
+        missingParams: [],
+      });
+
+      render(<CardContainer widget={makeWidget({ chartType: "bar" })} />);
+
+      expect(screen.getByText(/"series"/)).toBeDefined();
+    });
+
+    it("still renders the chart for the wide-format equivalent", () => {
+      mockUseWidgetQuery.mockReturnValue({
+        isPending: false,
+        fetchStatus: "idle",
+        isError: false,
+        data: {
+          data: [
+            { category: "Apparel", delivered: 100, shipped: 50 },
+            { category: "Home", delivered: 80, shipped: 20 },
+          ],
+          resultId: "r1",
+        },
+        missingParams: [],
+      });
+
+      render(<CardContainer widget={makeWidget({ chartType: "bar" })} />);
+
+      expect(screen.getByTestId("chart-renderer")).toBeDefined();
+    });
+  });
 });

app/src/components/card-container.tsx

@@ -276,8 +276,7 @@ export function CardContainer({
 
   // Resolve color scales config
   const conditionalFormatting = ws.conditionalFormatting as
-    | { colorScales?: ColorScaleConfig[] }
-    | undefined;
+    { colorScales?: ColorScaleConfig[] } | undefined;
   const colorScales = conditionalFormatting?.colorScales;
 
   if (!chartConfig) {
@@ -293,7 +292,8 @@ export function CardContainer({
 
   // Use preview data directly if provided
   if (previewData !== undefined) {
-    const validationError = chartConfig.validate?.(previewData) ?? null;
+    const validationError =
+      chartConfig.validate?.(previewData, columnMapping) ?? null;
     if (validationError) {
       return (
         <EmptyState
@@ -619,7 +619,8 @@ export function CardContainer({
   }
 
   const rawData = widgetQuery.data.data;
-  const validationError = chartConfig.validate?.(rawData) ?? null;
+  const validationError =
+    chartConfig.validate?.(rawData, columnMapping) ?? null;
   if (validationError) {
     return (
       <EmptyState

app/src/lib/plugin/chart-plugin-registry.ts

@@ -95,8 +95,14 @@ export interface ChartPluginConfig {
   /**
    * Validates raw data shape. Returns an error string when data is present
    * but malformed. Returns null for valid or empty data (empty = "No data" state).
+   *
+   * Receives the same mapping as `transformWithMapping` so a validator can
+   * check the columns that will actually be plotted. Without it, a validator
+   * has to assume positional defaults and would wrongly reject a result whose
+   * text columns the user had already mapped away (#1400).
    */
-  validate?: (data: unknown) => string | null;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ColumnMapping lives in component package
+  validate?: (data: unknown, mapping?: any) => string | null;
   /** Chart-specific options shown in the Chart Options panel. */
   options?: ChartOptionDef[];
   /** Example + column expectations shown to users when they pick this chart. */

app/src/plugins/bar/transform.ts

@@ -9,6 +9,7 @@ import {
   normalizeValue,
   collectAllKeys,
   toSeriesNumber,
+  validateNumericValueColumns,
   type ColumnMapping,
 } from "../transforms/shared-utils";
 
@@ -48,11 +49,14 @@ export function transformToBarData(
  * Validates raw data shape for bar charts.
  * Returns null if valid or empty, error string if rows exist but shape is wrong.
  */
-export function validateBarData(data: unknown): string | null {
+export function validateBarData(
+  data: unknown,
+  mapping?: ColumnMapping,
+): string | null {
   const records = toRecords(data);
   if (!records.length) return null;
   const cols = collectAllKeys(records).length;
   if (cols < 2)
     return `Bar chart requires at least 2 columns: first column for category labels (x-axis) and one or more columns for numeric values (y-axis). Your query returned only ${cols} column(s). Example: \`SELECT category, count FROM ...\``;
-  return null;
+  return validateNumericValueColumns(records, "Bar chart", mapping);
 }

app/src/plugins/line/transform.ts

@@ -9,6 +9,7 @@ import {
   normalizeValue,
   collectAllKeys,
   toSeriesNumber,
+  validateNumericValueColumns,
   type ColumnMapping,
 } from "../transforms/shared-utils";
 
@@ -47,11 +48,14 @@ export function transformToLineData(
  * Validates raw data shape for line charts.
  * Returns null if valid or empty, error string if rows exist but shape is wrong.
  */
-export function validateLineData(data: unknown): string | null {
+export function validateLineData(
+  data: unknown,
+  mapping?: ColumnMapping,
+): string | null {
   const records = toRecords(data);
   if (!records.length) return null;
   const cols = collectAllKeys(records).length;
   if (cols < 2)
     return `Line chart requires at least 2 columns: first column for x-axis values (dates, numbers, or labels) and one or more columns for numeric series. Your query returned only ${cols} column(s). Example: \`SELECT date, revenue FROM ...\``;
-  return null;
+  return validateNumericValueColumns(records, "Line chart", mapping);
 }

app/src/plugins/transforms/__tests__/bar.test.ts

@@ -107,3 +107,34 @@ describe("validateBarData", () => {
     expect(err).toContain("1 column");
   });
 });
+
+// #1400 — the reference dashboard itself demonstrated two stackMode options
+// with this query shape, which is how easy it is to fall into.
+describe("validateBarData — long format (#1400)", () => {
+  const longFormat = [
+    { category: "Apparel", series: "delivered", revenue: 100 },
+    { category: "Apparel", series: "shipped", revenue: 50 },
+    { category: "Home", series: "delivered", revenue: 80 },
+  ];
+
+  it("rejects a long-format result naming the offending column", () => {
+    const err = validateBarData(longFormat);
+    expect(err).not.toBeNull();
+    expect(err).toContain("series");
+  });
+
+  it("accepts the wide-format equivalent", () => {
+    expect(
+      validateBarData([
+        { category: "Apparel", delivered: 100, shipped: 50 },
+        { category: "Home", delivered: 80, shipped: 20 },
+      ]),
+    ).toBeNull();
+  });
+
+  it("accepts long format once the value column is mapped explicitly", () => {
+    expect(
+      validateBarData(longFormat, { xAxis: "category", yAxis: ["revenue"] }),
+    ).toBeNull();
+  });
+});

app/src/plugins/transforms/__tests__/line.test.ts

@@ -94,3 +94,35 @@ describe("validateLineData", () => {
     expect(err).toContain("Line chart");
   });
 });
+
+// #1400 — line was the worse of the two: one revenue line drawn across
+// duplicated x values, interleaving delivered -> shipped -> delivered, which
+// reads as a violently spiky time series but is pure artifact.
+describe("validateLineData — long format (#1400)", () => {
+  const longFormat = [
+    { week: "2026-01-05", series: "delivered", revenue: 100 },
+    { week: "2026-01-05", series: "shipped", revenue: 50 },
+    { week: "2026-01-12", series: "delivered", revenue: 80 },
+  ];
+
+  it("rejects a long-format result naming the offending column", () => {
+    const err = validateLineData(longFormat);
+    expect(err).not.toBeNull();
+    expect(err).toContain("series");
+  });
+
+  it("accepts the wide-format equivalent", () => {
+    expect(
+      validateLineData([
+        { week: "2026-01-05", delivered: 100, shipped: 50 },
+        { week: "2026-01-12", delivered: 80, shipped: 20 },
+      ]),
+    ).toBeNull();
+  });
+
+  it("accepts long format once the value column is mapped explicitly", () => {
+    expect(
+      validateLineData(longFormat, { xAxis: "week", yAxis: ["revenue"] }),
+    ).toBeNull();
+  });
+});

app/src/plugins/transforms/__tests__/shared.test.ts

@@ -5,6 +5,7 @@ import {
   resolveValueKeys,
   collectAllKeys,
   toSeriesNumber,
+  validateNumericValueColumns,
 } from "../shared-utils";
 
 describe("toRecords", () => {
@@ -126,3 +127,96 @@ describe("toSeriesNumber", () => {
     expect(toSeriesNumber(Number.NaN)).toBeNull();
   });
 });
+
+// ---------------------------------------------------------------------------
+// #1400 — long-format rejection
+// ---------------------------------------------------------------------------
+
+describe("validateNumericValueColumns (#1400)", () => {
+  // The natural shape of `GROUP BY a, b`. Every non-label column was treated
+  // as a value series, so `series` became a series of its own whose cells all
+  // coerced to null — a ghost legend entry, duplicated x labels, and a chart
+  // that looked like data.
+  const longFormat = [
+    { category: "Apparel", series: "delivered", revenue: 100 },
+    { category: "Apparel", series: "shipped", revenue: 50 },
+    { category: "Home", series: "delivered", revenue: 80 },
+  ];
+
+  it("rejects a value column whose every non-null cell is non-numeric", () => {
+    const msg = validateNumericValueColumns(longFormat, "Bar chart");
+    expect(msg).not.toBeNull();
+  });
+
+  it("names the offending column", () => {
+    const msg = validateNumericValueColumns(longFormat, "Bar chart");
+    expect(msg).toContain("series");
+  });
+
+  it("accepts a wide-format result", () => {
+    const wide = [
+      { category: "Apparel", delivered: 100, shipped: 50 },
+      { category: "Home", delivered: 80, shipped: 20 },
+    ];
+    expect(validateNumericValueColumns(wide, "Bar chart")).toBeNull();
+  });
+
+  it("accepts numeric strings", () => {
+    const rows = [
+      { month: "Jan", revenue: "100" },
+      { month: "Feb", revenue: "200" },
+    ];
+    expect(validateNumericValueColumns(rows, "Line chart")).toBeNull();
+  });
+
+  // An all-null column is a legitimate sparse series — what a LEFT JOIN
+  // produces — and `collectAllKeys` exists specifically to keep it. Rejecting
+  // it would re-break the case the union-of-keys logic was written for.
+  it("accepts an entirely null column as a sparse series", () => {
+    const sparse = [
+      { month: "Jan", revenue: 100, forecast: null },
+      { month: "Feb", revenue: 200, forecast: null },
+    ];
+    expect(validateNumericValueColumns(sparse, "Line chart")).toBeNull();
+  });
+
+  it("accepts a column that is only partly populated", () => {
+    const partial = [
+      { month: "Jan", revenue: 100, forecast: null },
+      { month: "Feb", revenue: 200, forecast: 250 },
+    ];
+    expect(validateNumericValueColumns(partial, "Line chart")).toBeNull();
+  });
+
+  it("respects an explicit yAxis mapping and ignores unmapped text columns", () => {
+    // The user mapped the value column themselves; `series` is not plotted,
+    // so it must not be flagged.
+    const msg = validateNumericValueColumns(longFormat, "Bar chart", {
+      xAxis: "category",
+      yAxis: ["revenue"],
+    });
+    expect(msg).toBeNull();
+  });
+
+  it("flags an explicitly mapped column that is non-numeric", () => {
+    const msg = validateNumericValueColumns(longFormat, "Bar chart", {
+      xAxis: "category",
+      yAxis: ["series"],
+    });
+    expect(msg).toContain("series");
+  });
+
+  it("names every offending column when there is more than one", () => {
+    const rows = [
+      { category: "A", series: "x", label: "p", revenue: 1 },
+      { category: "B", series: "y", label: "q", revenue: 2 },
+    ];
+    const msg = validateNumericValueColumns(rows, "Bar chart") ?? "";
+    expect(msg).toContain("series");
+    expect(msg).toContain("label");
+  });
+
+  it("returns null for empty data", () => {
+    expect(validateNumericValueColumns([], "Bar chart")).toBeNull();
+  });
+});