diff --git a/.changeset/lint-dataset-measure-aggregate-field-type.md b/.changeset/lint-dataset-measure-aggregate-field-type.md new file mode 100644 index 00000000000..c727e6d1e3a --- /dev/null +++ b/.changeset/lint-dataset-measure-aggregate-field-type.md @@ -0,0 +1,48 @@ +--- +'@objectstack/lint': minor +--- + +Refuse a dataset measure whose `aggregate` the field's declared type cannot carry, at authoring time + +A dataset measure pairs an `aggregate` with a `field`, and +`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec`) declares which of those pairs every +backend answers the same way. Nothing in the authoring path read that table, so `avg` over a +`datetime` field validated clean and shipped: one SQL family coerces the column's canonical UTC +text and returns a plausible number (the average *year*), another has no such function and fails at +query time — the answer decided by the deployment rather than by the document. The analytics service +refuses the pair when a query is built (`400 DATASET_INVALID`); this is the same verdict, from the +same table, at the door the author is standing in front of. + +New rule `measure-aggregate-field-type-refused`, gating (`error`), on `os validate` / `os build` / +`os lint`. It resolves the field's declared type on the object graph lint already indexes — including +a dotted `relationship.field` path, whose leaf type the compile leg cannot see — and refuses the +pair when `isAggregateCompatibleWithFieldType` says no. The message names the aggregate, the field, +its declared type and the accepted set, and the hint names the aggregates that type *does* accept, +both computed from the table rather than restated. It stays silent wherever the type cannot be +resolved (an object this stack does not define, a field path that resolves to nothing, an untyped +field, an aggregate outside the closed `AggregationFunction` vocabulary) rather than guessing. + +**BREAKING**: metadata that passed `os validate` / `os build` / `os lint` before can now fail. Every +pair this refuses is one the analytics service already refuses at query time, so nothing that +*worked* stops working — but a build that did not fail now does. + +Migration, per refused pair — FROM the aggregate the field's type cannot carry, TO one it accepts: + +- `avg` / `sum` over a `date` / `datetime` / `time` field → `min` / `max`, which return a real + instant of the field's own type, or `count` / `count_distinct`. A DURATION is not recoverable from + an aggregate over instants: store it as a number (a computed "days open" field) and aggregate that. +- `sum` over a `percent` field → `avg`. A rate does not add; the total routinely exceeds 100%. +- `min` / `max` over the string, option, reference, file, structured-JSON or `formula` classes → + `count` / `count_distinct` for "how many distinct values", or a SORT on the record list for "the + first / last record". String order is collation-dependent, so two backends answer two different + "smallest" values for one document. +- Any other refused pair → read the row for your aggregate in + `AGGREGATE_FIELD_TYPE_COMPATIBILITY`; the refusal message prints it. + +A `derived` measure whose `of` names a refused measure is fixed by fixing that measure, not the +`derived` one. A `date` / `datetime` / `text` field used as a DIMENSION — grouping, bucketing, +filtering — is untouched: this is about aggregation only. + +Clause-②: yes (narrowing) + + diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index b8497cd3208..beefa43ea99 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -589,7 +589,7 @@ os compile --json # JSON output for CI pipelines → Normalizing stack definition... → Lowering inline handlers... → Validating protocol compliance... - → Running author-time rules (45)... + → Running author-time rules (46)... → Checking capability providers (#3366)... → Collecting package docs (ADR-0046)... 0 collected → Writing artifact... diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 0fe9afaab82..4c02eeb2f05 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -189,6 +189,30 @@ The react `` block is **object-bound** (`objectName` + an inline gate below against a different rule — its result rows are keyed by the raw field names, exactly the opposite of the dataset case. +One level down from the axis, the measure itself is checked against the field it +aggregates. A measure pairs an `aggregate` with a `field`, and +`AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec`) declares which pairs +every backend answers the same way. A pair outside it is refused +(`measure-aggregate-field-type-refused`) naming the aggregate, the field, its +declared type and the accepted set: + +```ts +// object declares `closed_at` as `datetime` +measures: [{ name: 'avg_closed', aggregate: 'avg', field: 'closed_at' }] +// ↑ refused: the answer would be the dialect's, not the data's +``` + +`avg` over a temporal column is where this bites hardest: one SQL family coerces +the stored text and returns a plausible number (the average *year*), another has +no such function and fails at query time. `min`/`max` over the same field are +**accepted** — they return a real instant of the field's own type — and +`count`/`count_distinct` are accepted over every type, because they read no +arithmetic off the value. The analytics service refuses the same pair with +`400 DATASET_INVALID` when a query is built; this is the identical verdict, +from the identical table, one door earlier. The rule stays silent wherever the +field's type cannot be resolved (an object this stack does not define, a +dangling field path, an untyped field) rather than guessing. + ### 7. Navigation exposing objects nobody can read Navigation and permissions are separate metadata, each valid on its own — so an @@ -396,6 +420,7 @@ orthogonal to both, and no cell here can carry it; it is written out in | Zod-valid but functionally inert declarations — a `summary` with no operations (ADR-0078), a managed object advertising an API method its affordances refuse (#7521) | ✓ | ✓ | ✓ | ✓ᵒ | | View container shape | ✓ | ✓ | ✓ | — | | Widget-binding integrity (ADR-0021) | ✓ | ✓ | ✓ | ✓ᵈ | +| Dataset measure `aggregate` × the field's declared type — a pair the spec's compatibility table refuses, e.g. `avg` over a `datetime` field (#16354) | ✓ | ✓ | ✓ | — | | Dashboard action/route references (ADR-0049) | ✓ | ✓ | ✓ | — | | Filter placeholder resolvability (#3574) | ✓ | ✓ | ✓ | — | | Ordering comparands naming a date-range preset — `last_30_days` in a `>=` position (#8793) | ✓ | ✓ | ✓ | ✓ᵈᵛᵒᵖᶠ | @@ -624,7 +649,7 @@ A clean run walks the registry and reports timing: Config: /path/to/support-desk/objectstack.config.ts Load time: 21ms → Validating against ObjectStack Protocol... - → Running author-time rules (45)... + → Running author-time rules (46)... → Checking capability providers (#3366)... → Checking package docs (ADR-0046)... diff --git a/content/docs/getting-started/build-with-claude-code.mdx b/content/docs/getting-started/build-with-claude-code.mdx index d69897d52cd..c6e8fa06fde 100644 --- a/content/docs/getting-started/build-with-claude-code.mdx +++ b/content/docs/getting-started/build-with-claude-code.mdx @@ -265,7 +265,7 @@ visible: 'status != "resolved"' ◆ Validate ──────────────────────────────────────── → Validating against ObjectStack Protocol... - → Running author-time rules (45)... + → Running author-time rules (46)... ✗ Author-time rules failed (1 issue) • stack · action 'resolve_ticket' visible: bare reference `status` — a diff --git a/content/docs/ui/react-pages.mdx b/content/docs/ui/react-pages.mdx index 392d5c5d11e..5b0ccafbe86 100644 --- a/content/docs/ui/react-pages.mdx +++ b/content/docs/ui/react-pages.mdx @@ -381,7 +381,7 @@ objectstack validate ──────────────────────────────────────── → Loading configuration... → Validating against ObjectStack Protocol... - → Running author-time rules (45)... + → Running author-time rules (46)... → Checking capability providers (#3366)... → Checking package docs (ADR-0046)... diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index dc8588c9138..6491d2e2185 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -103,6 +103,7 @@ import { validateFunctionalCompleteness } from './validate-functional-completene import { validateManagedApiMethods } from './validate-managed-api-methods.js'; import { validateViewContainers } from './validate-view-containers.js'; import { validateWidgetBindings } from './validate-widget-bindings.js'; +import { validateDatasetMeasureAggregates } from './validate-dataset-measure-aggregates.js'; import { validateDashboardActionRefs } from './validate-dashboard-action-refs.js'; import { validateFilterTokens } from './validate-filter-tokens.js'; import { validateFlowFilterTokens } from './validate-flow-filter-tokens.js'; @@ -601,6 +602,40 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ runtimeTypes: ['dashboard'], run: (stack) => validateWidgetBindings(stack), }, + // #16354 — the AUTHORING-TIME leg of the aggregate × field-type contract + // (director ruling, decision batch #59, 2026-09-06: "both legs, table in + // spec"; the table is `AGGREGATE_FIELD_TYPE_COMPATIBILITY` in + // `@objectstack/spec`, #16353). The compile leg (`dataset-compiler`, + // `service-analytics`) refuses a refused pair with `400 DATASET_INVALID` + // when a query is built; this one refuses it while the author still has the + // document open. `parsed`, the same tier as `validateWidgetBindings` above, + // because the two read the SAME positions (`datasets[].measures[]`) and must + // not be handed two different documents to judge. + { + name: 'validateDatasetMeasureAggregates', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-dataset-measure-aggregates.ts', + // NOT RUNTIME_NEEDS_FULL_SNAPSHOT: the two collections this rule reads — + // `objects` and `datasets` — are both carried (#7529). What holds it off + // the door is the type axis: the metadata type that CARRIES the + // declaration is `dataset` (`allowRuntimeCreate: true`), and + // `TYPE_TO_STACK_KEY` in `runtime-gate.ts` has no `dataset` row, so a + // dataset write builds no per-write snapshot and no rule can be dispatched + // for it. Declaring another type here would only re-judge a STORED + // dataset, which the #4463 D4 differential cancels as someone else's + // pre-existing condition — wired, and enforcing nothing. Mapping the + // `dataset` type at the gate is its own card (every rule reading + // `stack.datasets` gains the door at once, including the existence rules). + surfaces: CLI_ONLY, + surfaceReason: + 'The declaring metadata type is `dataset`, which `runtime-gate.ts`\'s TYPE_TO_STACK_KEY does ' + + 'not map — a dataset write builds no per-write snapshot, so nothing can dispatch this rule ' + + 'there; declaring any other type would only re-judge a stored dataset, which the publish ' + + 'gate\'s differential cancels as somebody else\'s pre-existing condition.', + run: (stack) => validateDatasetMeasureAggregates(stack), + }, // ADR-0049 / #3367 — a dashboard header action naming a dead target ships a // button that renders and refuses (or does nothing) on click: a `script` // target must name a defined action, a `modal` target must name a declared diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 4ed37c7fbdc..7827e7efaf7 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -50,6 +50,18 @@ export { } from './validate-widget-bindings.js'; export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js'; +// [#16354] The authoring-time leg of the aggregate × field-type contract +// (director ruling, decision batch #59: "both legs, table in spec"). Exported +// as its own rule because the verdict is the SPEC TABLE's — the same +// `isAggregateCompatibleWithFieldType` the compile leg calls — so a consumer +// that authors datasets outside a config file (Studio, an MCP/AI author, a +// generator) runs one rule rather than re-deriving the table. +export { + validateDatasetMeasureAggregates, + MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, +} from './validate-dataset-measure-aggregates.js'; +export type { DatasetMeasureAggregateFinding } from './validate-dataset-measure-aggregates.js'; + export { validateStackExpressions, fieldRuleRootIssue, FIELD_RULE_BOUND_ROOTS } from './validate-expressions.js'; export type { ExprIssue } from './validate-expressions.js'; diff --git a/packages/lint/src/validate-dataset-measure-aggregates.test.ts b/packages/lint/src/validate-dataset-measure-aggregates.test.ts new file mode 100644 index 00000000000..cc2a1fce978 --- /dev/null +++ b/packages/lint/src/validate-dataset-measure-aggregates.test.ts @@ -0,0 +1,375 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `measure-aggregate-field-type-refused` — the authoring-time door for the +// aggregate × field-type contract (#16354, the lint leg of #16099). +// +// The load-bearing test in this file is `accepts avg over a number field`, and +// the sweep that generalises it (`agrees with the table on every aggregate × +// every declared FieldType`). A rule that refuses an INCOHERENT pair is easy; +// a rule that refuses only those is the whole product. A false positive here +// fails a build over metadata every backend answers identically, which is +// strictly worse than the gap this rule closes — so the negative controls are +// pinned per class and then swept exhaustively, with a floor on both sides of +// the sweep so a rule that stopped firing (or started firing on everything) +// cannot make the agreement vacuously true. +import { describe, expect, it } from 'vitest'; +import { + AGGREGATE_FIELD_TYPE_COMPATIBILITY, + BOOLEAN_VALUE_TYPES, + FieldType, + NUMERIC_VALUE_TYPES, + isAggregateCompatibleWithFieldType, +} from '@objectstack/spec/data'; + +import { runAuthoringRules } from './authoring-rules.js'; +import { + MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, + validateDatasetMeasureAggregates, +} from './validate-dataset-measure-aggregates.js'; + +const RULE = MEASURE_AGGREGATE_FIELD_TYPE_REFUSED; + +/** Every aggregate the spec table declares a row for. */ +const AGGREGATES = Object.keys(AGGREGATE_FIELD_TYPE_COMPATIBILITY); + +/** + * One dataset over one object: a measure applying `aggregate` to a field the + * object declares as `fieldType`. `fieldType: undefined` declares the field + * with no `type` at all; `field` overrides which name the measure binds. + */ +const stackWith = ( + aggregate: unknown, + fieldType: string | undefined, + overrides: { + field?: unknown; + objectName?: string; + datasetObject?: string; + fields?: Record; + } = {}, +): Record => ({ + name: 'analytics_probe', + objects: [ + { + name: overrides.objectName ?? 'crm_opportunity', + label: 'Opportunity', + sharingModel: 'private', + fields: overrides.fields ?? { + name: { type: 'text' }, + measured: fieldType ? { type: fieldType } : { label: 'Untyped' }, + }, + }, + ], + datasets: [ + { + name: 'opportunity_metrics', + object: overrides.datasetObject ?? overrides.objectName ?? 'crm_opportunity', + dimensions: [], + measures: [ + { + name: 'the_measure', + aggregate, + field: 'field' in overrides ? overrides.field : 'measured', + }, + ], + }, + ], +}); + +const findings = (stack: unknown) => + validateDatasetMeasureAggregates(stack).filter((f) => f.rule === RULE); + +describe('measure-aggregate-field-type-refused — refuses a pair the spec table refuses', () => { + // The card's own positive control. + it('refuses the card\'s example: avg over a datetime field', () => { + const found = findings(stackWith('avg', 'datetime')); + expect(found).toHaveLength(1); + const issue = found[0]; + expect(issue.severity).toBe('error'); + expect(issue.rule).toBe(RULE); + expect(issue.path).toBe('datasets[0].measures[0].aggregate'); + // The author must be able to act without opening the spec: the AGGREGATE, + // the FIELD, its declared TYPE, and the set that aggregate accepts. + expect(issue.message).toContain('avg'); + expect(issue.message).toContain('measured'); + expect(issue.message).toContain('datetime'); + for (const accepted of AGGREGATE_FIELD_TYPE_COMPATIBILITY.avg) { + expect(issue.message, `accepted type ${accepted} named`).toContain(accepted); + } + // And the way out, computed from the same table rather than prose: which + // aggregates WOULD accept a `datetime`. + expect(issue.hint).toContain('min'); + expect(issue.hint).toContain('max'); + expect(issue.hint).toContain('count_distinct'); + expect(issue.where).toBe('dataset "opportunity_metrics" › measure "the_measure"'); + }); + + // The card's third control. `analytics-service.ts` already calls this pair + // incoherent (`isIncoherentAggregate`), and the spec table refuses it on + // that predicate's own authority — so it is refused HERE as a contract + // verdict, beside the advisory one its neighbour raises. + it('refuses sum over a percent field', () => { + const found = findings(stackWith('sum', 'percent')); + expect(found).toHaveLength(1); + expect(found[0].message).toContain('percent'); + expect(found[0].message).toContain('sum'); + }); + + it('refuses avg and sum over every member of the temporal class', () => { + for (const fieldType of ['date', 'datetime', 'time']) { + for (const aggregate of ['avg', 'sum']) { + expect(findings(stackWith(aggregate, fieldType)), `${aggregate}(${fieldType})`).toHaveLength(1); + } + } + }); + + it('refuses min and max over the classes that have no backend-independent order', () => { + for (const fieldType of ['text', 'select', 'lookup', 'json', 'autonumber', 'formula']) { + for (const aggregate of ['min', 'max']) { + expect(findings(stackWith(aggregate, fieldType)), `${aggregate}(${fieldType})`).toHaveLength(1); + } + } + }); +}); + +describe('measure-aggregate-field-type-refused — stays silent on every pair the table accepts', () => { + // ⭐ The negative control the card names, and the leg that separates "the + // rule works" from "the rule fires on everything". + it('accepts avg over a number field', () => { + expect(findings(stackWith('avg', 'number'))).toEqual([]); + }); + + it('accepts avg and sum over the numeric class (sum minus the rate)', () => { + for (const fieldType of NUMERIC_VALUE_TYPES) { + expect(findings(stackWith('avg', fieldType)), `avg(${fieldType})`).toEqual([]); + if (fieldType === 'percent') continue; // a rate does not add — refused above + expect(findings(stackWith('sum', fieldType)), `sum(${fieldType})`).toEqual([]); + } + }); + + // #11152 (maintainer ruling, 2026-08-28), upheld by decision batch #80: + // booleans aggregate as NUMBERS on every backend, with no per-aggregate + // exception. A careless "numeric only" predicate breaks exactly this leg. + it('accepts every arithmetic and order aggregate over the boolean class', () => { + for (const fieldType of BOOLEAN_VALUE_TYPES) { + for (const aggregate of ['sum', 'avg', 'min', 'max']) { + expect(findings(stackWith(aggregate, fieldType)), `${aggregate}(${fieldType})`).toEqual([]); + } + } + }); + + it('accepts min and max over the temporal class — they return the field\'s own type (#15768)', () => { + for (const fieldType of ['date', 'datetime', 'time']) { + for (const aggregate of ['min', 'max']) { + expect(findings(stackWith(aggregate, fieldType)), `${aggregate}(${fieldType})`).toEqual([]); + } + } + }); + + it('accepts count and count_distinct over every declared FieldType', () => { + for (const fieldType of FieldType.options) { + for (const aggregate of ['count', 'count_distinct']) { + expect(findings(stackWith(aggregate, fieldType)), `${aggregate}(${fieldType})`).toEqual([]); + } + } + }); +}); + +describe('measure-aggregate-field-type-refused — the rule IS the table, on every pair', () => { + // The whole accept/refuse surface, asserted against the one authority rather + // than against a list retyped here: if the table moves, this test moves with + // it, and if the rule ever disagrees with the table it fails on the exact + // pair that diverged. + it('agrees with isAggregateCompatibleWithFieldType on every aggregate × every declared FieldType', () => { + let refused = 0; + let accepted = 0; + for (const aggregate of AGGREGATES) { + for (const fieldType of FieldType.options) { + const fires = findings(stackWith(aggregate, fieldType)).length > 0; + const tableAccepts = isAggregateCompatibleWithFieldType(aggregate, fieldType); + expect(fires, `${aggregate}(${fieldType})`).toBe(!tableAccepts); + if (fires) refused++; + else accepted++; + } + } + // Floors on BOTH sides, so neither "the rule never fires" nor "the rule + // fires on everything" can satisfy the equality above vacuously. + expect(refused).toBeGreaterThan(50); + expect(accepted).toBeGreaterThan(50); + expect(refused + accepted).toBe(AGGREGATES.length * FieldType.options.length); + }); +}); + +describe('measure-aggregate-field-type-refused — never hands the predicate a guess', () => { + it('is silent when the dataset\'s base object is not in this stack', () => { + // `validate-object-references.ts` owns an unresolvable base object; one + // typo must not also yield a type verdict. + expect(findings(stackWith('avg', 'datetime', { datasetObject: 'not_here' }))).toEqual([]); + }); + + it('is silent when the object declares no readable field map', () => { + const stack = stackWith('avg', 'datetime') as Record; + (stack.objects as Record[])[0].fields = {}; + expect(findings(stack)).toEqual([]); + }); + + it('is silent when the measure\'s field resolves to nothing — that is dataset-field-unknown\'s finding', () => { + expect(findings(stackWith('avg', 'datetime', { field: 'nope' }))).toEqual([]); + }); + + it('is silent when the resolved field declares no type', () => { + expect(findings(stackWith('avg', undefined))).toEqual([]); + }); + + it('is silent when the measure writes no field — a plain count, or a derived measure', () => { + expect(findings(stackWith('avg', 'datetime', { field: undefined }))).toEqual([]); + expect(findings(stackWith('count', 'datetime', { field: undefined }))).toEqual([]); + }); + + it('is silent when either position is not a string — the schema owns those', () => { + expect(findings(stackWith(['avg'], 'datetime'))).toEqual([]); + expect(findings(stackWith({ fn: 'avg' }, 'datetime'))).toEqual([]); + expect(findings(stackWith('avg', 'datetime', { field: ['measured'] }))).toEqual([]); + expect(findings(stackWith('', 'datetime'))).toEqual([]); + }); + + it('is silent for an aggregate outside the table\'s vocabulary — including a prototype key', () => { + // `AggregationFunction` is a closed enum, so `median` is a schema error and + // reporting it here would call a vocabulary problem a compatibility one. + expect(findings(stackWith('median', 'datetime'))).toEqual([]); + // And the shape that a bare property lookup on the frozen table would have + // resolved to a function rather than to `undefined`. + for (const key of ['toString', 'constructor', 'hasOwnProperty', '__proto__']) { + expect(findings(stackWith(key, 'datetime')), key).toEqual([]); + } + }); + + it('is silent on a stack with no datasets at all, and on a non-record input', () => { + expect(validateDatasetMeasureAggregates({ objects: [] })).toEqual([]); + expect(validateDatasetMeasureAggregates(undefined)).toEqual([]); + expect(validateDatasetMeasureAggregates('not a stack')).toEqual([]); + expect(validateDatasetMeasureAggregates([])).toEqual([]); + }); +}); + +describe('measure-aggregate-field-type-refused — the positions only authoring-time resolution can reach', () => { + /** A dataset whose measure binds a field across a declared relationship hop. */ + const joined = (aggregate: string) => ({ + name: 'analytics_probe', + objects: [ + { + name: 'crm_opportunity', + sharingModel: 'private', + fields: { + name: { type: 'text' }, + account: { type: 'lookup', reference: 'crm_account' }, + }, + }, + { + name: 'crm_account', + sharingModel: 'private', + fields: { + name: { type: 'text' }, + signed_at: { type: 'datetime' }, + arr: { type: 'currency' }, + }, + }, + ], + datasets: [ + { + name: 'opportunity_metrics', + object: 'crm_opportunity', + include: ['account'], + dimensions: [], + measures: [{ name: 'the_measure', aggregate, field: 'account.signed_at' }], + }, + ], + }); + + // The compile leg returns early on a dotted reference (its declared-type + // source answers for the base object only). Authoring time has the whole + // graph, so the LEAF's declared type is a read rather than a guess. + it('refuses a refused pair on a joined field, which the compile leg cannot judge', () => { + const found = findings(joined('avg')); + expect(found).toHaveLength(1); + expect(found[0].message).toContain('account.signed_at'); + expect(found[0].message).toContain('datetime'); + // The message must say which object the LEAF lives on, or the author reads + // the type as a claim about the dataset's own object. + expect(found[0].message).toContain('crm_account'); + }); + + it('accepts an accepted pair on that same joined field', () => { + expect(findings(joined('max'))).toEqual([]); + }); + + // [#16340] A registry-injected column is judged on the same axis as an + // authored one: the graph carries the registry's own definition, so + // `created_at` reads as the `datetime` it is — and the pair reaches the same + // backend whether the author declared the column or the platform did. + it('judges a registry-injected column', () => { + const stack = stackWith('avg', 'text', { field: 'created_at' }); + const found = findings(stack); + expect(found).toHaveLength(1); + expect(found[0].message).toContain('created_at'); + expect(found[0].message).toContain('datetime'); + }); + + it('reports once per refused measure, and once per dataset that has one', () => { + const stack = { + name: 'analytics_probe', + objects: [ + { + name: 'crm_opportunity', + sharingModel: 'private', + fields: { closed_at: { type: 'datetime' }, units: { type: 'number' } }, + }, + ], + datasets: [ + { + name: 'a', + object: 'crm_opportunity', + dimensions: [], + measures: [ + { name: 'bad_1', aggregate: 'avg', field: 'closed_at' }, + { name: 'fine', aggregate: 'avg', field: 'units' }, + { name: 'bad_2', aggregate: 'sum', field: 'closed_at' }, + ], + }, + { + name: 'b', + object: 'crm_opportunity', + dimensions: [], + measures: [{ name: 'bad_3', aggregate: 'avg', field: 'closed_at' }], + }, + ], + }; + const found = findings(stack); + expect(found.map((f) => f.path)).toEqual([ + 'datasets[0].measures[0].aggregate', + 'datasets[0].measures[2].aggregate', + 'datasets[1].measures[0].aggregate', + ]); + }); +}); + +describe('measure-aggregate-field-type-refused — reaches the author through the shared registry', () => { + // The rule exists to reach an author, and it reaches one only if the table + // every command runs carries it. This is the before/after artifact of #16354 + // made permanent: the same pair, through the same door the CLI uses. + it('fires through runAuthoringRules on all three commands, and only on the refused pair', () => { + const stack = stackWith('avg', 'datetime'); + for (const command of ['validate', 'build', 'lint'] as const) { + const found = runAuthoringRules(command, { normalized: stack, parsed: stack }).filter( + (f) => f.rule === RULE, + ); + expect(found, command).toHaveLength(1); + expect(found[0].severity, command).toBe('error'); + } + const accepted = stackWith('avg', 'number'); + expect( + runAuthoringRules('lint', { normalized: accepted, parsed: accepted }).filter( + (f) => f.rule === RULE, + ), + ).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-dataset-measure-aggregates.ts b/packages/lint/src/validate-dataset-measure-aggregates.ts new file mode 100644 index 00000000000..4d2666de8e0 --- /dev/null +++ b/packages/lint/src/validate-dataset-measure-aggregates.ts @@ -0,0 +1,245 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16354 — the AUTHORING-TIME leg of the aggregate × field-type contract] + * A dataset measure pairs an `aggregate` with a `field`. This rule refuses the + * pairs `AGGREGATE_FIELD_TYPE_COMPATIBILITY` (`@objectstack/spec/data`, #16353) + * does not accept, at the moment the author writes them. + * + * All judgement lives in the SHARED predicate — + * `isAggregateCompatibleWithFieldType`, the same call the compile leg makes — + * so this file is only the walk: where dataset measures live in a stack, how a + * field name becomes a declared `FieldType`, and how a refused pair becomes a + * finding with a location. ⛔ A second compatibility table here would BE the + * two-accounts-of-one-pair drift the spec table exists to remove; if a verdict + * seems wrong, the table is where to read and to argue. + * + * ## Why an authoring-time rule when the compile leg already refuses + * + * Both legs exist by the director ruling of decision batch #59 (2026-09-06, + * 「both legs, table in spec」). The compile leg (`dataset-compiler`, + * `service-analytics`) answers `400 DATASET_INVALID` when a query is built — + * which is the last door before a backend, and the FIRST one a human sees only + * if somebody happens to open the surface that binds this dataset. An + * incoherent measure can sit in a config file, survive `os validate`, ship, and + * surface as a 400 on someone else's dashboard weeks later, or — the dangerous + * half the ruling was measured on — as a plausible number: SQLite coerces a + * `datetime` column's canonical UTC text by its leading digits, so + * `avg(submitted_at)` returns the average YEAR (`2025.5`) with no error and no + * log, and the DEV datasource in this platform's default flow is SQLite. + * + * This rule puts the same verdict where the author is standing, before the + * document is committed. The compile leg is unchanged: this is a second + * consumer of one table, never a relaxation of the first. + * + * ## Where it reaches FURTHER than the compile leg, and why that is not a guess + * + * The compile leg returns early on a dotted `relationship.field` reference + * (`if (field.includes('.')) return;`): its declared-type source is the host's + * `AnalyticsServiceConfig.sourceFieldMeta`, which answers for the BASE object + * only, so a column living on a joined object is *not judged rather than judged + * wrongly*. At authoring time the whole object graph is in hand, so + * {@link resolveFieldPath} resolves the hops and hands back the LEAF's declared + * type — a read of the author's own declaration, not an inference. So a dotted + * pair is judged here, and the spec module's instruction is honoured in the + * direction that matters: a path whose type cannot be resolved is never handed + * to the predicate as a guess (see the skips below). + * + * ## Its relation to the two neighbours that look like it + * + * - `measure-aggregate-incoherent` (`validate-widget-bindings.ts`) is the + * SEMANTIC opinion — "does this number mean anything" — and it is advisory + * and suppressible. It fires on `sum` / `count_distinct` over a `percent` + * field. The table refuses `sum` × `percent` too, on that predicate's own + * authority (`aggregate-field-type-compatibility.ts` says so), so that ONE + * pair is reported twice: an advisory about meaning, and this gating refusal + * about the contract. The overlap is deliberate rather than tidied away, + * because the two questions have different answers elsewhere — + * `count_distinct` × `percent` is advised and ACCEPTED by the table, and + * `avg` × `datetime` is refused here and not advised there. + * - `rollup/non-numeric-aggregand` (`data-model-rules.ts`) judges a `summary` + * field's `summaryOperations`, and deliberately does NOT read this table: + * there the answer is STORED into a numeric column, so it refuses + * `min` / `max` over the temporal class, which this table accepts. Different + * question, different door, no shared verdict. + * + * ## Skips — never hand the predicate a guess + * + * `aggregate-field-type-compatibility.ts` states the consumer's tier: + * *"a consumer that cannot resolve a field's type … must NOT call the predicate + * with a guess — 'cannot answer, do not block' is the consumer's tier"*. So + * this rule stays silent when: + * + * 1. the dataset names no base `object`, or one this stack does not define, + * or one that declares no readable field map (ADR-0015 `external` and + * datasource-introspected schemas) — `validate-object-references.ts` owns + * the first, and the rest are the object graph's `unknowable` verdicts; + * 2. the measure writes no `aggregate` or no `field` — a plain `count` and a + * `derived` measure legitimately carry no field, and a non-string in + * either position is the schema's refusal to give, not this rule's; + * 3. the path does not RESOLVE — a dangling reference is + * `dataset-field-unknown`'s finding (`validate-dataset-references.ts`), + * and one typo must not also yield a type verdict about a column that + * does not exist; + * 4. the resolved leaf carries no declared `type` — an untyped field, and the + * driver-provisioned `id`, for which no definition table has an answer; + * 5. the `aggregate` is outside the table's own vocabulary — `AggregationFunction` + * is a closed enum and a value outside it is a schema error, so refusing it + * here would report a vocabulary problem as a compatibility one. + * + * A registry-injected column is NOT a skip: since #16340 the graph carries the + * registry's own definition, so `created_at` reads as the `datetime` it is and + * `avg` over it is refused exactly as over an authored field — the pair reaches + * the same backend either way. + */ + +import { + AGGREGATE_FIELD_TYPE_COMPATIBILITY, + isAggregateCompatibleWithFieldType, +} from '@objectstack/spec/data'; + +import { + indexObjectGraph, + isUnjudgeable, + recordsOf, + resolveFieldPath, + type ObjectGraph, +} from './object-graph.js'; + +/** + * Stable diagnostic id. Named for the AXIS it judges — the field's declared + * type — so it reads apart from its advisory neighbour + * `measure-aggregate-incoherent`, which judges the same subject on the + * semantic axis. `refused` is the ruling's own word for a pair outside the + * table ("every other pair: refused"). + */ +export const MEASURE_AGGREGATE_FIELD_TYPE_REFUSED = 'measure-aggregate-field-type-refused'; + +export interface DatasetMeasureAggregateFinding { + /** + * Always `error`. The pair is decidable from the author's own declarations — + * no runtime state, no call graph — and the alternative to refusing it is a + * number whose value is a property of the SQL dialect. The compile leg + * answers the same pair with `400 DATASET_INVALID`, so an advisory here + * would only mean the author hears about it later, from someone else's + * dashboard. + */ + severity: 'error'; + rule: typeof MEASURE_AGGREGATE_FIELD_TYPE_REFUSED; + /** Human-readable location, e.g. `dataset "sales" › measure "avg_closed"`. */ + where: string; + /** Config path, e.g. `datasets[0].measures[2].aggregate`. */ + path: string; + message: string; + hint: string; +} + +type AnyRec = Record; + +const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v); + +const strName = (v: unknown): string | undefined => + typeof v === 'string' && v.length > 0 ? v : undefined; + +/** + * The table as a Map, built once — for the MESSAGE only. + * + * A Map rather than property access on the frozen record because a + * property-key lookup also answers for `Object.prototype` members, so an + * author's `aggregate: 'toString'` would read as a declared row. The VERDICT + * is never taken from here: it is + * {@link isAggregateCompatibleWithFieldType}'s, which is fail-closed on both + * vocabulary and shape. This Map only PRESENTS the same table — the accepted + * set for the refused aggregate, and the aggregates that would accept this + * field — so the message cannot name a set the verdict was not taken from. + */ +const ACCEPTED_TYPES_BY_AGGREGATE: ReadonlyMap = new Map( + Object.entries(AGGREGATE_FIELD_TYPE_COMPATIBILITY).map( + ([fn, types]) => [fn, types as readonly string[]] as const, + ), +); + +/** Every aggregate the table accepts for `fieldType` — always non-empty (`count` accepts any type). */ +function aggregatesAccepting(fieldType: string): string[] { + const accepting: string[] = []; + for (const [fn] of ACCEPTED_TYPES_BY_AGGREGATE) { + if (isAggregateCompatibleWithFieldType(fn, fieldType)) accepting.push(fn); + } + return accepting; +} + +/** + * Refuse every dataset measure whose `aggregate` the field's declared + * `FieldType` cannot carry. Returns findings (empty = clean). Pure + * `(stack) => Finding[]` (ADR-0019): no I/O, and safe on both the + * schema-parsed stack and the raw config the `os lint` path carries. + */ +export function validateDatasetMeasureAggregates(stack: unknown): DatasetMeasureAggregateFinding[] { + const findings: DatasetMeasureAggregateFinding[] = []; + if (!isRec(stack)) return findings; + + const datasets = recordsOf(stack.datasets); + if (datasets.length === 0) return findings; + + const graph: ObjectGraph = indexObjectGraph(stack); + + datasets.forEach((ds, di) => { + // ── Skip 1: no base object, or one the graph cannot answer for ── + const object = strName(ds.object); + if (!object) return; + if (!graph.has(object) || !graph.get(object)) return; + + const dsName = strName(ds.name) ?? `#${di}`; + + recordsOf(ds.measures).forEach((measure, k) => { + // ── Skip 2: nothing written in one of the two positions ── + const aggregate = strName(measure.aggregate); + const field = strName(measure.field); + if (!aggregate || !field) return; + + // ── Skip 5: the aggregate is outside the table's vocabulary ── + const accepted = ACCEPTED_TYPES_BY_AGGREGATE.get(aggregate); + if (!accepted) return; + + // ── Skip 3: the path does not resolve — `dataset-field-unknown`'s finding ── + const verdict = resolveFieldPath(graph, object, field); + if (!verdict || isUnjudgeable(verdict) || verdict.kind !== 'ok') return; + + // ── Skip 4: the leaf declares no type, so nothing can be asked about it ── + const fieldType = verdict.meta?.type; + if (!fieldType) return; + + if (isAggregateCompatibleWithFieldType(aggregate, fieldType)) return; + + const measureName = strName(measure.name) ?? `#${k}`; + const onObject = + verdict.object === object + ? `object "${object}"` + : `object "${verdict.object}" (reached through this dataset's join chain)`; + findings.push({ + severity: 'error', + rule: MEASURE_AGGREGATE_FIELD_TYPE_REFUSED, + where: `dataset "${dsName}" › measure "${measureName}"`, + path: `datasets[${di}].measures[${k}].aggregate`, + message: + `measure "${measureName}" applies aggregate "${aggregate}" to field "${field}", which ` + + `${onObject} declares as \`${fieldType}\`. That pair is refused by the aggregate × ` + + `field-type compatibility table in @objectstack/spec, so the number a backend returns ` + + `for it is a property of the SQL dialect rather than of the data — one coerces the ` + + `stored form and answers something plausible, another has no such function and fails ` + + `at query time. "${aggregate}" accepts: ${accepted.join(', ')}.`, + hint: + `Either point "${aggregate}" at a field of an accepted type, or aggregate ` + + `"${field}" with one its \`${fieldType}\` type accepts: ` + + `${aggregatesAccepting(fieldType).join(', ')}. ` + + `\`count\` / \`count_distinct\` accept every type because they read no arithmetic off ` + + `the value; a quantity that must be added up or averaged has to be STORED as a ` + + `numeric field (a computed column) and aggregated as one. The compile leg refuses ` + + `this same pair with \`400 DATASET_INVALID\` before any SQL is emitted, so this is ` + + `the same fix made earlier.`, + }); + }); + }); + + return findings; +}