diff --git a/.changeset/chart-measure-unknown-presentation-positions.md b/.changeset/chart-measure-unknown-presentation-positions.md new file mode 100644 index 0000000000..4f1d16e175 --- /dev/null +++ b/.changeset/chart-measure-unknown-presentation-positions.md @@ -0,0 +1,20 @@ +--- +"@objectstack/lint": patch +--- + +`chart-measure-unknown` no longer blocks a build over a chart `series[].name` (or a page chart's `yAxis[].field`) that names nothing — those positions are presentation, and the message now says so. + +The rule fired at `error` on every measure position of the three chart surfaces it covers, with one consequence sentence: *"result rows are keyed by MEASURE NAME … so this series comes back empty"*. Read at the `@object-ui` revision this repo pins (`.objectui-sha`), that is true only where the position feeds the dataset query, and the three surfaces do not agree: + +- **Report charts** run the chart's own query out of the two axis strings (`useDatasetRows(dataset, [xAxis], [yAxis], …)` — *"the embedded chart queries only `chart.xAxis` × `chart.yAxis`"*), so `chart.xAxis`/`chart.yAxis` are the binding. `chart.series[]` is *"the author's per-chart override for ONE measure's display name"*, lowered through `mergeAuthoredSeries`, where *"an authored entry naming a measure that is NOT in the dataset selection is **ignored** — membership belongs to the dataset"*. +- **List-view charts** have no presentation position at all: `ListChartConfigSchema` is a strict object of `chartType`/`dataset`/`dimensions`/`values`, and `values[]` is handed to the chart as the dataset measures. +- **Dataset-bound page chart components** query `{ dimensions, measures: values }` and then replace the authored series wholesale with one derived entry per selected measure, so `properties.series[].name` reaches the renderer not at all and `properties.yAxis[].field` re-points nothing. + +**Behaviour change users see:** the three presentation positions — report `chart.series[].name`, page-component `properties.series[].name` and `properties.yAxis[].field` — drop from `error` to `warning`. A build or a metadata publish that used to be refused because of one of them now succeeds, with the finding on the advisory channel. The finding is KEPT, not deleted: the metadata really is wrong — the author wrote a key and believes it is in force. Every query position (report `chart.yAxis`, and `values[]` on all three surfaces) keeps `error` and its existing message verbatim. + +Two smaller corrections ride along, both from the same read: + +- The page surface's `yAxis[].field` refs are no longer concatenated into the `series[]` limb before the measure walk, so an axis position no longer takes the series message. Reading both shapes on that surface stays deliberate; giving them one sentence was not. +- `chart-axis-not-selected` (a declared measure outside the selection) took the same one-size consequence, *"the query does not return it, so the series plots nothing"*. It keeps that wording at a query position and states the real one at a presentation position, where no series is derived for the name in the first place. + +Note that none of these three surfaces declares `suppressWarnings` — it is a dashboard-widget key — so the new advisories cannot be individually silenced; the hint says so instead of pointing at a key that does not exist. diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts index c2f1abdcb6..994aee1273 100644 --- a/packages/lint/src/validate-chart-bindings.test.ts +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -175,6 +175,260 @@ describe('validateChartBindings — report charts', () => { }); expect(findings).toHaveLength(1); expect(findings[0].path).toBe('reports[0].chart.series[0].name'); + // #15575 — still reported, one tier down: see the block below for why. + expect(findings[0].severity).toBe('warning'); + }); +}); + +/** + * #15575 — the per-position tier and consequence, pinned per surface against + * the `@object-ui` revision `.objectui-sha` names. Each title names the + * renderer line that decides it, as the `chart-field-unknown` tests do: the + * tier follows that measurement, and a test that does not name it cannot be + * re-checked when the pin moves. + */ +describe('validateChartBindings — binding vs presentation positions (#15575)', () => { + const reportWith = (chart: Record) => ({ + ...baseStack(), + reports: [ + { name: 'r', dataset: 'task_metrics', values: ['task_count'], chart }, + ], + }); + + const pageWith = (properties: Record) => ({ + ...baseStack(), + pages: [ + { + name: 'p', + regions: [ + { name: 'main', components: [{ type: 'object-chart', properties }] }, + ], + }, + ], + }); + + // ── report charts ────────────────────────────────────────────────────── + it('report chart.yAxis stays ERROR — DatasetReportRenderer runs `useDatasetRows(dataset, [xAxis], [yAxis], …)`, i.e. "the embedded chart queries only `chart.xAxis` × `chart.yAxis`"', () => { + const findings = validateChartBindings( + reportWith({ type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('reports[0].chart.yAxis'); + expect(findings[0].message).toContain('this series comes back empty'); + }); + + it('report chart.series[].name is WARNING — `mergeAuthoredSeries` pairs an authored entry with the derived series whose key it EQUALS, so "an authored entry naming a measure that is NOT in the dataset selection is ignored"', () => { + const findings = validateChartBindings( + reportWith({ + type: 'bar', + xAxis: 'status', + yAxis: 'task_count', + series: [{ name: 'estimate_hours', color: '#f00' }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].path).toBe('reports[0].chart.series[0].name'); + // The consequence the pin actually has — and NOT the one it refutes. + expect(findings[0].message).toContain('DISPLAY-NAME override'); + expect(findings[0].message).toContain('lands on nothing'); + expect(findings[0].message).not.toContain('comes back empty'); + // No surface here carries `suppressWarnings` (dashboard widgets only), and + // the hint says so rather than advertising a key that does not exist. + expect(findings[0].hint).toContain('no `suppressWarnings` key'); + }); + + it('one report chart reports BOTH tiers — the query position gates, the presentation position advises', () => { + const findings = validateChartBindings( + reportWith({ + type: 'bar', + xAxis: 'status', + yAxis: 'estimate_hours', + series: [{ name: 'ghost' }], + }), + ); + expect(findings.map((f) => [f.path, f.severity])).toEqual([ + ['reports[0].chart.yAxis', 'error'], + ['reports[0].chart.series[0].name', 'warning'], + ]); + }); + + it('chart-axis-not-selected at a report series position drops the query sentence — the chart derives ONE series, from its own `chart.yAxis`', () => { + const findings = validateChartBindings( + reportWith({ + type: 'bar', + xAxis: 'status', + yAxis: 'task_count', + series: [{ name: 'est_hours' }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toContain('display-name override'); + expect(findings[0].message).not.toContain('the query does not return it'); + expect(findings[0].hint).toContain('chart.yAxis'); + }); + + it('chart-axis-not-selected at the report yAxis position keeps its wording — that position IS the query', () => { + const findings = validateChartBindings( + reportWith({ type: 'bar', xAxis: 'status', yAxis: 'est_hours' }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED); + expect(findings[0].message).toContain('the query does not return it'); + }); + + // ── list-view charts ─────────────────────────────────────────────────── + it('list chart values[] stays ERROR — ObjectView hands `values` to the chart as the dataset measures (`values: vals`), and the series list is synthesised FROM it', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'v', + list: { + chart: { + chartType: 'bar', + dataset: 'task_metrics', + dimensions: ['status'], + values: ['estimate_hours'], + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('views[0].list.chart.values[0]'); + }); + + it('a list chart has NO presentation position at all — `ListChartConfigSchema` is a strict object of chartType/dataset/dimensions/values, so a stray `series` key is not this rule to report', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'v', + list: { + chart: { + chartType: 'bar', + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + // Not declared by the schema — the strict parse refuses it, and + // this rule stays silent rather than inventing a second verdict. + series: [{ name: 'ghost_measure' }], + }, + }, + }, + ], + }); + expect(findings).toEqual([]); + }); + + // ── dataset-bound page chart components ──────────────────────────────── + it('page component series[].name is WARNING — ObjectChart REPLACES the authored array (`series: datasetChart.series`, one entry per selected measure)', () => { + const findings = validateChartBindings( + pageWith({ + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + series: [{ name: 'ghost_measure', stack: 'a' }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.series[0].name'); + expect(findings[0].message).toContain('REPLACES the authored array'); + expect(findings[0].message).not.toContain('comes back empty'); + }); + + it('page component yAxis[].field takes the AXIS sentence, not the series one — the two limbs no longer share a message', () => { + const findings = validateChartBindings( + pageWith({ + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'ghost_measure', stepSize: 1 }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.yAxis[0].field'); + expect(findings[0].message).toContain('axis PRESENTATION'); + expect(findings[0].message).toContain('the plotted columns come from `values`'); + expect(findings[0].message).not.toContain('REPLACES the authored array'); + }); + + it('page component values[] stays ERROR — ObjectChart queries `{ dimensions: schema.dimensions, measures: schema.values }`', () => { + const findings = validateChartBindings( + pageWith({ + dataset: 'task_metrics', + dimensions: ['status'], + values: ['estimate_hours'], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.values[0]'); + expect(findings[0].message).toContain('this series comes back empty'); + }); + + it('chart-axis-not-selected on the page surface names the derivation, per limb', () => { + const findings = validateChartBindings( + pageWith({ + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'est_hours' }], + series: [{ name: 'est_hours' }], + }), + ); + expect(findings.map((f) => [f.path, f.rule, f.severity])).toEqual([ + [ + 'pages[0].regions[0].components[0].properties.yAxis[0].field', + CHART_AXIS_NOT_SELECTED, + 'warning', + ], + [ + 'pages[0].regions[0].components[0].properties.series[0].name', + CHART_AXIS_NOT_SELECTED, + 'warning', + ], + ]); + expect(findings[0].message).toContain('the axis entry re-points nothing'); + expect(findings[1].message).toContain('the series are derived from `values`'); + expect(findings[0].message).not.toContain('the query does not return it'); + expect(findings[1].message).not.toContain('the query does not return it'); + }); + + it('the advisory positions never gate — every presentation finding this rule can raise is below `error`', () => { + const findings = validateChartBindings( + pageWith({ + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'ghost_a' }], + series: [{ name: 'ghost_b' }], + }), + ).concat( + validateChartBindings( + reportWith({ + type: 'bar', + xAxis: 'status', + yAxis: 'task_count', + series: [{ name: 'ghost_c' }], + }), + ), + ); + expect(findings).toHaveLength(3); + expect(findings.every((f) => f.severity === 'warning')).toBe(true); }); }); @@ -281,6 +535,8 @@ describe('validateChartBindings — dataset-bound page chart components', () => expect(findings[0].path).toBe( 'pages[0].regions[0].components[0].properties.yAxis[0].field', ); + // #15575 — presentation on this surface, so advisory rather than gating. + expect(findings[0].severity).toBe('warning'); }); it('accepts a resolved page chart', () => { @@ -353,6 +609,49 @@ describe('validateChartBindings — floor', () => { expect(findings).toEqual([]); }); + // The #15636 seam this file carries: its collection reader was a hand-copied + // `asArray` whose array branch was an unchecked cast, so a junk member was + // DEREFERENCED (`strName(entry.name)` on `null`) rather than skipped and the + // rule threw where it should have reported. Now `recordsOf`, which filters. + it('survives a null member in every collection it reads — skipped, not dereferenced', () => { + const stack = { + datasets: [ + null, + { + name: 'task_metrics', + object: 'showcase_task', + dimensions: [null, { name: 'status', field: 'status' }], + measures: [null, { name: 'task_count', aggregate: 'count' }], + }, + ], + reports: [ + null, + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { + type: 'bar', + xAxis: 'status', + yAxis: 'task_count', + series: [null, { name: 'ghost_measure' }], + }, + }, + ], + } as unknown as Record; + + const findings = validateChartBindings(stack); + // The junk members are gone rather than fatal, and the real declarations + // around them still resolve: `status`/`task_count` are found (no unknown- + // ref finding for either), and the one genuine defect is still reported. + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('warning'); + // Positions are indexes into the FILTERED collection — a dropped member + // shifts them, which is the honest price of not crashing on junk. + expect(findings[0].path).toBe('reports[0].chart.series[0].name'); + }); + it('does not mistake a tree view named "org_chart" for a chart', () => { const findings = validateChartBindings({ ...baseStack(), diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index ad2ec5f90d..17f6f5bf45 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -35,6 +35,69 @@ * rule has no business loading. It is checked by `validate-react-page-props` * instead, against the naming convention `chartAggregateResultKeys` * (`@objectstack/spec/ui`) now pins down (#3701). + * + * ## Which positions are BINDINGS, and which are presentation (#15575) + * + * `chart-measure-unknown`'s message names a QUERY consequence — *"this series + * comes back empty"* — and that is only true where the position feeds the + * dataset query. Read at the pinned `@object-ui` revision (`.objectui-sha`), + * the three surfaces do not agree, so the tier and the consequence are per + * POSITION rather than per rule: + * + * - **Report charts.** The chart runs its OWN query out of the two axis + * strings — `useDatasetRows(dataset, [xAxis], [yAxis], …)` in + * `plugin-report/src/DatasetReportRenderer.tsx`, and in that file's own + * words *"the embedded chart queries only `chart.xAxis` × `chart.yAxis`"*. + * So `chart.yAxis` IS the binding and keeps `error`. `chart.series[]` is + * not: it is *"The author's per-chart override for ONE measure's display + * name — the entry of `chart.series[]` whose `name` IS that measure"*, + * lowered through `mergeAuthoredSeries`, for which *"an authored entry + * naming a measure that is NOT in the dataset selection is **ignored** — + * membership belongs to the dataset"* (`@object-ui/core` + * `src/utils/chart-presentation.ts`). + * - **List-view charts.** `ListChartConfigSchema` is a STRICT object of + * `chartType` / `dataset` / `dimensions` / `values`: it declares no + * `series` and no `yAxis`, so this rule has no presentation position on + * that surface at all. Its one measure position is `values[]`, which + * `app-shell/src/views/ObjectView.tsx` hands to the chart component as the + * dataset measures (and synthesises the series list FROM). Query binding, + * `error`, unchanged. + * - **Dataset-bound page chart components.** `plugin-charts/src/ObjectChart.tsx` + * queries `{ dimensions: schema.dimensions, measures: schema.values }` and + * then REPLACES the authored series wholesale — + * `{ ...schema, data, xAxisKey, series: datasetChart.series }`, one derived + * entry per selected measure. An authored `properties.series[].name` reaches + * the renderer not at all, and an authored `properties.yAxis[].field` is + * inert for the same reason (`normalizeChartSchema` synthesises series from + * `yAxis[].field` only when there are none, which on this surface means an + * empty `values` — a chart with no measures to plot either way). Both are + * presentation; `dimensions` / `values` are the binding. + * + * So the presentation positions drop to `warning` and state what actually + * happens, which is the resolution the maintainer ruling on + * `chart-field-unknown` reached for the same question one rule over (see "The + * three refused binding keys" in `validate-widget-bindings.ts`). Two things + * follow that differ from that sibling, and are worth stating rather than + * leaving to be re-derived: + * + * - **There is no per-position suppression here.** `suppressWarnings` is + * declared on the dashboard WIDGET only (`spec/src/ui/dashboard.zod.ts`), + * and none of these three surfaces carries the key. A `warning` is advisory + * (the consumers split on `severity === 'error'`) but cannot be silenced + * individually; declaring the key on these surfaces would be a schema + * change, which this rule has no standing to make. + * - **The page surface's axis refs and series refs are separate limbs.** They + * used to be concatenated into one `series` array before the measure walk, + * so every `yAxis[].field` took the SERIES message. Reading both shapes on + * that surface is deliberate (the props bag mixes them); giving them one + * message was not — the pin refuses the two for different reasons, so they + * now carry different sentences. + * + * `chart-axis-not-selected` (declared measure, outside the selection) rides the + * same walk and took the same one-size sentence — *"the query does not return + * it, so the series plots nothing"*. That is the truth at a query position and + * not at a presentation one, where no series is derived for the name in the + * first place, so its consequence is per position too. */ export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown'; @@ -58,17 +121,16 @@ export interface ChartBindingFinding { hint: string; } -import { suggestName } from './object-graph.js'; +// `recordsOf` — the ONE collection reader (`object-graph.ts`), not the +// hand-copied `asArray` this file used to carry. That copy spelled the array +// branch as an unchecked `v as AnyRec[]`, so a junk member (`reports: [null, +// …]`) was dereferenced rather than skipped and the rule threw instead of +// reporting. Behaviour is otherwise identical, including the map branch that +// keeps a member whose value is not a record under the key the author named +// it with; see that function's header for why the seam reports nothing itself. +import { recordsOf, suggestName } from './object-graph.js'; import { walkPageComponents, type AnyRec } from './page-walk.js'; -function asArray(v: unknown): AnyRec[] { - if (Array.isArray(v)) return v as AnyRec[]; - if (v && typeof v === 'object') { - return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); - } - return []; -} - function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } @@ -94,16 +156,16 @@ interface DatasetNames { function indexDatasets(stack: AnyRec): Map { const out = new Map(); - for (const ds of asArray(stack.datasets)) { + for (const ds of recordsOf(stack.datasets)) { const name = strName(ds.name); if (!name) continue; const dimensions = new Set(); - for (const d of asArray(ds.dimensions)) { + for (const d of recordsOf(ds.dimensions)) { const n = strName(d.name); if (n) dimensions.add(n); } const measures = new Set(); - for (const m of asArray(ds.measures)) { + for (const m of recordsOf(ds.measures)) { const n = strName(m.name); if (n) measures.add(n); } @@ -127,13 +189,83 @@ interface ChartBinding { xAxis?: { name: string; path: string }; /** Single measure ref (report `yAxis`). */ yAxis?: { name: string; path: string }; - /** Series names — measure refs (ChartConfig shape). */ - series?: Array<{ name: string; path: string }>; + /** + * `series[].name` refs (ChartConfig shape) — a display-name / presentation + * override on every surface that has one, never a binding (#15575). `kind` + * names the surface because the two are refused DIFFERENTLY at the pin: a + * report entry is matched against the derived series and dropped when it + * pairs with none; a page component's authored array is replaced wholesale. + */ + series?: Array<{ name: string; path: string; kind: 'report-series' | 'page-series' }>; + /** + * Page-component `yAxis[].field` refs — axis PRESENTATION (#15575). A limb of + * its own rather than more entries of `series`: the axis keeps its slot and + * its scale/chrome while the plotted columns come from `values`, which is a + * different sentence from a series entry that pairs with nothing. + */ + axes?: Array<{ name: string; path: string }>; where: string; /** Path of the chart container, for the dataset-level finding. */ path: string; } +/** + * What a measure name is DOING at the position it was written (#15575). + * + * `query` is the only one the dataset query reads; the rest are presentation + * the pinned renderer refuses as a binding. The distinction decides both the + * severity and the consequence sentence — see the module docblock for the + * per-surface read that produced it. + */ +type MeasurePosition = 'query' | 'report-series' | 'page-series' | 'page-axis'; + +/** Presentation position → what actually happens when the name resolves to nothing. */ +const UNKNOWN_CONSEQUENCE: Record, string> = { + 'report-series': + 'On this surface `chart.series[]` is a per-measure DISPLAY-NAME override, not a ' + + 'binding: the renderer derives the series from the chart\'s own `xAxis`/`yAxis` ' + + 'query and pairs an authored entry with the derived series whose key it EQUALS, so ' + + 'an entry naming no declared measure is ignored. The override lands on nothing — ' + + 'the series is still drawn from the dataset selection.', + 'page-series': + 'On this surface `series[]` is presentation, not a binding: the renderer derives one ' + + 'series per selected measure and REPLACES the authored array with the derived one, ' + + 'so this entry never reaches the chart at all. It lands on nothing — the chart is ' + + 'still drawn from `dimensions`/`values`.', + 'page-axis': + 'On this surface `yAxis[].field` is axis PRESENTATION, not a binding: the entry keeps ' + + 'its slot (the count is what turns on a secondary axis) and its scale/chrome, while ' + + 'the plotted columns come from `values`. The key re-points nothing.', +}; + +/** Presentation position → the shape sentence in the hint. */ +const SHAPE_HINT: Record, string> = { + 'report-series': + '`series[].name` selects WHICH derived series the label and per-series presentation ' + + 'land on; it cannot add, remove or re-point one.', + 'page-series': + '`series[].name` cannot add, remove or re-point a series on a dataset-bound chart — ' + + 'membership belongs to the dataset selection.', + 'page-axis': + '`yAxis[]` carries presentation only (title, min/max, position, gridlines); the ' + + 'plotted columns come from `values`.', +}; + +/** Position → the consequence of naming a DECLARED measure outside the selection. */ +const UNSELECTED_CONSEQUENCE: Record = { + query: 'the query does not return it, so the series plots nothing.', + 'report-series': + 'this entry is a display-name override matched against the ONE series the chart ' + + 'derives (from its own `chart.yAxis` query), so unless it names that measure the ' + + 'override lands on nothing. The series drawn is unaffected.', + 'page-series': + 'the series are derived from `values`, so none is derived for it and this entry ' + + 'lands on nothing. The chart drawn is unaffected.', + 'page-axis': + 'the plotted columns come from `values`, so the axis entry re-points nothing. The ' + + 'chart drawn is unaffected.', +}; + export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { const findings: ChartBindingFinding[] = []; if (!stack || typeof stack !== 'object') return findings; @@ -178,25 +310,40 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { }); }; - const measureRef = (name: string, path: string, selected?: Set) => { + const measureRef = ( + name: string, + path: string, + position: MeasurePosition, + selected?: Set, + ) => { if (!ds.measures.has(name)) { findings.push({ - severity: 'error', + // A query position is the only one where an unknown measure breaks + // the chart; the presentation positions are advisory (#15575, and + // the `chart-field-unknown` ruling one rule over). + severity: position === 'query' ? 'error' : 'warning', rule: CHART_MEASURE_UNKNOWN, where: binding.where, path, message: `"${name}" is not a measure declared by dataset "${dsName}". ` + - `Post-ADR-0021 result rows are keyed by MEASURE NAME (e.g. "sum_amount"), ` + - `not the base field (e.g. "amount"), so this series comes back empty.`, + (position === 'query' + ? `Post-ADR-0021 result rows are keyed by MEASURE NAME (e.g. "sum_amount"), ` + + `not the base field (e.g. "amount"), so this series comes back empty.` + : UNKNOWN_CONSEQUENCE[position]), hint: `Dataset measures: ${list(ds.measures)}.${suggestName(name, ds.measures)} ` + - `Declare the measure on the dataset, or bind an existing one.`, + (position === 'query' + ? `Declare the measure on the dataset, or bind an existing one.` + : `${SHAPE_HINT[position]} Correct the name or drop the entry — this ` + + `surface carries no \`suppressWarnings\` key to silence the advisory with.`), }); return; } - // Declared but not part of this chart's selection: the query never asks - // for it, so the axis still plots nothing. Advisory — the selection may + // Declared, but outside this chart's selection. At a QUERY position that + // means the query never asks for it and the axis plots nothing; at a + // presentation position no series is derived for it at all, so the + // override lands on nothing. Advisory either way — the selection may // legitimately be widened at runtime. if (selected && selected.size > 0 && !selected.has(name)) { findings.push({ @@ -206,9 +353,13 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { path, message: `"${name}" is a declared measure of "${dsName}" but is not in this chart's ` + - `selected values (${list(selected)}) — the query does not return it, ` + - `so the series plots nothing.`, - hint: `Add "${name}" to \`values\`, or point the axis at a selected measure.`, + `selected values (${list(selected)}) — ${UNSELECTED_CONSEQUENCE[position]}`, + hint: + position === 'report-series' + ? `Point the entry at the measure this chart plots (\`chart.yAxis\`), or drop it.` + : `Add "${name}" to \`values\`, or point the ${ + position === 'query' ? 'axis' : 'entry' + } at a selected measure.`, }); } }; @@ -223,16 +374,22 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { const selected = new Set(valSel?.names ?? []); if (valSel) { for (let i = 0; i < valSel.names.length; i++) { - measureRef(valSel.names[i], `${valSel.path}[${i}]`); + // The SELECTION itself — the names the dataset query asks for on every + // surface that has this limb. Always a binding. + measureRef(valSel.names[i], `${valSel.path}[${i}]`, 'query'); } } if (binding.xAxis) dimensionRef(binding.xAxis.name, binding.xAxis.path); - if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, selected); - for (const s of binding.series ?? []) measureRef(s.name, s.path, selected); + if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, 'query', selected); + // Axes before series, the order the page surface reported them in when the + // two shared one limb — the split (#15575) changes the message, not the + // walk. + for (const a of binding.axes ?? []) measureRef(a.name, a.path, 'page-axis', selected); + for (const s of binding.series ?? []) measureRef(s.name, s.path, s.kind, selected); }; // ── 1. Report charts (report.chart + report.blocks[].chart) ── - const reports = asArray(stack.reports); + const reports = recordsOf(stack.reports); for (let ri = 0; ri < reports.length; ri++) { const report = reports[ri]; if (!isRec(report)) continue; @@ -254,9 +411,13 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { values: { names: values, path: `${path}.values` }, xAxis: strName(chart.xAxis) ? { name: strName(chart.xAxis)!, path: `${path}.chart.xAxis` } : undefined, yAxis: strName(chart.yAxis) ? { name: strName(chart.yAxis)!, path: `${path}.chart.yAxis` } : undefined, - series: asArray(chart.series) - .map((s, si) => ({ name: strName(s.name), path: `${path}.chart.series[${si}].name` })) - .filter((s): s is { name: string; path: string } => !!s.name), + series: recordsOf(chart.series) + .map((s, si) => ({ + name: strName(s.name), + path: `${path}.chart.series[${si}].name`, + kind: 'report-series' as const, + })) + .filter((s): s is { name: string; path: string; kind: 'report-series' } => !!s.name), where, path: `${path}.chart`, }); @@ -298,7 +459,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { }); }; - const views = asArray(stack.views); + const views = recordsOf(stack.views); for (let vi = 0; vi < views.length; vi++) { const view = views[vi]; if (!isRec(view)) continue; @@ -311,7 +472,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { } } - const objects = asArray(stack.objects); + const objects = recordsOf(stack.objects); for (let oi = 0; oi < objects.length; oi++) { const obj = objects[oi]; if (!isRec(obj) || !isRec(obj.listViews)) continue; @@ -329,7 +490,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { // A chart component arrives through the untyped `properties` bag. The // presence of a `dataset` key is what marks it dataset-bound (and so // checkable); an object-bound chart has none and is left alone. - const pages = asArray(stack.pages); + const pages = recordsOf(stack.pages); for (let pi = 0; pi < pages.length; pi++) { const page = pages[pi]; if (!isRec(page)) continue; @@ -340,17 +501,24 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { // A page chart mixes the list-chart selection (`dataset`/`dimensions`/ // `values`) with ChartConfig-style axes (`yAxis: [{ field }]`), so both // shapes are read here. - const axisRefs = asArray(props.yAxis) + const axisRefs = recordsOf(props.yAxis) .map((a, ai) => ({ name: strName(a.field), path: `${path}.properties.yAxis[${ai}].field` })) .filter((a): a is { name: string; path: string } => !!a.name); - const seriesRefs = asArray(props.series) - .map((s, si) => ({ name: strName(s.name), path: `${path}.properties.series[${si}].name` })) - .filter((s): s is { name: string; path: string } => !!s.name); + const seriesRefs = recordsOf(props.series) + .map((s, si) => ({ + name: strName(s.name), + path: `${path}.properties.series[${si}].name`, + kind: 'page-series' as const, + })) + .filter((s): s is { name: string; path: string; kind: 'page-series' } => !!s.name); check({ dataset: strName(props.dataset), dimensions: { names: strList(props.dimensions), path: `${path}.properties.dimensions` }, values: { names: strList(props.values), path: `${path}.properties.values` }, - series: [...axisRefs, ...seriesRefs], + // #15575 — two limbs, not one concatenated `series` array: the pin + // refuses an axis `field` and a series `name` for different reasons. + axes: axisRefs, + series: seriesRefs, where: `page "${pageName}" · ${strName(component.type) ?? 'chart'}`, path: `${path}.properties`, });