Repository navigation
Commit 362dcc3
lint: refuse a dataset measure whose aggregate its field type cannot carry (#19138)
Fixes #16354
Clause-②: yes (narrowing)
The lint leg of the aggregate × field-type contract (director ruling,
decision batch #59, 2026-09-06, 「both legs, table in spec」). A dataset
measure pairs an `aggregate` with a `field`;
`AGGREGATE_FIELD_TYPE_COMPATIBILITY` in `@objectstack/spec` declares
which of those pairs every backend answers the same way, and nothing in
the authoring path read it. New gating rule
`measure-aggregate-field-type-refused` in
`packages/lint/src/validate-dataset-measure-aggregates.ts`, registered
in `AUTHORING_RULES`, so all three commands run it. The verdict is the
shared predicate's on every pair — `isAggregateCompatibleWithFieldType`,
the same call the compile leg makes — and there is no second table in
this repo.
## The take-order test, as an artifact
The maintainer's test for a contract-surface card: take a piece of
author-written metadata, feed it in before and after the change, and see
whether accept/reject flips. Both arms below were **run**, not reasoned
from the diff: one probe script, byte-identical in both arms, driving
the real shared registry (`runAuthoringRules('lint', …)` — the table `os
lint` / `os validate` / `os build` all run).
The sample (author-written metadata, three measures over one object):
```ts
objects: [{ name: 'crm_opportunity', sharingModel: 'private', fields: {
name: { type: 'text' }, units: { type: 'number' },
closed_at: { type: 'datetime' }, win_rate: { type: 'percent' },
} }],
datasets: [{ name: 'opportunity_metrics', object: 'crm_opportunity', dimensions: [], measures: [
{ name: 'avg_closed_at', aggregate: 'avg', field: 'closed_at' }, // positive control
{ name: 'avg_units', aggregate: 'avg', field: 'units' }, // negative control
{ name: 'total_win_rate', aggregate: 'sum', field: 'win_rate' }, // third control
] }]
```
| measure | BEFORE (`c70581bc8`, pristine worktree) | AFTER (this
branch) |
|---|---|---|
| `avg(closed_at)` — `datetime` | **ACCEPTED**, no finding |
**REFUSED**, `error` |
| `avg(units)` — `number` | ACCEPTED, no finding | ACCEPTED, no finding
(unchanged) |
| `sum(win_rate)` — `percent` | ADVISED only —
`measure-aggregate-incoherent`, `warning` | **REFUSED**, `error`, beside
that same warning |
| total findings on the sample | 1 | 3 |
⇒ accept/reject flips on author-written metadata. Contract surface.
**BEFORE arm** — taken on the pristine worktree (`git status
--porcelain` printed 0 lines) at the branch point, through the package
source, with `@objectstack/spec` freshly built in this worktree
beforehand:
```
$ pnpm exec tsx probe.mjs /home/user/wt-16354/packages/lint/src/index.ts
total findings over the whole sample: 1
--- measure[0] avg(closed_at) "avg_closed_at" -> ACCEPTED (no finding)
--- measure[1] avg(units) "avg_units" -> ACCEPTED (no finding)
--- measure[2] sum(win_rate) "total_win_rate" -> ADVISED (warning)
[warning] measure-aggregate-incoherent @ datasets[0].measures[2]
```
**AFTER arm** — same probe, unchanged, on this branch; run twice,
through the source and through a freshly built `dist`, with identical
verdicts:
```
$ pnpm exec tsx probe.mjs .../packages/lint/src/index.ts # and: node probe.mjs .../packages/lint/dist/index.js
total findings over the whole sample: 3
--- measure[0] avg(closed_at) "avg_closed_at" -> REFUSED (error)
[error] measure-aggregate-field-type-refused @ datasets[0].measures[0].aggregate
--- measure[1] avg(units) "avg_units" -> ACCEPTED (no finding)
--- measure[2] sum(win_rate) "total_win_rate" -> REFUSED (error)
[warning] measure-aggregate-incoherent @ datasets[0].measures[2]
[error] measure-aggregate-field-type-refused @ datasets[0].measures[2].aggregate
```
**Build provenance, because a stale `dist` reads exactly like a real
reading.** Both arms resolve `@objectstack/spec` through its build.
`packages/spec/dist` was absent in this fresh worktree and was built
here (`pnpm --filter '@objectstack/lint^...' build`, run twice: once
before the BEFORE arm, once after merging `main`); `packages/lint/dist`
was built for the dist arm. **Nothing was served from a turbo cache: no
`turbo` process was in either pipeline** — `pnpm --filter … build`
invokes each package's own script directly, and `grep -ciE 'cache
hit|turbo|FULL TURBO'` over the build logs returns 0. The spec `dist`
timestamp is newer than every source it was built from.
## The three controls the card names
All three are permanent tests in
`validate-dataset-measure-aggregates.test.ts` (23 tests in the file; the
whole package is 105 files / 3978 tests, green):
1. **positive control** — `avg` over a `datetime` field **fires**: one
`error` at `datasets[0].measures[0].aggregate`, message pinned to name
the aggregate, the field, its type and every accepted type.
2. **negative control** — `avg` over a `number` field is **silent**.
Generalised rather than left as one case: `avg`/`sum` over the whole
numeric class, all four arithmetic and order aggregates over the boolean
class (maintainer ruling #11152), `min`/`max` over the temporal class,
and `count`/`count_distinct` over **every** declared `FieldType` are
each asserted silent.
3. **`sum` over `percent`** **fires** — the pair `analytics-service.ts`
already calls incoherent. It now carries two findings: the older
advisory about meaning (suppressible) and this gating refusal about the
contract. Deliberate, and documented in the rule's header: the two
questions disagree elsewhere — `count_distinct` × `percent` is advised
and **accepted** by the table, `avg` × `datetime` is refused here and
not advised there.
And the strongest anti-false-positive assertion, because a false
positive here is worse than the gap being closed: the rule's verdict is
compared against `isAggregateCompatibleWithFieldType` for **every
aggregate × every declared `FieldType`** (6 × 44 = 264 pairs), with
floors on *both* sides of the sweep (more than 50 refused, more than 50
accepted) so neither "stopped firing" nor "fires on everything" can
satisfy the equality vacuously.
## The message the rule emits, verbatim
```
measure "avg_closed_at" applies aggregate "avg" to field "closed_at", which object
"crm_opportunity" declares as `datetime`. 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. "avg" accepts: number, currency, percent, rating, slider, progress,
summary, boolean, toggle.
```
hint:
```
Either point "avg" at a field of an accepted type, or aggregate "closed_at" with one
its `datetime` type accepts: count, count_distinct, min, max. `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 repair made
earlier.
```
Both halves are **computed from the table** — the accepted set for the
refused aggregate, and the aggregates that would accept this field's
type — never prose restating it, so neither can drift from the rows.
## Where it stands down, and where it reaches further
Silent wherever the type cannot be resolved, per the spec module's own
instruction that a consumer must not hand the predicate a guess: a
dataset naming no base object or one this stack does not define, an
object with no readable field map, a field path that resolves to nothing
(that is `dataset-field-unknown`'s finding — one typo must not also
yield a type verdict), an untyped leaf, a non-string in either position,
and an aggregate outside the closed `AggregationFunction` vocabulary.
Each is a test.
It reaches further than the compile leg in exactly one direction: a
dotted `relationship.field` reference. The compile leg returns early on
those because its declared-type source answers for the base object only;
authoring time has the whole object graph, so the leaf's declared type
is a read rather than an inference, and the refusal names the object the
leaf lives on. Registry-injected columns are judged on the same axis as
authored ones.
## Verification
- `pnpm --filter @objectstack/lint test` — 105 files, **3978 passed, 0
skipped**. Re-run after merging current `main` into this branch (the
merge landed sibling work inside this package), and again after the last
commit.
- `pnpm --filter @objectstack/lint typecheck` — clean, including
`check:test-typecheck` over the test layer.
- `pnpm --filter '@objectstack/lint^...' build` and `pnpm --filter
@objectstack/lint build` — clean, dts emitted.
- Gates run locally, each exit code captured before any pipe:
`check:nul-bytes`, `check-empty-changeset --base origin/main`,
`check-adr-0087-registration --base origin/main` (judges this changeset:
`[BREAKING+clause-②-narrowing] not-required (already-registered)`),
`check-changeset-no-major`, `check:changeset-gate-self-tests`,
`check-changeset-fixed`, `check:doc-anchors`, `check-doc-frontmatter`,
`check:docs-single-h1`, `check-docs-section-name`,
`check:doc-authoring`, `check:docs-transcript-drift`,
`check:docs-spec-enumerations`, `check:docs-redirects`,
`check:docs-audit-scope`, `check:published-files`,
`check:published-readme-links`, `check:cross-package-test-inputs`,
`check:test-source-alias`, `check:type-check-coverage`,
`check-undeclared-dep-imports`, `check-comment-mask-adoption`,
`check-comment-mask-corpus`, `check-keyed-text-bounds`,
`check-section-landing-index`, `check-doc-route-spelling --advisory`,
`docs-audit/check-affected-docs`, `docs-audit/check-drift-comment`,
`check:cli-examples-parity`, `check:corpus-claim-drift`, spec
`check:docs` and `check:yaml-examples`, and the lint package's own two
doc gates. **All green.**
- **Two of those gates went red on my first pass and drove real
corrections**, both mechanical consequences of registering a rule:
`check:doc-authoring` refused a tracker id inside the new entry's
runtime `surfaceReason` string, and `check:docs-transcript-drift`
derives the author-time rule count from the registry and found four CLI
transcripts still printing the old one (45 → 46).
- ESLint, as a declared narrowing rather than a farm run: `eslint
--no-inline-config --format json` over the four touched TypeScript files
— **4 files linted, 0 errors, 0 warnings**. The invariance that makes
the narrowing a measurement rather than a skipped check is stated by the
config itself: this repo runs one `eslint.config.mjs`, which *"never
enables type-aware linting (no `parserOptions.project`, no typed
`@typescript-eslint` rules) for ANY file, test or not"* — so no verdict
on an untouched file can move on account of this diff, which changes no
config and no shared roster. The population is that config's own `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']` blocks minus their `ignores`;
the repo-wide run belongs to CI.
- `check:type-check-debt` reported **PREREQUISITE NOT MET (exit 3)** —
the whole-workspace closure is not built here, and it says in its own
words that this "is NOT a pass and NOT a finding". Recorded as not
measured, not as green. Its sibling `check:type-check-coverage` did run
and passed.
- The gate derivation (`scripts/pm/dispatch-gates.mjs --commands`) names
86 families for this change set; 26 were run locally and the rest —
repo-wide scans and populations CI owns — are left to CI, as a declared
narrowing.
## Acceptance notes
Out of scope here, filed nowhere by this PR, recorded so they are not
rediscovered:
- **The `dataset` metadata type is not gated at the runtime publish door
at all.** `dataset` is a registered type with `allowRuntimeCreate: true`
and `supportsOverlay: true`, but `TYPE_TO_STACK_KEY` in
`runtime-gate.ts` maps no `dataset` row, so a dataset write builds no
per-write snapshot and **zero** author-time rules dispatch for it —
including the existence rules whose whole failure mode is a chart that
renders empty. That is why this rule declares `surfaces: cli` with a
reason naming the type axis rather than the snapshot's contents: both
collections it reads are carried, so the usual reason does not apply.
Mapping that type is a card of its own, and it would hand the door every
rule reading `stack.datasets` at once.
- **The refusal's divergence and remedy prose lives only in
`service-analytics`.** The compile leg builds its message from two local
helpers; `@objectstack/lint` cannot import them (its dependency
direction is lint → spec, never a service), so this rule states the same
two facts in its own words. Two accounts of one refusal can drift.
Moving that prose into `@objectstack/spec` beside the table would make
both legs read one sentence.
- **One pair now yields two findings.** `sum` × `percent` is reported by
both this rule (`error`, contract) and `measure-aggregate-incoherent`
(`warning`, semantics, suppressible). Consolidating them is a decision
about a published rule id's severity and suppressibility, which is its
own PR by this repo's own convention.
**Note on the declaration — it diverges from the claim comment,
deliberately.** The claim carries `Clause-②: no`, which is right about
the refusal on its own: narrowing an accept set is a semantic surface
and does not by itself touch clause ②. But this diff *also* adds
published exports from `@objectstack/lint`
(`validateDatasetMeasureAggregates`,
`MEASURE_AGGREGATE_FIELD_TYPE_REFUSED`,
`DatasetMeasureAggregateFinding`) and an `AUTHORING_RULES` entry — both
widening tells against a `no` declaration — so the honest value is
`yes`, and `yes (narrowing)` is the spelling for a diff that widens
*and* narrows. Body and changeset carry the identical line. The
changeset additionally carries the migration per refused class and the
ADR-0087 disposition `not-required (already-registered …)`: the two
semantic entries registering this exact surface already exist, and the
compile-time leg declared the same disposition against the first of
them. The claim comment is the seat's to amend, not this PR's.
This PR opens as a draft and stays draft.
---
_Generated by [Claude
Code](https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e6c34f6 commit 362dcc3
9 files changed
Lines changed: 744 additions & 4 deletions
File tree
- .changeset
- content/docs
- deployment
- getting-started
- ui
- packages/lint/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
589 | 589 | | |
590 | 590 | | |
591 | 591 | | |
592 | | - | |
| 592 | + | |
593 | 593 | | |
594 | 594 | | |
595 | 595 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
189 | 189 | | |
190 | 190 | | |
191 | 191 | | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
192 | 216 | | |
193 | 217 | | |
194 | 218 | | |
| |||
396 | 420 | | |
397 | 421 | | |
398 | 422 | | |
| 423 | + | |
399 | 424 | | |
400 | 425 | | |
401 | 426 | | |
| |||
624 | 649 | | |
625 | 650 | | |
626 | 651 | | |
627 | | - | |
| 652 | + | |
628 | 653 | | |
629 | 654 | | |
630 | 655 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
265 | 265 | | |
266 | 266 | | |
267 | 267 | | |
268 | | - | |
| 268 | + | |
269 | 269 | | |
270 | 270 | | |
271 | 271 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
381 | 381 | | |
382 | 382 | | |
383 | 383 | | |
384 | | - | |
| 384 | + | |
385 | 385 | | |
386 | 386 | | |
387 | 387 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| 106 | + | |
106 | 107 | | |
107 | 108 | | |
108 | 109 | | |
| |||
601 | 602 | | |
602 | 603 | | |
603 | 604 | | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
604 | 639 | | |
605 | 640 | | |
606 | 641 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
53 | 65 | | |
54 | 66 | | |
55 | 67 | | |
| |||
0 commit comments