Skip to content

Commit 89758ac

Browse files
claude[bot]claude
andauthored
fix(lint): chart-axis-not-selected resolves a report chart against its own chart.yAxis (#15789)
The report surface fed `report.values` in as the set `chart-axis-not-selected` resolves against. At the pinned `@object-ui` revision the embedded report chart never queries that set: `DatasetReportRenderer.tsx` runs its own axis-pair query (`useDatasetRows(dataset, [xAxis], [yAxis], …)`) and derives ONE series from it. So the warning at `chart.yAxis` named a query consequence its own pin refutes, and a `series[].name` override was measured against the wrong set in both directions. `checkReportChart` now carries `ownSelection` — the chart's own `{ chart.yAxis }` — which the not-selected limb reads; the check at the `chart.yAxis` position is removed on that surface. `chart-measure-unknown` is untouched everywhere, as are the list-view and page-component surfaces, where `values` IS the query's measure set. Ruled by the maintainer on #15734 (option 1, batch #47). Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent d8d2776 commit 89758ac

3 files changed

Lines changed: 232 additions & 13 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
`chart-axis-not-selected` resolves a report chart against its own `chart.yAxis`, not `report.values` (#15734)
6+
7+
**Behaviour change — one false finding removed on the report surface.** A report chart whose `chart.yAxis` names a declared measure that `report.values` does not select no longer raises a `chart-axis-not-selected` warning. Nothing else about the rule moves, and no other surface moves at all.
8+
9+
The warning stated a query consequence the renderer refutes. Read at the `@object-ui` revision this repo pins (`.objectui-sha`), `plugin-report/src/DatasetReportRenderer.tsx` does not query `report.values` for the chart at all — it runs the chart's own, narrower query out of the two axis strings:
10+
11+
```
12+
const state = useDatasetRows(
13+
dataset,
14+
plan.kind === 'series' && xAxis ? [xAxis] : [],
15+
wantsQuery && yAxis ? [yAxis] : [],
16+
```
17+
18+
and says so in that file's own words at the `scopeOrder` docblock: *"the embedded chart queries only `chart.xAxis` × `chart.yAxis`"*. So the measure the warning said "the query does not return" is exactly the one the query asks for, and the chart plots it. `report.values` is the selection of the TABLE beneath the chart.
19+
20+
Both limbs follow from that one measurement:
21+
22+
- **No not-selected check at the report `chart.yAxis`.** That position IS the chart's query, so it cannot fail to select itself. `chart-measure-unknown` there is untouched: an UNDECLARED measure is still no column at all, and still an `error`.
23+
- **`chart.series[].name` resolves against the singleton `{ chart.yAxis }`.** The entry is a display-name override paired with a DERIVED series, and the chart derives exactly one (`buildChartSeries(…, [xAxis], [yAxis], …)`). An entry naming `chart.yAxis` now lands however the table is selected, and one naming any other declared measure is still reported — including a measure `report.values` does select, which it could not reach before.
24+
25+
The list-view and page-component surfaces are unchanged, and carry firing controls that say so: on both, `values` IS the measure set the query asks for (`ObjectView` hands it to the chart; `ObjectChart` queries `{ dimensions: schema.dimensions, measures: schema.values }`), so the existing resolution is the right one there.
26+
27+
The per-position tier and consequence wording is untouched — only the SET the report surface resolves against moves.

packages/lint/src/validate-chart-bindings.test.ts

Lines changed: 140 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,14 @@ describe('validateChartBindings — report charts', () => {
110110
expect(findings[0].path).toBe('reports[0].chart.xAxis');
111111
});
112112

113-
it('warns when the yAxis measure is declared but not selected', () => {
113+
// #15734 — this fixture WARNED until the set moved, and this pin held the
114+
// warning in place. At the pinned `@object-ui` revision the embedded chart
115+
// runs its OWN query out of the axis pair — `useDatasetRows(dataset,
116+
// plan.kind === 'series' && xAxis ? [xAxis] : [], wantsQuery && yAxis ?
117+
// [yAxis] : [], …)` — so `est_hours` IS asked for and IS plotted. The
118+
// warning stated "the query does not return it", which that query refutes.
119+
// `report.values` selects the TABLE beneath the chart, not the chart.
120+
it('says nothing when the yAxis measure is declared but outside `report.values` — a report chart cannot fail to select what it queries', () => {
114121
const findings = validateChartBindings({
115122
...baseStack(),
116123
reports: [
@@ -122,9 +129,58 @@ describe('validateChartBindings — report charts', () => {
122129
},
123130
],
124131
});
132+
expect(findings).toEqual([]);
133+
});
134+
135+
// The other direction of the same set move: `series[].name` is a display-name
136+
// override paired with a DERIVED series, and the report chart derives exactly
137+
// one — from its own `chart.yAxis`. So the singleton `{ chart.yAxis }` is
138+
// what decides whether an entry lands, in BOTH directions.
139+
it('a report series[].name that names `chart.yAxis` is silent even when `report.values` does not select it', () => {
140+
const findings = validateChartBindings({
141+
...baseStack(),
142+
reports: [
143+
{
144+
name: 'r',
145+
dataset: 'task_metrics',
146+
values: ['task_count'],
147+
chart: {
148+
type: 'bar',
149+
xAxis: 'status',
150+
yAxis: 'est_hours',
151+
series: [{ name: 'est_hours', color: '#f00' }],
152+
},
153+
},
154+
],
155+
});
156+
expect(findings).toEqual([]);
157+
});
158+
159+
it('a report series[].name that names a declared measure OTHER than `chart.yAxis` fires — even one `report.values` does select', () => {
160+
const findings = validateChartBindings({
161+
...baseStack(),
162+
reports: [
163+
{
164+
name: 'r',
165+
dataset: 'task_metrics',
166+
values: ['task_count', 'est_hours'],
167+
chart: {
168+
type: 'bar',
169+
xAxis: 'status',
170+
yAxis: 'est_hours',
171+
series: [{ name: 'task_count' }],
172+
},
173+
},
174+
],
175+
});
125176
expect(findings).toHaveLength(1);
126-
expect(findings[0].severity).toBe('warning');
127177
expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED);
178+
expect(findings[0].severity).toBe('warning');
179+
expect(findings[0].path).toBe('reports[0].chart.series[0].name');
180+
// The SET the message names is the singleton the chart derives, not
181+
// `report.values` — which here selects `task_count` and still cannot make
182+
// the override land.
183+
expect(findings[0].message).toContain("selected values (est_hours)");
128184
});
129185

130186
it('errors on an unresolvable dataset', () => {
@@ -273,13 +329,16 @@ describe('validateChartBindings — binding vs presentation positions (#15575)',
273329
expect(findings[0].hint).toContain('chart.yAxis');
274330
});
275331

276-
it('chart-axis-not-selected at the report yAxis position keeps its wording — that position IS the query', () => {
332+
// #15734 replaced this case rather than re-worded it: the position keeps the
333+
// wording it was given here, but nothing on this surface reaches it any more.
334+
// A report `chart.yAxis` IS the query the chart issues, so it cannot name a
335+
// measure that query does not ask for — the not-selected limb is gone at that
336+
// position, and `chart-measure-unknown` (the case above) is untouched.
337+
it('chart-axis-not-selected does not fire at the report yAxis position at all — that position IS the query', () => {
277338
const findings = validateChartBindings(
278339
reportWith({ type: 'bar', xAxis: 'status', yAxis: 'est_hours' }),
279340
);
280-
expect(findings).toHaveLength(1);
281-
expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED);
282-
expect(findings[0].message).toContain('the query does not return it');
341+
expect(findings).toEqual([]);
283342
});
284343

285344
// ── list-view charts ───────────────────────────────────────────────────
@@ -487,6 +546,36 @@ describe('validateChartBindings — list-view charts', () => {
487546
expect(findings[0].path).toBe('objects[0].listViews.by_status.chart.values[0]');
488547
});
489548

549+
// #15734 — the report surface moved to the chart's own `{ chart.yAxis }`.
550+
// This surface did not, and this control shows it firing: `ObjectView` hands
551+
// `values` to the chart as the dataset measures, so `values` IS the query's
552+
// measure set here and a name outside the dataset still gates at its own
553+
// position. The shape declares no `yAxis`/`series` limb, so no singleton can
554+
// enter this surface in the first place.
555+
it('list-view charts resolve against `values`, unchanged by #15734 — firing control', () => {
556+
const findings = validateChartBindings({
557+
...baseStack(),
558+
views: [
559+
{
560+
name: 'v',
561+
list: {
562+
chart: {
563+
chartType: 'bar',
564+
dataset: 'task_metrics',
565+
dimensions: ['status'],
566+
values: ['task_count', 'estimate_hours'],
567+
},
568+
},
569+
},
570+
],
571+
});
572+
expect(findings).toHaveLength(1);
573+
expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN);
574+
expect(findings[0].severity).toBe('error');
575+
expect(findings[0].path).toBe('views[0].list.chart.values[1]');
576+
expect(findings[0].message).toContain('this series comes back empty');
577+
});
578+
490579
it('accepts a fully resolved list chart', () => {
491580
const findings = validateChartBindings({
492581
...baseStack(),
@@ -539,6 +628,51 @@ describe('validateChartBindings — dataset-bound page chart components', () =>
539628
expect(findings[0].severity).toBe('warning');
540629
});
541630

631+
// #15734 — the companion control on the page surface, where
632+
// `chart-axis-not-selected` DOES have presentation limbs to fire at.
633+
// `ObjectChart` queries `{ dimensions: schema.dimensions, measures:
634+
// schema.values }`, so both limbs stay resolved against `values`: the SET
635+
// named in each message is the page chart's own `values`, never a
636+
// `{ yAxis[0].field }` singleton borrowed from the report surface.
637+
it('page-component charts resolve against `values`, unchanged by #15734 — firing control on both limbs', () => {
638+
const findings = validateChartBindings({
639+
...baseStack(),
640+
pages: [
641+
{
642+
name: 'p',
643+
regions: [
644+
{
645+
name: 'main',
646+
components: [
647+
{
648+
type: 'object-chart',
649+
properties: {
650+
dataset: 'task_metrics',
651+
dimensions: ['status'],
652+
values: ['task_count'],
653+
yAxis: [{ field: 'est_hours' }],
654+
series: [{ name: 'est_hours' }],
655+
},
656+
},
657+
],
658+
},
659+
],
660+
},
661+
],
662+
});
663+
expect(findings).toHaveLength(2);
664+
expect(findings.every((f) => f.rule === CHART_AXIS_NOT_SELECTED)).toBe(true);
665+
expect(findings.every((f) => f.severity === 'warning')).toBe(true);
666+
expect(findings[0].path).toBe(
667+
'pages[0].regions[0].components[0].properties.yAxis[0].field',
668+
);
669+
expect(findings[1].path).toBe(
670+
'pages[0].regions[0].components[0].properties.series[0].name',
671+
);
672+
expect(findings[0].message).toContain('selected values (task_count)');
673+
expect(findings[1].message).toContain('selected values (task_count)');
674+
});
675+
542676
it('accepts a resolved page chart', () => {
543677
const findings = validateChartBindings({
544678
...baseStack(),

packages/lint/src/validate-chart-bindings.ts

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,33 @@
9898
* it, so the series plots nothing"*. That is the truth at a query position and
9999
* not at a presentation one, where no series is derived for the name in the
100100
* first place, so its consequence is per position too.
101+
*
102+
* ## Which SET `chart-axis-not-selected` resolves against (#15734)
103+
*
104+
* The consequence is per POSITION; the selection it is measured against is per
105+
* SURFACE — and on the report surface that selection is not `report.values`.
106+
* At the same pinned revision `DatasetReportRenderer.tsx` runs
107+
* `useDatasetRows(dataset, plan.kind === 'series' && xAxis ? [xAxis] : [],
108+
* wantsQuery && yAxis ? [yAxis] : [], …)` and derives the plotted series with
109+
* `buildChartSeries(…, [xAxis], [yAxis], …)` — *"the selection is exactly one
110+
* dimension × one measure, so this takes the helper's single-dimension branch
111+
* and returns ONE series"*. `report.values` is the selection of the TABLE
112+
* beneath the chart; the chart's own is the axis pair. Two things follow:
113+
*
114+
* - **Nothing to report at the report `chart.yAxis`.** That position IS the
115+
* chart's query, so it cannot fail to select itself. The warning that fired
116+
* for a declared measure outside `report.values` stated a query consequence
117+
* its own pin refutes: the chart asks for exactly that measure and plots
118+
* it. `chart-measure-unknown` at that position is untouched — an UNDECLARED
119+
* measure still returns no column, and still gates.
120+
* - **`chart.series[].name` resolves against the singleton `{ chart.yAxis }`.**
121+
* The override is paired with a DERIVED series and the chart derives one,
122+
* so that singleton — not `report.values` — is the set an entry can land
123+
* on. An entry naming `chart.yAxis` lands however the table is selected;
124+
* one naming any other declared measure lands on nothing.
125+
*
126+
* The list-view and page-component surfaces keep `values`: there it IS the
127+
* measure set the query asks for, so the existing resolution is correct.
101128
*/
102129

103130
export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown';
@@ -204,6 +231,19 @@ interface ChartBinding {
204231
* different sentence from a series entry that pairs with nothing.
205232
*/
206233
axes?: Array<{ name: string; path: string }>;
234+
/**
235+
* The measures THIS chart's own query selects, when that is narrower than
236+
* the `values` limb above (#15734). Set on the report surface and nowhere
237+
* else: the embedded report chart queries `chart.xAxis` × `chart.yAxis`
238+
* alone and derives exactly ONE series from it, while `report.values` is the
239+
* selection of the table beneath it. `chart-axis-not-selected` resolves
240+
* against this set where it is present — which is both why a `series[].name`
241+
* override is measured against the singleton `{ chart.yAxis }`, and why the
242+
* report `chart.yAxis` carries no not-selected check at all (a chart cannot
243+
* fail to select what it queries). The other two surfaces leave it undefined
244+
* and resolve against `values`, which on them IS the query's measure set.
245+
*/
246+
ownSelection?: string[];
207247
where: string;
208248
/** Path of the chart container, for the dataset-level finding. */
209249
path: string;
@@ -372,6 +412,11 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
372412
}
373413
const valSel = binding.values;
374414
const selected = new Set(valSel?.names ?? []);
415+
// What this chart DERIVES A SERIES FOR — the set `chart-axis-not-selected`
416+
// is measured against (#15734). Equal to the selection above wherever the
417+
// chart IS the surface's selection; the narrower `{ chart.yAxis }` on the
418+
// report surface, whose chart runs its own axis-pair query.
419+
const derived = binding.ownSelection ? new Set(binding.ownSelection) : selected;
375420
if (valSel) {
376421
for (let i = 0; i < valSel.names.length; i++) {
377422
// The SELECTION itself — the names the dataset query asks for on every
@@ -380,12 +425,16 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
380425
}
381426
}
382427
if (binding.xAxis) dimensionRef(binding.xAxis.name, binding.xAxis.path);
383-
if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, 'query', selected);
428+
// No selection is passed at this position: the report `yAxis` IS the query
429+
// the chart issues, so it cannot name a measure that query does not ask
430+
// for (#15734). The `chart-measure-unknown` half still runs — an undeclared
431+
// measure is no column at all.
432+
if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, 'query');
384433
// Axes before series, the order the page surface reported them in when the
385434
// two shared one limb — the split (#15575) changes the message, not the
386435
// walk.
387-
for (const a of binding.axes ?? []) measureRef(a.name, a.path, 'page-axis', selected);
388-
for (const s of binding.series ?? []) measureRef(s.name, s.path, s.kind, selected);
436+
for (const a of binding.axes ?? []) measureRef(a.name, a.path, 'page-axis', derived);
437+
for (const s of binding.series ?? []) measureRef(s.name, s.path, s.kind, derived);
389438
};
390439

391440
// ── 1. Report charts (report.chart + report.blocks[].chart) ──
@@ -403,14 +452,23 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
403452
path: string,
404453
) => {
405454
if (!isRec(chart)) return;
455+
const yAxisName = strName(chart.yAxis);
406456
check({
407457
dataset,
408-
// `values` is the report's measure SELECTION, not a chart ref; feeding
409-
// it in lets the yAxis "declared but not selected" check work without
410-
// reporting the selection itself twice.
458+
// `values` is the REPORT's measure selection — what the table beneath
459+
// the chart shows — not a chart ref. It is fed in so those names are
460+
// resolved against the dataset (`chart-measure-unknown`) once, here,
461+
// rather than reported twice or not at all. It is NOT what the chart
462+
// queries, so it is no longer the set `chart-axis-not-selected` reads
463+
// on this surface; `ownSelection` below is (#15734).
411464
values: { names: values, path: `${path}.values` },
412465
xAxis: strName(chart.xAxis) ? { name: strName(chart.xAxis)!, path: `${path}.chart.xAxis` } : undefined,
413-
yAxis: strName(chart.yAxis) ? { name: strName(chart.yAxis)!, path: `${path}.chart.yAxis` } : undefined,
466+
yAxis: yAxisName ? { name: yAxisName, path: `${path}.chart.yAxis` } : undefined,
467+
// The chart's own selection: the ONE measure it queries and derives a
468+
// series from. Empty when no `yAxis` is authored — the chart plots
469+
// nothing at all then, which the shape rules own, so no override can
470+
// be reported as landing on nothing.
471+
ownSelection: yAxisName ? [yAxisName] : [],
414472
series: recordsOf(chart.series)
415473
.map((s, si) => ({
416474
name: strName(s.name),

0 commit comments

Comments
 (0)