diff --git a/.changeset/8650-chart-foreign-dialect-retired.md b/.changeset/8650-chart-foreign-dialect-retired.md new file mode 100644 index 0000000000..687df21814 --- /dev/null +++ b/.changeset/8650-chart-foreign-dialect-retired.md @@ -0,0 +1,51 @@ +--- +'@object-ui/plugin-charts': minor +--- + +Retire the "Tremor/simple format" adapter in `ChartRenderer` — the `index`, +`category` and `value` reads (objectui#8650, triage ruling `5619609278` on +AGENTS.md #0.1: route to the producer, ⛔ not a declaration). + +**Breaking, deliberately, for three keys — and NOT for the fourth.** The card +filed four undeclared reads as one group. A cast-aware read census plus a +TypeScript-checker declaredness reading measured them apart, and they do not +share one verdict: + +- `index` / `category` (they aliased the category axis) and `value` (it became + a single series) are **retired**. They are declared on no published face — + not `ChartSchema`, not its zod mirror, not `ChartRendererProps.schema` — are + advertised by no registry `inputs`, and are taught by no doc, guide or skill. + A structural producer census over `packages/`, `apps/`, `examples/`, + `content/docs/` and the skills corpus (5695 files, 74 chart nodes) found + **zero** nodes writing `category` or `value` and **one** writing `index` — + this repo's own test for the adapter. The zero is read against controls that + fire in the same population (`xAxisKey` 41 nodes, `series` 44, `chartType` + 40, `data` 52) and a nonsense key that returns 0. +- `categories` is **not retired and is unaffected**. It is a declared member of + the published `ChartSchema` and of its zod mirror, is documented in the + schema reference as an alternative series list, and was ruled live by + objectui#6896. `normalizeChartSchema` — the single translation point + (objectui#2880 S1) — already consumed it, so `ChartRenderer`'s own branch was + a second, un-normalized read that no well-formed chart could reach. Removing + it changes nothing for a well-formed chart and removes two wrong answers for + a malformed one: `categories: 'revenue'` reached `.map` on a string and threw + during render, and a `categories` whose entries the normalizer rejects + produced a `[{ dataKey: '' }]` series. + +**Migration.** Write the canonical spellings, which every producer in the +measured corpora already writes: `xAxisKey` (or the spec's `xAxis: { field }`) +for the category axis, and `series` (or `categories`) for the plotted columns. +A chart that still writes `index` / `category` binds no category axis, so +`AdvancedChartImpl` falls back to its default category key, `name`. What that +degrades to depends on the rows, and only one half of it is a refusal: rows +carrying no `name` column hit its existing on-screen `missing-category-key` +refusal, while rows that DO carry one plot silently against `name` instead of +the column the author named — a wrong picture rather than a refusal. One that +still writes `value` plots nothing. + +Also deletes six `(schema as any)` casts that the published declarations had +already made unnecessary — `colors`, `categoryColors` and `categoryOrder` on +`ChartRenderer`, and `colors`, `compareTo` and `series` on `ObjectChart` (the +objectui#8327 bucket-(b) class: declared, then read through a needless cast). +No behaviour changes with them; `ObjectChart.tsx` now has no `(schema as any)` +read left at all. diff --git a/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx b/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx new file mode 100644 index 0000000000..df3fae3049 --- /dev/null +++ b/packages/plugin-charts/src/ChartRenderer.foreignDialectRetired-8650.test.tsx @@ -0,0 +1,219 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#8650 — the "Tremor/simple format" adapter inside `ChartRenderer` is + * retired, and the four keys it read do NOT share one verdict. + * + * ## What was measured, and why the four split + * + * The card filed this as one group of four undeclared reads. A cast-aware read + * census (`schemaReads`, objectui#6576) plus a TypeScript-checker declaredness + * reading (`getPropertyOfType`, never a grep — objectui#8410) measured them + * apart: + * + * - `categories` is a DECLARED member of the published `ChartSchema` and of + * its zod mirror, is documented in the schema reference as an alternative + * series list, and was ruled LIVE by objectui#6896. It was never a foreign + * spelling. `normalizeChartSchema` — the ONE translation point + * (objectui#2880 S1) — already consumed it, so `ChartRenderer`'s own branch + * was a SECOND, un-normalized read of a key the normalizer owns and could + * not be reached by any well-formed chart. ⇒ the branch goes, the + * capability stays. The cases below pin that it still plots. + * - `index`, `category` and `value` are declared on NO published face, are + * advertised by no registry `inputs`, are taught by no doc or skill, and a + * structural producer census over this repo found ZERO nodes writing them. + * ⇒ retired, per AGENTS.md #0.1 (the remedy belongs at the producer, and + * there is no producer). + * + * ## The failure mode the retirement degrades to — measured, not assumed + * + * ⚠️ It is CONDITIONAL on the rows, and an earlier reading of it here was too + * strong. With no `xAxisKey` bound, `AdvancedChartImpl` falls back to its + * default category key `name`: rows carrying no `name` column — `DATA` below + * — hit its on-screen `missing-category-key` refusal (objectui#8168's family), + * but rows that DO carry one plot SILENTLY against `name`. The cases below pin + * the refusing half, and read the refusal by its `data-chart-error` CODE — the + * machine-readable half that sibling suites already pin — never by its wording, + * which no consumer parses. + * + * ⛔ The negative form these cases used to take — sleep a fixed window, then + * assert no plot surface YET — is gone and must not come back. It was a race in + * the FALSE-GREEN direction: warm time-to-surface for the canonical control was + * measured at 97–168 ms (cold 578 ms) against a 150 ms window, so on a loaded + * runner a re-added alias that plotted LATE would satisfy it — a pin that + * cannot fail. A `waitFor` on the POSITIVE signal fails the other way: a slow + * runner makes it slower, never green. + * + * ⭐ Every refusal below is read against the CANONICAL control in the same + * file: the same data and the same chart, written with `xAxisKey` / `series`, + * must plot. Without it a renderer that had stopped drawing anything at all + * would satisfy every negative case here. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0×0 +// under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import { ChartRenderer } from './ChartRenderer'; +// `ChartRenderer` lazy-loads its implementation; import it eagerly with the +// SAME specifier so the dynamic import's cost lands in the import phase rather +// than inside `waitFor`'s budget (the reasoning is spelled out in +// `ChartRenderer.specSeries.test.tsx`). +import './AdvancedChartImpl'; + +afterEach(cleanup); + +const DATA = [ + { month: 'Jan', revenue: 120, margin: 40 }, + { month: 'Feb', revenue: 80, margin: 90 }, +]; + +const marks = (c: HTMLElement) => ({ + bars: c.querySelectorAll('.recharts-bar').length, + lines: c.querySelectorAll('.recharts-line').length, +}); + +/** Waits for the real plot past the lazy boundary, then counts the marks. */ +const plotted = async (c: HTMLElement) => { + await waitFor(() => expect(c.querySelector('.recharts-surface')).toBeTruthy()); + return marks(c); +}; + +/** + * The positive reading for a retired AXIS alias: wait for the refusal + * `AdvancedChartImpl` renders when nothing binds the category axis. Re-add the + * alias and the chart plots instead, no refusal ever arrives, and this + * `waitFor` reddens on timeout. + */ +const expectCategoryAxisRefusal = async (c: HTMLElement) => { + await waitFor(() => + expect(c.querySelector('[data-chart-error="missing-category-key"]')).not.toBeNull(), + ); + // The refusal REPLACES the chart, so no plot surface may coexist with it. + expect(c.querySelector('.recharts-surface')).toBeNull(); +}; + +const renderChart = (schema: Record) => + render().container; + +describe('objectui#8650 — the canonical spellings still plot (the control)', () => { + it('plots `xAxisKey` + `series` — every refusal below is read against this', async () => { + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + xAxisKey: 'month', + series: [{ dataKey: 'revenue' }, { dataKey: 'margin' }], + isAnimationActive: false, + }); + expect(await plotted(container)).toEqual({ bars: 2, lines: 0 }); + }); +}); + +describe('objectui#8650 — `categories` is NOT retired: the declared key still plots', () => { + it('plots a `categories` series list through `normalizeChartSchema`', async () => { + // The read that used to serve this case lived in `ChartRenderer`; it is + // gone, and the chart still draws both columns because the ONE translation + // point has always consumed this key. If this case ever reds, the + // objectui#6896 capability was removed by accident. + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + xAxisKey: 'month', + categories: ['revenue', 'margin'], + isAnimationActive: false, + }); + expect(await plotted(container)).toEqual({ bars: 2, lines: 0 }); + }); + + it('still ignores `categories` when `series` is present — the documented precedence', async () => { + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + xAxisKey: 'month', + series: [{ dataKey: 'revenue' }], + categories: ['revenue', 'margin'], + isAnimationActive: false, + }); + expect(await plotted(container)).toEqual({ bars: 1, lines: 0 }); + }); + + it('no longer throws on a malformed `categories` — the retired branch called `.map` on it', async () => { + // `categories: 'revenue'` (a string, not a list) reached `.map` on a string + // in the retired branch and threw during render. The normalizer answers + // "no series" instead, which is the honest reading of an off-contract + // value: the chart mounts, and plots nothing. + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + xAxisKey: 'month', + categories: 'revenue', + isAnimationActive: false, + }); + await waitFor(() => expect(container.querySelector('.recharts-surface')).toBeTruthy()); + expect(marks(container)).toEqual({ bars: 0, lines: 0 }); + }); +}); + +describe('objectui#8650 — the foreign dialect is retired', () => { + it('`index` no longer binds the category axis', async () => { + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + index: 'month', + series: [{ dataKey: 'revenue' }, { dataKey: 'margin' }], + isAnimationActive: false, + }); + await expectCategoryAxisRefusal(container); + }); + + it('`category` no longer binds the category axis', async () => { + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + category: 'month', + series: [{ dataKey: 'revenue' }, { dataKey: 'margin' }], + isAnimationActive: false, + }); + await expectCategoryAxisRefusal(container); + }); + + it('`value` no longer becomes a single series', async () => { + const container = renderChart({ + type: 'chart', + chartType: 'bar', + data: DATA, + xAxisKey: 'month', + value: 'revenue', + isAnimationActive: false, + }); + // The axis IS bound here, so this case isolates the series half — and the + // reading is positive: a bar with an empty series list reaches the plot + // surface with no refusal, so `plotted` waits for that surface to ARRIVE + // rather than for a fixed window to elapse, and then counts. Re-add the + // `value` read and a bar appears on it. + expect(await plotted(container)).toEqual({ bars: 0, lines: 0 }); + }); +}); diff --git a/packages/plugin-charts/src/ChartRenderer.specSeries.test.tsx b/packages/plugin-charts/src/ChartRenderer.specSeries.test.tsx index 82c0ac5555..64cb2d1492 100644 --- a/packages/plugin-charts/src/ChartRenderer.specSeries.test.tsx +++ b/packages/plugin-charts/src/ChartRenderer.specSeries.test.tsx @@ -152,14 +152,22 @@ describe('ChartRenderer — the spec `series` shape', () => { expect(await plotted(container)).toEqual({ bars: 2, lines: 0 }); }); - it('still adapts the Tremor-ish `categories` form', async () => { + it('still plots the `categories` series list', async () => { + // `categories` is a declared member of the published `ChartSchema` and its + // zod mirror, ruled LIVE by objectui#6896 — not a foreign spelling. This + // case used to prove `ChartRenderer`'s OWN `(schema as any).categories` + // branch; objectui#8650 removed that second read and the key still plots, + // because `normalizeChartSchema` — the one translation point — has always + // consumed it. The axis is written in the canonical `xAxisKey`: the + // Tremor-ish `index` alias that stood here is retired, and its pin lives in + // `ChartRenderer.foreignDialectRetired-8650.test.tsx`. const { container } = render( = ({ schema, onChartCli // `normalizeSeries` could not translate at all (no `dataKey` and no `name` // on any entry). const authored = Array.isArray(schema.series) ? schema.series : undefined; - let series: any[] | undefined = spec.series ?? authored; - let xAxisKey = schema.xAxisKey ?? spec.xAxisKey; + const series: any[] | undefined = spec.series ?? authored; + const xAxisKey = schema.xAxisKey ?? spec.xAxisKey; let config = schema.config; - // Adapt the Tremor/simple format (categories -> series, index -> xAxisKey) - if (!xAxisKey) { - if ((schema as any).index) xAxisKey = (schema as any).index; - else if ((schema as any).category) xAxisKey = (schema as any).category; // Support Pie/Donut category - } - - if (!series) { - if ((schema as any).categories) { - series = (schema as any).categories.map((cat: string) => ({ dataKey: cat })); - } else if ((schema as any).value) { - // Single value adapter (for Pie/Simple charts) - series = [{ dataKey: (schema as any).value }]; - } - } + // ⛔ The "Tremor/simple format" adapter that used to sit here is RETIRED + // (objectui#8650). It read four keys off `schema` behind `as any` casts -- + // `index` / `category` (-> `xAxisKey`) and `categories` / `value` + // (-> `series`) -- and the census that ruled on it found the four do NOT + // share one verdict: + // + // - `categories` was never a foreign spelling at all. It is a declared + // member of the published `ChartSchema` and of its zod mirror, + // documented in the schema reference as an ALTERNATIVE SERIES LIST, and + // ruled LIVE by objectui#6896 (maintainer ruling 2026-08-31, prose + // follows machine). `normalizeChartSchema` -- the ONE translation point + // (objectui#2880 S1) -- already consumes it, so `spec.series` above is + // populated before the old branch could be reached. That branch was a + // SECOND, un-normalized read of a key the normalizer owns: the shape + // objectui#7681 removed for `series`. It was unreachable for every + // well-formed chart, and on malformed input it was WORSE than nothing + // (`categories: 'revenue'` reached `.map` on a string and threw; a + // `categories` whose entries the normalizer rejects produced + // `[{ dataKey: '' }]`). The capability is untouched and stays pinned in + // `normalizeChartSchema.test.ts`. + // - `index`, `category` and `value` WERE a second authoring vocabulary: + // declared on no published face (not `ChartSchema`, not its zod mirror, + // not `ChartRendererProps.schema` above), advertised by no registry + // `inputs`, taught by no doc, guide or skill, and written by ZERO + // producers anywhere in this repo. AGENTS.md #0.1 puts the remedy at + // the producer; with no producer to route, the read was tolerance for a + // dialect nobody speaks. The canonical spellings are `xAxisKey` / + // `xAxis` for the category axis and `series` / `categories` for the + // plotted columns. + // + // ⛔ Do not re-add a key-aliasing branch here. A new inbound spelling is + // translated in `normalizeChartSchema`, which is where every other dialect + // this renderer accepts already resolves -- a second site here is how this + // one became invisible to the normalizer's own tests. // Auto-generate config/colors if missing. A spec `series[].color` is an // explicit author choice, so it wins over the positional palette. if (!config && series) { - const colors = (schema as any).colors || ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))']; + const colors = schema.colors || ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))']; const newConfig: ChartContainerConfig = {}; series.forEach((s: any, idx: number) => { newConfig[s.dataKey] = { label: s.label || s.dataKey, color: s.color || colors[idx % colors.length] }; @@ -213,9 +233,9 @@ export const ChartRenderer: React.FC = ({ schema, onChartCli xAxisKey={props.xAxisKey} series={props.series} className={props.className} - colors={Array.isArray((schema as any).colors) ? (schema as any).colors : undefined} - categoryColors={(schema as any).categoryColors} - categoryOrder={(schema as any).categoryOrder} + colors={Array.isArray(schema.colors) ? schema.colors : undefined} + categoryColors={schema.categoryColors} + categoryOrder={schema.categoryOrder} isAnimationActive={schema.isAnimationActive} onChartClick={onChartClick} xAxis={props.spec.xAxis} diff --git a/packages/plugin-charts/src/ObjectChart.tsx b/packages/plugin-charts/src/ObjectChart.tsx index 32b7ef61ca..d0936d2714 100644 --- a/packages/plugin-charts/src/ObjectChart.tsx +++ b/packages/plugin-charts/src/ObjectChart.tsx @@ -559,8 +559,8 @@ export const ObjectChart = (props: ObjectChartProps) => { [schema.filter], ); const compareToKey = useMemo( - () => ((schema as any).compareTo ? JSON.stringify((schema as any).compareTo) : ''), - [(schema as any).compareTo], + () => (schema.compareTo ? JSON.stringify(schema.compareTo) : ''), + [schema.compareTo], ); // ADR-0021 (#1890): a chart can bind to a semantic-layer `dataset` instead of // the legacy inline `objectName` + `aggregate` query. Stable key over the @@ -811,7 +811,7 @@ export const ObjectChart = (props: ObjectChartProps) => { // value apart from its presence: the ONE discriminator (`.kind`) is // read where the shift is computed — `shiftFilterByCompareTo` — so // this file has no second copy of the branch table to drift from it. - const compareTo: CompareToConfig | undefined = (schema as any).compareTo; + const compareTo: CompareToConfig | undefined = schema.compareTo; const wantsComparison = !!compareTo && supportsCompareTo(schema.chartType); // shiftFilterByCompareTo expects the raw filter (with date macros) // so it can substitute `{current_*}` tokens or re-resolve macros @@ -1102,7 +1102,7 @@ export const ObjectChart = (props: ObjectChartProps) => { // Merge data if not provided in schema. When `compareTo` is configured // for a supported chart type, also synthesize a second series so the // chart implementation renders the comparison overlay (dashed / muted). - const compareToConfig: CompareToConfig | undefined = (schema as any).compareTo; + const compareToConfig: CompareToConfig | undefined = schema.compareTo; // The result column this aggregate projects its value under, and the column // the comparison overlay arrives in (framework#3701). const valueKey = schema.aggregate ? aggregateValueKey(schema.aggregate) : undefined; @@ -1114,7 +1114,7 @@ export const ObjectChart = (props: ObjectChartProps) => { finalData.some((row: Record) => row[comparisonKey] != null); const augmentedSeries = useMemo(() => { - const existing = Array.isArray((schema as any).series) ? (schema as any).series : null; + const existing = Array.isArray(schema.series) ? schema.series : null; if (!enableComparisonSeries) return existing; const primary = existing || [{ dataKey: valueKey }]; const labelMap: Record = { @@ -1135,7 +1135,7 @@ export const ObjectChart = (props: ObjectChartProps) => { variant: 'comparison', }, ]; - }, [enableComparisonSeries, (schema as any).series, valueKey, comparisonKey, schema.filter, compareToConfig]); + }, [enableComparisonSeries, schema.series, valueKey, comparisonKey, schema.filter, compareToConfig]); // ADR-0021 (#1759): when the chart binds to a dataset, derive data/xAxisKey/ // series from its dimensions/measures via the shared buildChartSeries helper — @@ -1182,11 +1182,11 @@ export const ObjectChart = (props: ObjectChartProps) => { // over the field's option colors. We split the two and pass the palette as // `colors` and the merged map as `categoryColors`. const explicitColorMap: Record | null = - (schema as any).colors && !Array.isArray((schema as any).colors) && typeof (schema as any).colors === 'object' - ? ((schema as any).colors as Record) + schema.colors && !Array.isArray(schema.colors) && typeof schema.colors === 'object' + ? schema.colors : null; const paletteColors: string[] | undefined = - Array.isArray((schema as any).colors) ? ((schema as any).colors as string[]) : undefined; + Array.isArray(schema.colors) ? schema.colors : undefined; const mergedCategoryColors = (fieldOptionColors || explicitColorMap) ? { ...(fieldOptionColors || {}), ...(explicitColorMap || {}) } : undefined; diff --git a/packages/plugin-charts/src/normalizeChartSchema.ts b/packages/plugin-charts/src/normalizeChartSchema.ts index 3217874418..49c1226285 100644 --- a/packages/plugin-charts/src/normalizeChartSchema.ts +++ b/packages/plugin-charts/src/normalizeChartSchema.ts @@ -378,8 +378,15 @@ function normalizeAxis(raw: unknown, language: string | null | undefined): Norma */ function normalizeSeries(raw: unknown, language: string | null | undefined): NormalizedSeries | undefined { if (!isRec(raw)) { - // A bare string is accepted as a shorthand for `{ name }` — the Tremor-ish - // `categories: ['a','b']` form ChartRenderer already adapts. + // A bare string is accepted as a shorthand for `{ name }` — the entry form + // the Tremor-ish `categories: ['a','b']` series list feeds in, one entry at + // a time (the branch that reads `categories` is below, in this file). + // ⚠️ THIS FILE is the only place that adapts it: `ChartRenderer` carried a + // second, un-normalized read of `categories` until objectui#8650 retired + // that branch, which leaves `normalizeChartSchema` the one translation + // point it was always meant to be (objectui#2880 S1). ⛔ Do not restore a + // reader-side `categories` branch to "help" — that is the second de-facto + // contract AGENTS.md #0.1 exists to refuse. const bare = str(raw); return bare ? { dataKey: bare } : undefined; }