From 0c2504b2d0312408a7f0a1c55f5f1a33be525b89 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 16:40:33 +0000 Subject: [PATCH] feat(spec,lint): pin aggregate result-column naming and validate its axes (#3701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3684 extended ADR-0021 axis checking to report charts, list-view charts and dataset-bound page chart components, but had to leave the react block out: it is OBJECT-bound (objectName + an inline aggregate), `aggregate` existed in the contract only as the description string '{ field, function, groupBy }', and nothing in the repo said what the aggregated result columns were called. With no columns to resolve against, its axis bindings could not be validated, and inventing a convention would have manufactured false positives (ADR-0072 D1). Record the convention rather than invent one. Every path that can serve an object-bound chart already agreed: the engine's structured-groupBy aggregate (whose alias objectui sets to `field || function`), the legacy analytics query (which remaps its measure key back to `field`), the client-side fallback, and the console's own chart-view wiring (`xAxisKey: groupBy`, `series[].dataKey: field`). An object-bound aggregate returns rows keyed by the RAW FIELD NAMES — `groupBy` for the category column, `field` for the value column, the literal `count` for a fieldless count, plus `__comparison` under a comparison overlay. That is the deliberate opposite of the dataset path, whose rows are keyed by the declared measure `name`; only the dataset path has an author-chosen name to key by. spec: - ChartAggregateSchema / ChartGroupBySchema / ChartAggregateFunctionSchema in chart.zod.ts replace the description string, and reject a non-count function with no field (which reached the renderer as sum(undefined) and drew a blank chart). - chart-aggregate.ts records the convention and exports the derivations — chartAggregateCategoryKey / chartAggregateValueKey / chartAggregateResultKeys — so producers and checkers cannot drift apart by re-deriving the rule separately. - 's contract now names the props the block actually reads. ChartConfig's xAxis/yAxis/series shapes reached it and were silently dropped, which ADR-0078 forbids; they leave dataProps, and chartType, xAxisKey and series join the React overlay where the other bindings live. lint: validate-react-page-props now reads attribute VALUES, not just names, for — react-chart-field-unknown (aggregate.field/groupBy off the object), react-chart-aggregate-invalid (unimplemented function, or a non-count function with nothing to aggregate), react-chart-axis-unknown (an axis naming a column the aggregate never returns, incl. a dataset-style sum_total, or a category axis bound to the value column), react-chart-axis-inert (the axis shapes this block never reads). Value reading evaluates only static literals: a prop driven by React state, a usage carrying a spread, a chart given inline data, and objects another package defines are skipped — an unresolvable binding is not a wrong one. The golden page binds its axes explicitly so the chain is proven end to end: swapping `total` for `sum_total` there fails `os validate` with the new rule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UMHZBHTjH4rw8xmDYirFC7 --- .../objectchart-aggregate-result-columns.md | 60 ++++ .../docs/deployment/validating-metadata.mdx | 36 ++- content/docs/references/ui/chart.mdx | 63 +++- .../src/ui/pages/renewals-pipeline.page.ts | 7 +- packages/lint/src/index.ts | 8 +- packages/lint/src/validate-chart-bindings.ts | 17 +- .../src/validate-react-page-props.test.ts | 174 ++++++++++- .../lint/src/validate-react-page-props.ts | 282 +++++++++++++++++- packages/spec/api-surface.json | 11 + packages/spec/json-schema.manifest.json | 3 + packages/spec/src/ui/chart-aggregate.test.ts | 96 ++++++ packages/spec/src/ui/chart-aggregate.ts | 139 +++++++++ packages/spec/src/ui/chart.zod.ts | 86 ++++++ packages/spec/src/ui/index.ts | 1 + packages/spec/src/ui/react-blocks.ts | 15 +- skills/objectstack-ui/SKILL.md | 17 ++ .../contracts/react-blocks.contract.json | 48 +-- .../evals/analytics-inline-vs-dataset.json | 2 +- .../objectstack-ui/references/react-blocks.md | 10 +- 19 files changed, 1023 insertions(+), 52 deletions(-) create mode 100644 .changeset/objectchart-aggregate-result-columns.md create mode 100644 packages/spec/src/ui/chart-aggregate.test.ts create mode 100644 packages/spec/src/ui/chart-aggregate.ts diff --git a/.changeset/objectchart-aggregate-result-columns.md b/.changeset/objectchart-aggregate-result-columns.md new file mode 100644 index 0000000000..37b40b8397 --- /dev/null +++ b/.changeset/objectchart-aggregate-result-columns.md @@ -0,0 +1,60 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +--- + +`` aggregate result-column naming is now a contract, and its axis bindings are validated (issue #3701) + +Split out of #3583 Phase 2 (#3684), which extended ADR-0021 axis checking to +report charts, list-view charts, and dataset-bound page chart components but had +to leave the react `` block out: it is OBJECT-bound (`objectName` + +an inline `aggregate`), `aggregate` existed in the contract only as the +description string `'{ field, function, groupBy }'`, and nothing in the repo said +what the aggregated result columns were called. Without that, `xAxis`/`yAxis` had +nothing to resolve against, and guessing a convention would have manufactured +false positives (ADR-0072 D1). + +**The convention, recorded rather than invented.** Every path that can serve an +object-bound chart already agreed — the engine's structured-`groupBy` aggregate +(whose alias objectui sets to `field || function`), the legacy analytics query +(which remaps its measure key back to `field`), the client-side fallback, and the +console's own chart-view wiring (`xAxisKey: groupBy`, `series[].dataKey: field`). +`packages/spec/src/ui/chart-aggregate.ts` writes it down and exports it: + +* an object-bound aggregate returns rows keyed by the **raw field names** — + `groupBy` for the category column, `field` for the value column, the literal + `count` for a fieldless count, plus `__comparison` under a comparison + overlay; +* `chartAggregateCategoryKey` / `chartAggregateValueKey` / `chartAggregateResultKeys` + derive those columns so producers and checkers cannot re-derive them apart; +* `ChartAggregateSchema` replaces the description string with a real Zod schema + and rejects a non-`count` function with no `field` (which used to reach the + renderer as `sum(undefined)` and render blank). + +This is the deliberate opposite of the dataset path, whose rows are keyed by the +declared measure `name` (`sum_amount`) — the trap `chart-measure-unknown` catches. +Only the dataset path has an author-chosen name to key by. + +**``'s contract now names the props it actually reads.** The block +consumes `xAxisKey` and `series[].dataKey`; `ChartConfig`'s `xAxis`/`yAxis`/`series` +shapes reached it and were silently dropped, which ADR-0078 forbids. They are +removed from the block's `dataProps`; `chartType`, `xAxisKey`, and `series` are +declared in the React overlay where the other bindings live. + +**`validate-react-page-props` now reads attribute VALUES**, not just names, for +``: + +* `react-chart-field-unknown` (error) — `aggregate.field` / `aggregate.groupBy` + naming a field the bound object does not declare; +* `react-chart-aggregate-invalid` (error) — an unimplemented aggregation + function, or a non-`count` function with nothing to aggregate; +* `react-chart-axis-unknown` (error) — `xAxisKey` / `series[].dataKey` naming a + column the aggregate does not return (including a dataset-style `sum_total`), + or a category axis bound to the value column; +* `react-chart-axis-inert` (warning) — the `xAxis` / `yAxis` shapes this block + never reads. + +Value reading is opt-in per block and evaluates only static literals: a prop +driven by React state or a variable, a usage carrying a `{...spread}`, a chart +given inline `data`, and objects another package defines are all skipped +silently — an unresolvable binding is not a wrong one. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 83b7db800f..51367511fa 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -156,8 +156,12 @@ chart: { type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' } An axis naming a measure the dataset declares but this chart does not *select* (not in `values`) is a **warning**: the query never returns it, so it plots -nothing. The react `` block is object-bound rather than -dataset-bound and is not checked here. +nothing. + +The react `` block is **object-bound** (`objectName` + an inline +`aggregate`) rather than dataset-bound, so it is checked by the react-page prop +gate below against a different rule — its result rows are keyed by the raw field +names, exactly the opposite of the dataset case. ### 7. Navigation exposing objects nobody can read @@ -177,6 +181,34 @@ ships. Skipped for platform-provided objects (whose own packages grant them), for stacks that declare no permission sets at all, and when any set carries a wildcard (`objects: { '*': … }`) grant. +### 8. `` axes bound to the wrong result column + +A `kind:'react'` page's `` is **object-bound**: `objectName` plus an +inline `aggregate`, run as one ad-hoc query. Its rows come back keyed by the +**raw field names** the aggregate was given — `groupBy` names the category +column, `field` names the value column (the literal `count` for a fieldless +`count`). That is the exact opposite of the dataset case in §6, and mixing the +two conventions up is the usual cause of a chart that renders axes and no bars. + +```jsx + +// ↑ a dataset-style measure name; the rows +// are keyed `total` → error +``` + +Both halves are checked: `aggregate.field` / `aggregate.groupBy` must be fields +the object declares, and `xAxisKey` / `series[].dataKey` must name a column the +aggregate actually returns (plus `__comparison` when a comparison overlay +is on). `ChartConfig`'s `xAxis` / `yAxis` shapes are **not** read by this block +and are reported as inert (warning). + +Skipped, to keep false positives at zero: any prop whose value is not a static +literal (it comes from React state or a variable), a usage carrying a `{...spread}`, +a chart given static `data` (its columns are the author's own), and objects +another package defines. + ## The one gate, two entry points `os validate` and `os build` (alias of `os compile`) run the **same** validator: diff --git a/content/docs/references/ui/chart.mdx b/content/docs/references/ui/chart.mdx index f2c0b07d77..d9e8acbbb7 100644 --- a/content/docs/references/ui/chart.mdx +++ b/content/docs/references/ui/chart.mdx @@ -18,13 +18,41 @@ Provides a comprehensive set of chart types for data visualization. ## TypeScript Usage ```typescript -import { ChartAnnotation, ChartAxis, ChartConfig, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui'; -import type { ChartAnnotation, ChartAxis, ChartConfig, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui'; +import { ChartAggregate, ChartAggregateFunction, ChartAnnotation, ChartAxis, ChartConfig, ChartGroupBy, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui'; +import type { ChartAggregate, ChartAggregateFunction, ChartAnnotation, ChartAxis, ChartConfig, ChartGroupBy, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui'; // Validate data -const result = ChartAnnotation.parse(data); +const result = ChartAggregate.parse(data); ``` +--- + +## ChartAggregate + +Inline aggregation for an object-bound chart + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **field** | `string` | optional | Field to aggregate — required for sum/avg/min/max, optional for count | +| **function** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max'>` | ✅ | Aggregation function | +| **groupBy** | `string \| { field: string; dateGranularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; alias?: string }` | ✅ | Field the rows are grouped by — the chart category axis | + + +--- + +## ChartAggregateFunction + +### Allowed Values + +* `count` +* `sum` +* `avg` +* `min` +* `max` + + --- ## ChartAnnotation @@ -85,6 +113,35 @@ const result = ChartAnnotation.parse(data); | **aria** | `{ ariaLabel?: string; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes | +--- + +## ChartGroupBy + +### Union Options + +This schema accepts one of the following structures: + +#### Option 1 + +Field to group by + +Type: `string` + +--- + +#### Option 2 + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **field** | `string` | ✅ | Field to group by | +| **dateGranularity** | `Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>` | optional | Bucket date values into uniform periods | +| **alias** | `string` | optional | Alias for the projected group value (defaults to `field`) — this becomes the category column | + +--- + + --- ## ChartInteraction diff --git a/examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts b/examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts index 815b8ca2f1..de55230dae 100644 --- a/examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts +++ b/examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts @@ -18,6 +18,11 @@ import { definePage } from '@objectstack/spec/ui'; * cross-object reads declaratively (zero data code). * (This comparison absorbed the former Account Cockpit page.) * + * The chart's axes are bound explicitly (#3701) to show the object-bound + * result-column rule: an inline `aggregate` returns rows keyed by the RAW + * FIELD NAMES — `status` (its `groupBy`) and `total` (its `field`) — not by a + * dataset-style measure name. `os validate` now checks both halves. + * * Styling (ADR-0065): no Tailwind — inline `style={{}}` with `hsl(var(--token))`; * data blocks and the drawer bring their own compiled styling. The drawer sets * NO pixel width: per #2578 pixel widths are deprecated (the author can't know @@ -114,7 +119,7 @@ function Page() { - + diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 251266ecbd..05643cd139 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -75,7 +75,13 @@ export { validateJsxPages } from './validate-jsx-pages.js'; export type { JsxPageFinding, JsxPageSeverity } from './validate-jsx-pages.js'; export { validateReactPages } from './validate-react-pages.js'; export type { ReactPageFinding, ReactPageSeverity } from './validate-react-pages.js'; -export { validateReactPageProps } from './validate-react-page-props.js'; +export { + validateReactPageProps, + REACT_CHART_FIELD_UNKNOWN, + REACT_CHART_AGGREGATE_INVALID, + REACT_CHART_AXIS_UNKNOWN, + REACT_CHART_AXIS_INERT, +} from './validate-react-page-props.js'; export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js'; export { validatePageSourceStyling, PAGE_SOURCE_CLASSNAME } from './validate-page-source-styling.js'; export type { SourceStyleFinding, SourceStyleSeverity } from './validate-page-source-styling.js'; diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index cf593e0fb7..5b893128b9 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -27,15 +27,14 @@ * binding shape as a list chart, but it arrives through the untyped * `properties` bag. * - * Deliberately NOT covered: the react `` block. It is - * OBJECT-bound (`objectName` + an inline `aggregate`), not dataset-bound, and - * nothing in the repo pins down what the runtime names the aggregated result - * column — so there is no defensible answer for what its `yAxis[].field` - * should resolve against. Guessing one would manufacture false positives on - * the single authored usage. Its `aggregate.field` / `groupBy` ARE checkable - * against the object's fields, but only by reading JSX attribute values, which - * `validate-react-page-props` does not do today. Left for a follow-up rather - * than half-built (ADR-0078 §5: verify, then enforce). + * Not covered HERE, and deliberately so: the react `` block. It is + * OBJECT-bound (`objectName` + an inline `aggregate`), so its result rows are + * keyed by the RAW FIELD NAMES rather than by a measure name — the opposite + * convention, which would make `chart-measure-unknown`'s message a lie. It also + * arrives as JSX rather than config, so it needs the TypeScript compiler this + * 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). */ export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown'; diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 4380949ddf..c76503a8de 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -1,6 +1,12 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { validateReactPageProps } from './validate-react-page-props.js'; +import { + validateReactPageProps, + REACT_CHART_FIELD_UNKNOWN, + REACT_CHART_AGGREGATE_INVALID, + REACT_CHART_AXIS_UNKNOWN, + REACT_CHART_AXIS_INERT, +} from './validate-react-page-props.js'; const page = (source: string) => ({ pages: [{ name: 'p', kind: 'react', source }] }); @@ -40,3 +46,169 @@ describe('validateReactPageProps (ADR-0081 Phase 2)', () => { expect(f).toEqual([]); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// binding integrity (#3701) +// ───────────────────────────────────────────────────────────────────────── + +const invoice = { + name: 'invoice', + fields: [{ name: 'total' }, { name: 'status' }, { name: 'closed_at' }], +}; + +const chartPage = (source: string, objects: unknown[] = [invoice]) => ({ + objects, + pages: [{ name: 'p', kind: 'react', source }], +}); + +const chart = (attrs: string) => `function Page(){ return ; }`; + +describe('validateReactPageProps — bindings (#3701)', () => { + it('passes an aggregate whose field and groupBy exist on the object', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }}`)), + ); + expect(f).toEqual([]); + }); + + it('flags an aggregate.field the object does not declare', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'amount', function: 'sum', groupBy: 'status' }}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_FIELD_UNKNOWN && /aggregate\.field "amount"/.test(x.message))).toBe(true); + }); + + it('flags an aggregate.groupBy the object does not declare', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'stage' }}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_FIELD_UNKNOWN && /aggregate\.groupBy "stage"/.test(x.message))).toBe(true); + }); + + it('reads the field out of a structured, date-bucketed groupBy', () => { + const ok = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: { field: 'closed_at', dateGranularity: 'quarter' } }}`)), + ); + expect(ok).toEqual([]); + const bad = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: { field: 'signed_at' } }}`)), + ); + expect(bad.some((x) => x.rule === REACT_CHART_FIELD_UNKNOWN && /signed_at/.test(x.message))).toBe(true); + }); + + it('flags an aggregation function no chart renderer implements', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'median', groupBy: 'status' }}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_AGGREGATE_INVALID && /median/.test(x.message))).toBe(true); + }); + + it('flags a non-count function with nothing to aggregate', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ function: 'sum', groupBy: 'status' }}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_AGGREGATE_INVALID && /no "field"/.test(x.message))).toBe(true); + }); + + it('accepts a fieldless count', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ function: 'count', groupBy: 'status' }} series={[{ dataKey: 'count' }]}`)), + ); + expect(f).toEqual([]); + }); + + it('accepts axes bound to the aggregate result columns', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="status" series={[{ dataKey: 'total' }]}`)), + ); + expect(f).toEqual([]); + }); + + it('flags a series bound to the DATASET-style measure name', () => { + // The exact bug #3684 catches on dataset charts, on the object-bound path: + // an object aggregate keys its rows `total`, never `sum_total`. + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} series={[{ dataKey: 'sum_total' }]}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_AXIS_UNKNOWN && /sum_total/.test(x.message))).toBe(true); + }); + + it('flags an xAxisKey that is neither result column', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="closed_at"`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_AXIS_UNKNOWN && /closed_at/.test(x.message))).toBe(true); + }); + + it('flags an xAxisKey bound to the VALUE column (a real column, wrong axis)', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="total"`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_AXIS_UNKNOWN && /VALUE column/.test(x.message))).toBe(true); + }); + + it('accepts the comparison-overlay column', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} series={[{ dataKey: 'total' }, { dataKey: 'total__comparison' }]}`)), + ); + expect(f).toEqual([]); + }); + + it('warns that the ChartConfig axis shapes are inert on this block', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} yAxis={[{ field: 'total' }]}`)), + ); + const inert = f.find((x) => x.rule === REACT_CHART_AXIS_INERT); + expect(inert?.severity).toBe('warning'); + expect(inert?.hint).toMatch(/series=/); + }); + + // ── false-positive guards ─────────────────────────────────────────────── + + it('skips an aggregate built from variables (not knowable at build time)', () => { + const f = validateReactPageProps( + chartPage('function Page(){ const agg = { field: f, function: "sum", groupBy: g }; return ; }'), + ); + expect(f).toEqual([]); + }); + + it('skips a shorthand property (its value is hidden)', () => { + const f = validateReactPageProps( + chartPage('function Page(){ const field = "nope"; return ; }'), + ); + expect(f).toEqual([]); + }); + + it('skips an object declared in another package', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="sys_user" aggregate={{ field: 'nope', function: 'sum', groupBy: 'also_nope' }}`)), + ); + expect(f.some((x) => x.rule === REACT_CHART_FIELD_UNKNOWN)).toBe(false); + }); + + it('does not flag system fields the registry injects', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" aggregate={{ function: 'count', groupBy: 'owner_id' }}`)), + ); + expect(f).toEqual([]); + }); + + it('skips the axis check when static `data` supplies the rows', () => { + const f = validateReactPageProps( + chartPage(chart(`objectName="invoice" data={[{ x: 1 }]} aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="x"`)), + ); + expect(f).toEqual([]); + }); + + it('skips everything behind a spread', () => { + const f = validateReactPageProps( + chartPage('function Page(){ const p = {}; return ; }'), + ); + expect(f).toEqual([]); + }); + + it('leaves a chart with no aggregate alone (dataset-bound or data-bound)', () => { + const f = validateReactPageProps(chartPage(chart(`objectName="invoice"`))); + expect(f).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index 577a127978..cf04721765 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -14,10 +14,19 @@ // (e.g. `onSucces` → `onSuccess`) → warning. We do NOT flag arbitrary // unknown props (the contract's data props are a curated subset) — only the // likely typos, to keep false positives near zero. +// - 's data BINDINGS, by reading the attribute VALUES (#3701). +// See the block comment above `checkObjectChart` for why that block, and +// why only now. +// +// Reading values is opt-in per block and per prop: everything below evaluates +// only STATIC literals (`objectName="invoice"`, an `aggregate={{…}}` object +// literal). A value that comes from a variable, a call, or a spread is not +// knowable at build time and is skipped silently — an unresolvable binding is +// not a wrong one (ADR-0072 D1). import { createRequire } from 'node:module'; import type ts from 'typescript'; -import { REACT_BLOCKS } from '@objectstack/spec/ui'; +import { REACT_BLOCKS, chartAggregateResultKeys } from '@objectstack/spec/ui'; // The TypeScript compiler must NOT be imported at module top level: it is // ~9 MB of CJS (~70 ms+ to parse, worse on container cold starts), and @@ -107,8 +116,267 @@ function nearestKnown(prop: string, known: Set): string | null { return bestD <= 2 ? best : null; } +// ─── Static attribute values ────────────────────────────────────────────── +// +// The gate above only needed prop NAMES. Checking a binding needs its VALUE, +// and a JSX attribute's value is an arbitrary expression. `NOT_STATIC` is the +// sentinel for "this expression is not knowable at build time" — distinct from +// a literal `undefined`, which IS knowable. + +const NOT_STATIC = Symbol('not-static'); + +/** Evaluate a JSX expression to a plain JS value, or `NOT_STATIC`. */ +function staticValue(tsc: typeof ts, sf: ts.SourceFile, node: ts.Node | undefined): unknown { + if (!node) return NOT_STATIC; + if (tsc.isParenthesizedExpression(node)) return staticValue(tsc, sf, node.expression); + if (tsc.isStringLiteral(node) || tsc.isNoSubstitutionTemplateLiteral(node)) return node.text; + if (tsc.isNumericLiteral(node)) return Number(node.text); + if (node.kind === tsc.SyntaxKind.TrueKeyword) return true; + if (node.kind === tsc.SyntaxKind.FalseKeyword) return false; + if (node.kind === tsc.SyntaxKind.NullKeyword) return null; + if (tsc.isArrayLiteralExpression(node)) { + const out: unknown[] = []; + for (const el of node.elements) { + const v = staticValue(tsc, sf, el); + if (v === NOT_STATIC) return NOT_STATIC; + out.push(v); + } + return out; + } + if (tsc.isObjectLiteralExpression(node)) { + const out: Record = {}; + for (const p of node.properties) { + // A shorthand (`{ field }`) or spread (`{ ...cfg }`) hides its value. + if (!tsc.isPropertyAssignment(p)) return NOT_STATIC; + const key = tsc.isIdentifier(p.name) || tsc.isStringLiteral(p.name) ? p.name.text : null; + if (key === null) return NOT_STATIC; + const v = staticValue(tsc, sf, p.initializer); + if (v === NOT_STATIC) return NOT_STATIC; + out[key] = v; + } + return out; + } + return NOT_STATIC; +} + +/** The static value of one JSX attribute (`x="s"` or `x={…}`), or `NOT_STATIC`. */ +function attrValue(tsc: typeof ts, sf: ts.SourceFile, attr: ts.JsxAttribute): unknown { + const init = attr.initializer; + if (!init) return true; // bare `showLegend` — JSX shorthand for `={true}` + if (tsc.isStringLiteral(init)) return init.text; + if (tsc.isJsxExpression(init)) return staticValue(tsc, sf, init.expression); + return NOT_STATIC; +} + +// ─── binding integrity (#3701) ────────────────────────────── +// +// `validate-chart-bindings` covers every DATASET-bound chart surface: a +// dataset declares its dimensions/measures by NAME, result rows are keyed by +// those names, and an axis is checked against them. The react `` +// block was excluded because it is OBJECT-bound — `objectName` + an inline +// `aggregate` — and nothing said what the aggregated result columns were +// called, so `xAxisKey`/`series[].dataKey` had nothing to resolve against. +// +// #3701 closed that: `chartAggregateResultKeys` in @objectstack/spec/ui now +// records the convention every renderer already implements (rows keyed by the +// RAW FIELD NAMES — `groupBy` for the category, `field` for the value, the +// literal `'count'` for a fieldless count). With the columns pinned, both +// halves of the binding are checkable: +// +// * `aggregate.field` / `aggregate.groupBy` are RAW FIELD names → check them +// against the object's declared fields. +// * `xAxisKey` / `series[].dataKey` are RESULT COLUMN names → check them +// against what this aggregate produces. + +export const REACT_CHART_FIELD_UNKNOWN = 'react-chart-field-unknown'; +export const REACT_CHART_AGGREGATE_INVALID = 'react-chart-aggregate-invalid'; +export const REACT_CHART_AXIS_UNKNOWN = 'react-chart-axis-unknown'; +export const REACT_CHART_AXIS_INERT = 'react-chart-axis-inert'; + +const CHART_FUNCTIONS = ['count', 'sum', 'avg', 'min', 'max'] as const; + +/** + * Registry-injected fields present on (almost) every object but absent from + * `object.fields`. Same set as `validate-page-field-bindings`, for the same + * reason: over-inclusion costs at worst a missed finding, under-inclusion + * costs a false one. + */ +const SYSTEM_FIELDS = new Set([ + 'id', + 'created_at', 'created_by', 'updated_at', 'updated_by', + 'owner_id', 'organization_id', 'tenant_id', 'user_id', + 'deleted_at', +]); + +/** + * Both `objects` and an object's `fields` are authored either as an array of + * `{ name }` records or as a name-keyed map — normalize to the array form, the + * same way the other reference-integrity rules do. + */ +function namedArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v.filter((x): x is AnyRec => !!x && typeof x === 'object'); + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ + name, + ...(def && typeof def === 'object' ? (def as AnyRec) : {}), + })); + } + return []; +} + +/** object name → its declared field names. */ +function indexObjectFields(stack: AnyRec): Map> { + const out = new Map>(); + for (const obj of namedArray(stack.objects)) { + const name = typeof obj.name === 'string' ? obj.name : undefined; + if (!name) continue; + const names = new Set(); + for (const f of namedArray(obj.fields)) { + if (typeof f.name === 'string' && f.name) names.add(f.name); + } + out.set(name, names); + } + return out; +} + +const isRec = (v: unknown): v is Record => + !!v && typeof v === 'object' && !Array.isArray(v); + +const strOf = (v: unknown): string | undefined => + typeof v === 'string' && v.length > 0 ? v : undefined; + +interface ChartAttrs { + /** Statically resolved attribute values, keyed by prop name. */ + values: Map; + where: string; + path: string; +} + +function checkObjectChart( + attrs: ChartAttrs, + objectFields: Map>, + findings: ReactPropFinding[], +): void { + const { values, where, path } = attrs; + const push = (severity: ReactPropSeverity, rule: string, message: string, hint: string) => + findings.push({ severity, rule, where, path, message, hint }); + + // The ChartConfig axis shapes reach this block but are never read by it — + // it consumes `xAxisKey` + `series[].dataKey`. Silently inert props are what + // ADR-0078 forbids, so say so rather than let the chart render blank. + for (const inert of ['xAxis', 'yAxis'] as const) { + if (!values.has(inert)) continue; + push( + 'warning', + REACT_CHART_AXIS_INERT, + ` prop "${inert}" is not read by this block — it renders nothing.`, + inert === 'xAxis' + ? 'Use xAxisKey="" (the aggregate\'s groupBy).' + : 'Use series={[{ dataKey: "" }]} (the aggregate\'s field).', + ); + } + + // Inline `data` wins over the aggregate query: the columns then come from + // the author's own rows, which this rule cannot see. Nothing further to say. + if (values.has('data')) return; + + const aggregate = values.get('aggregate'); + if (aggregate === undefined || aggregate === NOT_STATIC) return; + if (!isRec(aggregate)) return; + + const fn = strOf(aggregate.function); + const field = strOf(aggregate.field); + const groupBy = aggregate.groupBy; + const groupByField = strOf(groupBy) ?? (isRec(groupBy) ? strOf(groupBy.field) : undefined); + + // 1. The aggregate declaration itself. + if (fn && !(CHART_FUNCTIONS as readonly string[]).includes(fn)) { + push( + 'error', + REACT_CHART_AGGREGATE_INVALID, + `aggregate.function "${fn}" is not an aggregation this chart can run.`, + `Use one of: ${CHART_FUNCTIONS.join(', ')}.`, + ); + } else if (fn && fn !== 'count' && !field) { + push( + 'error', + REACT_CHART_AGGREGATE_INVALID, + `aggregate.function "${fn}" has no "field" to aggregate.`, + 'Add aggregate.field, or use function "count" (the only one that may omit it).', + ); + } + + // 2. `field` / `groupBy` are RAW field names on the bound object. + const objectName = strOf(values.get('objectName')); + const known = objectName ? objectFields.get(objectName) : undefined; + // No object name, or an object declared in another package: unknowable here + // — the same skip the widget/flow/page rules take. + if (objectName && known) { + const fieldRef = (name: string | undefined, prop: string) => { + if (!name) return; + // A relationship path (`account.name`) is resolved by the query engine. + if (name.includes('.')) return; + if (known.has(name) || SYSTEM_FIELDS.has(name)) return; + push( + 'error', + REACT_CHART_FIELD_UNKNOWN, + `aggregate.${prop} "${name}" is not a field on object "${objectName}" — ` + + `the aggregate query has nothing to ${prop === 'groupBy' ? 'group by' : 'aggregate'}, so the chart comes back empty.`, + `Fix the field name, or add "${name}" to ${objectName}.` + + (known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''), + ); + }; + fieldRef(field, 'field'); + fieldRef(groupByField, 'groupBy'); + } + + // 3. The axes name RESULT COLUMNS, not fields — the #3701 convention. + const keys = chartAggregateResultKeys({ field, function: fn, groupBy }); + const columns = [keys.category, keys.value].filter((k): k is string => !!k); + if (columns.length === 0) return; // an aggregate too incomplete to judge against + + const axisRef = (name: string | undefined, prop: string) => { + if (!name) return; + if (columns.includes(name)) return; + // The comparison overlay's column only exists when `compareTo` is on, but + // binding it is legitimate — never flag it as unknown. + if (keys.comparison && name === keys.comparison) return; + push( + 'error', + REACT_CHART_AXIS_UNKNOWN, + `"${name}" is not a column this aggregate returns, so the axis plots nothing. ` + + `Object-bound aggregate rows are keyed by the RAW FIELD NAMES ` + + `(unlike a dataset, whose rows are keyed by measure name).`, + `Result columns: ${columns.join(', ')}` + + (keys.comparison ? ` (plus "${keys.comparison}" with a comparison overlay)` : '') + + `. Bind ${prop} to one of them.`, + ); + }; + + axisRef(strOf(values.get('xAxisKey')), 'xAxisKey'); + const series = values.get('series'); + if (Array.isArray(series)) { + for (const s of series) { + if (isRec(s)) axisRef(strOf(s.dataKey), 'series[].dataKey'); + } + } + + // A category axis bound to anything but the groupBy is always wrong, and the + // check above lets it through when it happens to equal the VALUE column. + const xAxisKey = strOf(values.get('xAxisKey')); + if (xAxisKey && keys.category && xAxisKey !== keys.category && xAxisKey === keys.value) { + push( + 'error', + REACT_CHART_AXIS_UNKNOWN, + `xAxisKey "${xAxisKey}" is the aggregate's VALUE column, not its category column.`, + `The category axis is keyed by groupBy — use xAxisKey="${keys.category}".`, + ); + } +} + export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { const findings: ReactPropFinding[] = []; + const objectFields = indexObjectFields(stack); const pages = asArray(stack.pages); for (let p = 0; p < pages.length; p++) { const page = pages[p]; @@ -135,9 +403,14 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { if (block) { let hasSpread = false; const used = new Set(); + const values = new Map(); for (const a of node.attributes.properties) { if (tsc.isJsxSpreadAttribute(a)) { hasSpread = true; continue; } - if (tsc.isJsxAttribute(a)) used.add(a.name.getText(sf)); + if (tsc.isJsxAttribute(a)) { + const propName = a.name.getText(sf); + used.add(propName); + values.set(propName, attrValue(tsc, sf, a)); + } } const where = `page "${name}" › <${tag}>`; const path = `pages[${p}].source`; @@ -166,6 +439,11 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { }); } } + // A spread can supply any of the bindings below, so the values we + // can see are an incomplete picture — skip rather than guess. + if (tag === 'ObjectChart' && !hasSpread) { + checkObjectChart({ values, where, path }, objectFields, findings); + } } } tsc.forEachChild(node, visit); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 3a1a64ef71..8d9fa76ec2 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -3072,13 +3072,21 @@ "BreakpointColumnMapSchema (const)", "BreakpointName (type)", "BreakpointOrderMapSchema (const)", + "CHART_AGGREGATE_COMPARISON_SUFFIX (const)", "CalendarConfigSchema (const)", + "ChartAggregate (type)", + "ChartAggregateFunction (type)", + "ChartAggregateFunctionSchema (const)", + "ChartAggregateLike (interface)", + "ChartAggregateSchema (const)", "ChartAnnotation (type)", "ChartAnnotationSchema (const)", "ChartAxis (type)", "ChartAxisSchema (const)", "ChartConfig (type)", "ChartConfigSchema (const)", + "ChartGroupBy (type)", + "ChartGroupBySchema (const)", "ChartInteraction (type)", "ChartInteractionSchema (const)", "ChartSeries (type)", @@ -3392,6 +3400,9 @@ "ZIndexSchema (const)", "actionForm (const)", "appForm (const)", + "chartAggregateCategoryKey (function)", + "chartAggregateResultKeys (function)", + "chartAggregateValueKey (function)", "dashboardForm (const)", "datasetForm (const)", "defineAction (function)", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index d73db3c904..05d4b2d1ba 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -1587,9 +1587,12 @@ "ui/BreakpointName", "ui/BreakpointOrderMap", "ui/CalendarConfig", + "ui/ChartAggregate", + "ui/ChartAggregateFunction", "ui/ChartAnnotation", "ui/ChartAxis", "ui/ChartConfig", + "ui/ChartGroupBy", "ui/ChartInteraction", "ui/ChartSeries", "ui/ChartType", diff --git a/packages/spec/src/ui/chart-aggregate.test.ts b/packages/spec/src/ui/chart-aggregate.test.ts new file mode 100644 index 0000000000..47dbb8841e --- /dev/null +++ b/packages/spec/src/ui/chart-aggregate.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { describe, it, expect } from 'vitest'; +import { ChartAggregateSchema } from './chart.zod'; +import { + CHART_AGGREGATE_COMPARISON_SUFFIX, + chartAggregateCategoryKey, + chartAggregateValueKey, + chartAggregateResultKeys, +} from './chart-aggregate'; + +describe('ChartAggregateSchema', () => { + it('accepts a field aggregation grouped by a bare field name', () => { + expect(() => + ChartAggregateSchema.parse({ field: 'total', function: 'sum', groupBy: 'status' }), + ).not.toThrow(); + }); + + it('accepts a structured, date-bucketed groupBy', () => { + expect(() => + ChartAggregateSchema.parse({ + field: 'total', + function: 'sum', + groupBy: { field: 'closed_at', dateGranularity: 'quarter' }, + }), + ).not.toThrow(); + }); + + it('accepts a fieldless count (it counts rows, not a column)', () => { + expect(() => ChartAggregateSchema.parse({ function: 'count', groupBy: 'status' })).not.toThrow(); + }); + + it('rejects a non-count function with no field', () => { + // Left to the renderer this produced `sum(undefined)` — an empty chart with + // no diagnostic. The contract rejects it instead. + expect(() => ChartAggregateSchema.parse({ function: 'sum', groupBy: 'status' })).toThrow(); + }); + + it('rejects an aggregation function no chart renderer implements', () => { + expect(() => + ChartAggregateSchema.parse({ field: 'id', function: 'count_distinct', groupBy: 'status' }), + ).toThrow(); + }); +}); + +describe('result-column naming convention (#3701)', () => { + // The whole point of the convention: rows are keyed by the RAW FIELD NAMES, + // NOT by a derived measure name. A dataset keys its rows `sum_amount`; an + // object-bound aggregate keys the same numbers `amount`. + const agg = { field: 'total', function: 'sum', groupBy: 'status' }; + + it('keys the category column by groupBy', () => { + expect(chartAggregateCategoryKey(agg)).toBe('status'); + }); + + it('keys the value column by the raw field — never "sum_total"', () => { + expect(chartAggregateValueKey(agg)).toBe('total'); + }); + + it('honours a structured groupBy alias over its field', () => { + expect(chartAggregateCategoryKey({ ...agg, groupBy: { field: 'closed_at', alias: 'quarter' } })).toBe('quarter'); + expect(chartAggregateCategoryKey({ ...agg, groupBy: { field: 'closed_at' } })).toBe('closed_at'); + }); + + it('names a fieldless count column "count"', () => { + expect(chartAggregateValueKey({ function: 'count', groupBy: 'status' })).toBe('count'); + }); + + it('prefers an explicit field even for count', () => { + expect(chartAggregateValueKey({ field: 'total', function: 'count', groupBy: 'status' })).toBe('total'); + }); + + it('has no value column for a non-count aggregate with no field', () => { + expect(chartAggregateValueKey({ function: 'sum', groupBy: 'status' })).toBeUndefined(); + }); + + it('has no category column for an ungrouped aggregate', () => { + expect(chartAggregateCategoryKey({ field: 'total', function: 'sum' })).toBeUndefined(); + expect(chartAggregateCategoryKey(undefined)).toBeUndefined(); + }); + + it('derives the comparison-overlay column from the value column', () => { + expect(chartAggregateResultKeys(agg)).toEqual({ + category: 'status', + value: 'total', + comparison: `total${CHART_AGGREGATE_COMPARISON_SUFFIX}`, + }); + }); + + it('emits no comparison column when there is no value column', () => { + expect(chartAggregateResultKeys({ function: 'sum', groupBy: 'status' })).toEqual({ + category: 'status', + value: undefined, + comparison: undefined, + }); + }); +}); diff --git a/packages/spec/src/ui/chart-aggregate.ts b/packages/spec/src/ui/chart-aggregate.ts new file mode 100644 index 0000000000..e45a347583 --- /dev/null +++ b/packages/spec/src/ui/chart-aggregate.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Object-bound chart aggregation — the shape, and the **result-column naming + * convention** its axis bindings resolve against (issue #3701). + * + * ## Why this file exists + * + * A chart binds its data one of two ways: + * + * * **dataset-bound** (ADR-0021) — `dataset` + `dimensions` + `values`. + * Result rows are keyed by the DECLARED dimension/measure NAME + * (`sum_amount`), because a `DatasetMeasure` carries an explicit `name`. + * That explicit name is what lets `validate-chart-bindings` resolve an + * axis against the dataset. + * * **object-bound** — `objectName` + an inline `aggregate`. Rows come back + * from one ad-hoc `IDataEngine.aggregate()` call. Nothing here carries an + * author-chosen name, so until now nothing in the repo said what the + * result columns are called — and an axis binding that cannot be resolved + * cannot be validated (#3701, split out of #3583 Phase 2 / #3684). + * + * The runtime has in fact always agreed on an answer; it was simply never + * written down. All four paths that can serve an object-bound chart produce + * the SAME columns: + * + * | path | category column | value column | + * |---|---|---| + * | engine `aggregate()` w/ structured groupBy (`objectui` ObjectChart) | `groupBy.alias ?? groupBy.field` | the aggregation `alias`, which ObjectChart sets to `field \|\| function` | + * | legacy analytics/cube query | `groupBy` | remapped back to `field` | + * | client-side fallback (`aggregateRecords`) | `groupBy` | `field` | + * | the console's own object chart-view wiring (`ObjectView`) | `xAxisKey: groupBy` | `series[].dataKey: field` | + * + * So the convention below is a RECORD of the contract the renderers already + * implement, not a new invention — which is the only defensible basis for a + * lint rule (ADR-0072 D1: a false positive is what makes authors stop trusting + * the linter; ADR-0078 §5: verify, then enforce). + * + * ## The convention + * + * An object-bound aggregate query returns one row per group, keyed by the RAW + * FIELD NAMES it was given — there is no `sum_`/`_sum` decoration: + * + * ```ts + * { field: 'total', function: 'sum', groupBy: 'status' } + * // → [{ status: 'open', total: 1200 }, { status: 'paid', total: 800 }, …] + * ``` + * + * * **category column** = the `groupBy` field name (`groupBy.alias ?? + * groupBy.field` when `groupBy` is a structured node). + * * **value column** = `aggregate.field`. Only `count` may omit `field`; the + * column is then the literal `'count'` (the fieldless-count alias). + * * **comparison column** — when a chart also renders a period-over-period + * overlay, the previous window's value arrives under the value column plus + * {@link CHART_AGGREGATE_COMPARISON_SUFFIX} (`total__comparison`). + * + * Contrast this with the dataset path deliberately: there the value column is + * the measure's `name`, NOT the underlying field, which is exactly the trap + * `chart-measure-unknown` catches. The two paths key their rows differently + * because only one of them has an author-chosen name to key by. + * + * ## No business logic here (Prime Directive #2) + * + * The helpers below are pure contract derivations — they map a declaration to + * the column names the contract says it produces, the same way + * `data/display-name.ts` derives a title field from an object declaration. + * They exist so producers (renderers) and checkers (lint) cannot drift apart + * by each re-deriving the rule. The SHAPE they read — `ChartAggregateSchema` + * — lives with the other chart schemas in `chart.zod.ts`, per Zod-First. + */ + +/** + * Suffix the previous-window value carries when a chart renders a + * period-over-period comparison overlay: `total` → `total__comparison`. + */ +export const CHART_AGGREGATE_COMPARISON_SUFFIX = '__comparison'; + +/** The subset of {@link ChartAggregate} the naming rule reads. */ +export interface ChartAggregateLike { + field?: string; + function?: string; + groupBy?: unknown; +} + +/** + * The CATEGORY column an object-bound aggregate produces — what a chart's + * x-axis binding must name. + * + * `undefined` when `groupBy` is absent or malformed: an ungrouped aggregate + * returns a single row with no category column at all. + */ +export function chartAggregateCategoryKey(aggregate: ChartAggregateLike | undefined): string | undefined { + const groupBy = aggregate?.groupBy; + if (typeof groupBy === 'string') return groupBy || undefined; + if (groupBy && typeof groupBy === 'object' && !Array.isArray(groupBy)) { + const node = groupBy as { field?: unknown; alias?: unknown }; + const alias = typeof node.alias === 'string' && node.alias ? node.alias : undefined; + const field = typeof node.field === 'string' && node.field ? node.field : undefined; + return alias ?? field; + } + return undefined; +} + +/** + * The VALUE column an object-bound aggregate produces — what a chart's series + * / y-axis binding must name. + * + * `aggregate.field`, or the literal `'count'` for a fieldless count (the alias + * the engine projects `COUNT(*)` under). `undefined` when neither applies, + * i.e. a non-count aggregate with no `field` — a declaration + * {@link ChartAggregateSchema} already rejects. + */ +export function chartAggregateValueKey(aggregate: ChartAggregateLike | undefined): string | undefined { + const field = aggregate?.field; + if (typeof field === 'string' && field) return field; + return aggregate?.function === 'count' ? 'count' : undefined; +} + +/** + * Every column name an object-bound aggregate can put on a result row — the + * exact set an axis binding may resolve against. + * + * `comparison` is only ever present alongside `value`; a chart without a + * `compareTo` overlay simply never emits that column, so binding a series to + * it plots nothing. It is included here because a chart WITH the overlay binds + * it legitimately, and a checker must not flag that as unknown. + */ +export function chartAggregateResultKeys(aggregate: ChartAggregateLike | undefined): { + category?: string; + value?: string; + comparison?: string; +} { + const category = chartAggregateCategoryKey(aggregate); + const value = chartAggregateValueKey(aggregate); + return { + category, + value, + comparison: value ? `${value}${CHART_AGGREGATE_COMPARISON_SUFFIX}` : undefined, + }; +} diff --git a/packages/spec/src/ui/chart.zod.ts b/packages/spec/src/ui/chart.zod.ts index 0f2a6ccfe0..61949e31c1 100644 --- a/packages/spec/src/ui/chart.zod.ts +++ b/packages/spec/src/ui/chart.zod.ts @@ -210,7 +210,93 @@ export const ChartConfigSchema = lazySchema(() => z.object({ aria: AriaPropsSchema.optional().describe('ARIA accessibility attributes'), })); +/** + * Object-bound chart aggregation + * + * A chart binds its data one of two ways: DATASET-bound (ADR-0021 — `dataset` + * + `dimensions` + `values`), or OBJECT-bound (`objectName` + the inline + * `aggregate` below, run as one ad-hoc `IDataEngine.aggregate()` call). + * + * The two key their result rows DIFFERENTLY, and that difference is the usual + * cause of a chart that draws axes and no data. `./chart-aggregate.ts` records + * the object-bound rule and exports the helpers that derive the columns + * (`chartAggregateResultKeys`) — read it before binding an axis. + */ + +/** + * Aggregation functions an object-bound chart may ask for. + * + * A deliberate subset of the engine's `AggregationFunction`: these are the five + * the chart renderers implement in every path, including the client-side + * fallback. `count_distinct` / `array_agg` / `string_agg` are engine-level + * capabilities with no chart renderer behind them — advertising them here would + * be a declared-but-not-delivered claim (Prime Directive #10). + */ +export const ChartAggregateFunctionSchema = lazySchema(() => + z.enum(['count', 'sum', 'avg', 'min', 'max']), +); + +/** + * What the rows are grouped by — the chart's category axis. + * + * Either a bare field name, or the structured node the date-bucketing engine + * takes (`{ field, dateGranularity }`, see `data/query.zod.ts` + * `GroupByNodeSchema`). Mirrored rather than imported so the UI protocol does + * not depend on the query AST module; the two shapes are kept identical on + * purpose — the structured form is passed straight through to + * `IDataEngine.aggregate()`. + */ +export const ChartGroupBySchema = lazySchema(() => + z.union([ + z.string().describe('Field to group by'), + z.object({ + field: z.string().describe('Field to group by'), + dateGranularity: z + .enum(['day', 'week', 'month', 'quarter', 'year']) + .optional() + .describe('Bucket date values into uniform periods'), + alias: z + .string() + .optional() + .describe('Alias for the projected group value (defaults to `field`) — this becomes the category column'), + }), + ]), +); + +/** + * Inline aggregation for an OBJECT-bound chart (`objectName` + `aggregate`). + * + * `field` is optional only because `count` counts rows rather than a column; + * every other function needs something to aggregate, which the refinement + * enforces rather than leaving to a renderer to shrug off (it used to reach the + * renderer as `sum(undefined)` and render blank). + */ +export const ChartAggregateSchema = lazySchema(() => + z + .object({ + field: z + .string() + .optional() + .describe('Field to aggregate — required for sum/avg/min/max, optional for count'), + function: ChartAggregateFunctionSchema.describe('Aggregation function'), + groupBy: ChartGroupBySchema.describe('Field the rows are grouped by — the chart category axis'), + }) + .superRefine((agg, ctx) => { + if (agg.function !== 'count' && !agg.field) { + ctx.addIssue({ + code: 'custom', + path: ['field'], + message: `aggregate.function "${agg.function}" needs a "field" to aggregate (only "count" may omit it).`, + }); + } + }) + .describe('Inline aggregation for an object-bound chart'), +); + export type ChartConfig = z.infer; +export type ChartAggregate = z.infer; +export type ChartAggregateFunction = z.infer; +export type ChartGroupBy = z.infer; export type ChartAxis = z.infer; export type ChartSeries = z.infer; export type ChartAnnotation = z.infer; diff --git a/packages/spec/src/ui/index.ts b/packages/spec/src/ui/index.ts index a60dabc391..cb3b3052c5 100644 --- a/packages/spec/src/ui/index.ts +++ b/packages/spec/src/ui/index.ts @@ -11,6 +11,7 @@ */ export * from './chart.zod'; +export * from './chart-aggregate'; export * from './i18n.zod'; export * from './responsive.zod'; export * from './app.zod'; diff --git a/packages/spec/src/ui/react-blocks.ts b/packages/spec/src/ui/react-blocks.ts index 3182dee82c..9617775ffd 100644 --- a/packages/spec/src/ui/react-blocks.ts +++ b/packages/spec/src/ui/react-blocks.ts @@ -103,13 +103,22 @@ export const REACT_BLOCKS: ReactBlockDef[] = [ { tag: 'ObjectChart', schemaType: 'object-chart', - summary: 'Chart over an object’s aggregated data. Config props come from the spec Chart config schema.', + summary: 'Chart over an object’s aggregated data. Bind objectName + aggregate; the axes name the aggregate’s RESULT COLUMNS (see chartAggregateResultKeys).', schema: ChartConfigSchema, - dataProps: ['title', 'series', 'xAxis', 'yAxis', 'colors', 'showLegend'], + // `xAxis`/`yAxis`/`series` from ChartConfigSchema are deliberately NOT + // surfaced here (#3701): this block reads `xAxisKey` + `series[].dataKey`, + // so the ChartConfig axis shapes were silently inert — advertising them + // violated ADR-0078 (a prop the author writes is honored or rejected, + // never silently dropped). The real bindings are declared in the overlay + // below; `validate-react-page-props` rejects the ChartConfig spellings. + dataProps: ['title', 'colors', 'showLegend'], interactions: [ OBJECT_NAME, { name: 'filter', type: 'FilterArray', kind: 'controlled', description: 'ObjectQL filter scoping the data; drive from React state.' }, - { name: 'aggregate', type: '{ field, function, groupBy }', kind: 'binding', description: 'Aggregation: function (sum/avg/count) over field, grouped by groupBy.' }, + { name: 'aggregate', type: "{ field?: string; function: 'count' | 'sum' | 'avg' | 'min' | 'max'; groupBy: string | { field: string; dateGranularity?: 'day' | 'week' | 'month' | 'quarter' | 'year' } }", kind: 'binding', description: 'Aggregation run against objectName. Result rows are keyed by the RAW FIELD NAMES: one column named after groupBy (the category) and one named after field (the value; the literal "count" for a fieldless count). Bind the axes to those names.' }, + { name: 'chartType', type: "'bar' | 'column' | 'horizontal-bar' | 'line' | 'area' | 'pie' | 'donut' | 'radar' | 'scatter' | 'funnel' | 'combo' | 'treemap' | 'sankey'", kind: 'binding', description: 'Which chart to draw (default bar).' }, + { name: 'xAxisKey', type: 'string', kind: 'binding', description: 'Result column plotted on the category axis. Defaults to the aggregate’s groupBy — set it only to be explicit, and only to that name.' }, + { name: 'series', type: 'Array<{ dataKey: string; label?: string }>', kind: 'binding', description: 'Plotted series. Each dataKey names a result column — the aggregate’s field (or "count"), plus "__comparison" when a comparison overlay is on. NOT the ChartConfig series shape.' }, { name: 'data', type: 'any[]', kind: 'binding', description: 'Static/precomputed data to chart directly instead of binding via objectName + aggregate.' }, ], }, diff --git a/skills/objectstack-ui/SKILL.md b/skills/objectstack-ui/SKILL.md index 7c459f7f5b..0ad75e28eb 100644 --- a/skills/objectstack-ui/SKILL.md +++ b/skills/objectstack-ui/SKILL.md @@ -640,6 +640,23 @@ dataset, so Level B only surfaces in Studio previews and hand-coded react-page - Dataset-bound widgets need at least one `values` entry, and every `dataset`/`dimensions`/`values` name must resolve to its `defineDataset` — `os validate` fails on an unresolved name (an empty chart otherwise). +- **The two paths key their result rows differently — this is the #1 way a + chart renders blank.** A DATASET returns rows keyed by the declared measure + NAME (`sum_amount`), because a measure has an author-chosen `name`. An + OBJECT-bound inline `aggregate` has no such name, so its rows are keyed by + the RAW FIELD NAMES it was given: `groupBy` for the category column, `field` + for the value column (the literal `count` for a fieldless `count`). Bind + ``'s `xAxisKey` / `series[].dataKey` to *those* names — never to + a `sum_`-style measure name, and never to a field the aggregate did not + select. `os validate` checks both halves (`react-chart-field-unknown`, + `react-chart-axis-unknown`). + + ```jsx + + // rows: [{ status: 'open', total: 1200 }, …] ← keyed by the raw field names + ``` - Studio's Dashboard Widget Inspector can author per-widget `dataset`, `dimensions`, and `values`; curated metadata-admin forms merge server-only fields back into the payload, so saving through Studio should diff --git a/skills/objectstack-ui/contracts/react-blocks.contract.json b/skills/objectstack-ui/contracts/react-blocks.contract.json index b214b6a961..1485ef6fe3 100644 --- a/skills/objectstack-ui/contracts/react-blocks.contract.json +++ b/skills/objectstack-ui/contracts/react-blocks.contract.json @@ -289,7 +289,7 @@ { "tag": "ObjectChart", "schemaType": "object-chart", - "summary": "Chart over an object’s aggregated data. Config props come from the spec Chart config schema.", + "summary": "Chart over an object’s aggregated data. Bind objectName + aggregate; the axes name the aggregate’s RESULT COLUMNS (see chartAggregateResultKeys).", "specSchema": true, "props": [ { @@ -301,10 +301,31 @@ }, { "name": "aggregate", - "type": "{ field, function, groupBy }", + "type": "{ field?: string; function: 'count' | 'sum' | 'avg' | 'min' | 'max'; groupBy: string | { field: string; dateGranularity?: 'day' | 'week' | 'month' | 'quarter' | 'year' } }", "kind": "binding", "required": false, - "description": "Aggregation: function (sum/avg/count) over field, grouped by groupBy." + "description": "Aggregation run against objectName. Result rows are keyed by the RAW FIELD NAMES: one column named after groupBy (the category) and one named after field (the value; the literal \"count\" for a fieldless count). Bind the axes to those names." + }, + { + "name": "chartType", + "type": "'bar' | 'column' | 'horizontal-bar' | 'line' | 'area' | 'pie' | 'donut' | 'radar' | 'scatter' | 'funnel' | 'combo' | 'treemap' | 'sankey'", + "kind": "binding", + "required": false, + "description": "Which chart to draw (default bar)." + }, + { + "name": "xAxisKey", + "type": "string", + "kind": "binding", + "required": false, + "description": "Result column plotted on the category axis. Defaults to the aggregate’s groupBy — set it only to be explicit, and only to that name." + }, + { + "name": "series", + "type": "Array<{ dataKey: string; label?: string }>", + "kind": "binding", + "required": false, + "description": "Plotted series. Each dataKey names a result column — the aggregate’s field (or \"count\"), plus \"__comparison\" when a comparison overlay is on. NOT the ChartConfig series shape." }, { "name": "data", @@ -327,27 +348,6 @@ "required": false, "description": "Chart title" }, - { - "name": "series", - "type": "object[]", - "kind": "data", - "required": false, - "description": "Defined series configuration" - }, - { - "name": "xAxis", - "type": "object", - "kind": "data", - "required": false, - "description": "X-Axis configuration" - }, - { - "name": "yAxis", - "type": "object[]", - "kind": "data", - "required": false, - "description": "Y-Axis configuration (support dual axis)" - }, { "name": "colors", "type": "string[] | object", diff --git a/skills/objectstack-ui/evals/analytics-inline-vs-dataset.json b/skills/objectstack-ui/evals/analytics-inline-vs-dataset.json index 615c28ea7e..601f9e1989 100644 --- a/skills/objectstack-ui/evals/analytics-inline-vs-dataset.json +++ b/skills/objectstack-ui/evals/analytics-inline-vs-dataset.json @@ -46,7 +46,7 @@ { "id": 5, "prompt": "In a kind:'react' page, drop a quick one-off bar chart of tasks grouped by status. Write the block.", - "expected_output": "Uses the inline block directly — an ad-hoc in-page chart does not require a dataset, and the react ObjectChart block has no dataset prop. Naming a dataset would only be for reuse/governance (Level B), not needed here.", + "expected_output": "Uses the inline block directly — an ad-hoc in-page chart does not require a dataset, and the react ObjectChart block has no dataset prop. Naming a dataset would only be for reuse/governance (Level B), not needed here. Note the axes name the aggregate's RESULT COLUMNS: `status` (its groupBy) and `count` (the fieldless-count column) — a fieldless count needs no field.", "files": [], "assertions": { "must_contain": ["ObjectChart", "aggregate", "groupBy"], diff --git a/skills/objectstack-ui/references/react-blocks.md b/skills/objectstack-ui/references/react-blocks.md index 794def75c4..35de808379 100644 --- a/skills/objectstack-ui/references/react-blocks.md +++ b/skills/objectstack-ui/references/react-blocks.md @@ -65,18 +65,18 @@ Server-connected object table with toolbar and switchable visualizations (grid/k ## `` — `object-chart` -Chart over an object’s aggregated data. Config props come from the spec Chart config schema. +Chart over an object’s aggregated data. Bind objectName + aggregate; the axes name the aggregate’s RESULT COLUMNS (see chartAggregateResultKeys). | prop | type | kind | required | description | |------|------|------|:--------:|-------------| | `objectName` | `string` | binding | ✓ | The object this block binds to (server-connected). | -| `aggregate` | `{ field, function, groupBy }` | binding | | Aggregation: function (sum/avg/count) over field, grouped by groupBy. | +| `aggregate` | `{ field?: string; function: 'count' \| 'sum' \| 'avg' \| 'min' \| 'max'; groupBy: string \| { field: string; dateGranularity?: 'day' \| 'week' \| 'month' \| 'quarter' \| 'year' } }` | binding | | Aggregation run against objectName. Result rows are keyed by the RAW FIELD NAMES: one column named after groupBy (the category) and one named after field (the value; the literal "count" for a fieldless count). Bind the axes to those names. | +| `chartType` | `'bar' \| 'column' \| 'horizontal-bar' \| 'line' \| 'area' \| 'pie' \| 'donut' \| 'radar' \| 'scatter' \| 'funnel' \| 'combo' \| 'treemap' \| 'sankey'` | binding | | Which chart to draw (default bar). | +| `xAxisKey` | `string` | binding | | Result column plotted on the category axis. Defaults to the aggregate’s groupBy — set it only to be explicit, and only to that name. | +| `series` | `Array<{ dataKey: string; label?: string }>` | binding | | Plotted series. Each dataKey names a result column — the aggregate’s field (or "count"), plus "__comparison" when a comparison overlay is on. NOT the ChartConfig series shape. | | `data` | `any[]` | binding | | Static/precomputed data to chart directly instead of binding via objectName + aggregate. | | `filter` | `FilterArray` | controlled | | ObjectQL filter scoping the data; drive from React state. | | `title` | `string` | data | | Chart title | -| `series` | `object[]` | data | | Defined series configuration | -| `xAxis` | `object` | data | | X-Axis configuration | -| `yAxis` | `object[]` | data | | Y-Axis configuration (support dual axis) | | `colors` | `string[] \| object` | data | | Color palette (string[]) or value→color map ({ value: color }) | | `showLegend` | `boolean` | data | | Display legend |