feat(lint): refuse a min/max roll-up whose answer cannot fit the summary column - #17012
Conversation
…ary column `FieldSchema.summaryOperations` admits `min`/`max` over ANY child field, and `aggregateSummaryValue` returns the driver's answer verbatim (only an empty-set fallback). A `summary` field is a member of the spec's `NUMERIC_VALUE_TYPES`, so `valueSchemaFor` answers `z.number().finite()` for it. An ordinary "latest shipment" roll-up — `max` over a `datetime` child field — therefore computes an instant into a column the value contract says holds a finite number, and nothing between author and driver correlated the two. Add `rollup/non-numeric-aggregand` (error) to `lintDataModel`, beside `rollup/missing-summary`. Its predicate is the roll-up door's OWN: the numeric class union the boolean class, read from `NUMERIC_VALUE_TYPES` and `BOOLEAN_VALUE_TYPES` — the analytics table's min/max row narrowed by exactly the temporal class, because that table answers "can every backend give one answer" while this door answers "does that answer fit the column this roll-up is stored into". Silent on anything the pass cannot resolve (unknown child object, undeclared field, missing type), per the aggregate table's own consumer tier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x
…mmary-rollup-refusal-lint
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x
The scope note claimed the analytics table 'already refuses' sum/avg over a non-numeric child. Measured on this tree, AGGREGATE_FIELD_TYPE_COMPATIBILITY and isAggregateCompatibleWithFieldType have no consumer at all outside their own module and test, so the table's rows EXCLUDE those pairs but nothing enforces that exclusion. Say what the tree does, and record the gap as a gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c90180792f2e83f1f13f652725810595ea9bf4d2 && git checkout c90180792f2e83f1f13f652725810595ea9bf4d2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 92949593df61413dcd79f339e609e8e8df6bc1a7 986236bff4adeb06314c342af5739bfd064044a8 && git checkout -B drift-repro 92949593df61413dcd79f339e609e8e8df6bc1a7 && git merge --no-ff 986236bff4adeb06314c342af5739bfd064044a8
node scripts/docs-audit/affected-docs.mjs --json 92949593df61413dcd79f339e609e8e8df6bc1a7 |
Fixes #16237
A
min/maxroll-up over a non-numeric child field is refused at authoring time, aterror.Clause-② (carried from the dispatch, verbatim)
The defect
Three shipped statements, each re-read on this branch's base:
packages/spec/src/data/field-value.zod.tsputssummaryinNUMERIC_VALUE_TYPES, andvalueSchemaForanswersz.number().finite()for every member of that class. The value contract for asummaryfield is a finite number.packages/drivers/driver-sql/src/sql-driver.ts—case 'summary': col = table.float(name); break;insidecreateColumn. (Located by the assignment. There is a second switch casing tosummaryinsidevarcharColumnChars, which returns null; that one is a decoy.)FieldSchema.summaryOperationsadmitsmin/maxover ANY child field, and objectql'saggregateSummaryValuereturns the driver's answer verbatim — only an empty-set fallback stands between the backend and the stored value.So
{ type: 'summary', summaryOperations: { object: 'invoice_line', field: 'shipped_at', function: 'max' } }— an ordinary "latest shipment" roll-up — computes an instant into a column the value contract says holds a finite number. Nothing between author and driver correlated the two. Refused at authoring rather than tolerated in a consumer (Prime Directive #12).The rule as landed
rollup/non-numeric-aggregand, severityerror, inlintDataModelbesiderollup/missing-summary. The rule id is an inline string literal, not an exported constant — therollup/*shape, not theunique/*shape — so the diff adds zero exports.min/maxonly.Which spec set the predicate reads, and why it is the right one
The accept set is composed as
NUMERIC_VALUE_TYPESunionBOOLEAN_VALUE_TYPES, imported from@objectstack/spec/data.NUMERIC_VALUE_TYPESis not merely a numeric list — it is the set that defines the acceptance criterion. It is the membershipvalueSchemaForconsults to answerz.number().finite(), so a type joining that class moves thesummaryvalue contract and this door together, in one edit.BOOLEAN_VALUE_TYPESis admitted on the authority of ruling [finding]AGGREGATION_ROWShas no boolean column, so the cross-driver aggregation conformance family cannot see a boolean aggregand on any face #11152, pinned by the spec's ownAGGREGATION_CASES(min(flag)=0,max(flag)=1, enrolled on six backends) and implemented by driver-sql'sintcast on Postgres (driver-sql: boolean aggregands need a lowering cast on PG (+ a MySQL min/max presentation check) — the ruledfalse/true+ arithmetic answers are unproducible on the PG face #11635). The answer is a number, so it fits.Deliberately not read:
NON_TEXT_STORED_VALUE_TYPES, whose membership is these same two classes today. It is defined by a third question — "is the stored value never text" (#14079) — and excludes the temporal class for a dialect reason, not for this one. Composing the union states why each half is in, so a future member of that set cannot widen this door as a side effect.Why it is NOT
isAggregateCompatibleWithFieldTypeThat table deliberately accepts
min/maxover the temporal class, because there the answer is returned to a caller and "return[s] a value of the field's OWN type" (#15768). Reusing it here would have accepted the card's own example and the rule would have been green because it never fires.Two questions that look alike and are not: "can every backend give one answer" versus "does that answer fit the column this roll-up is stored into". This predicate is that table's
min/maxrow narrowed by exactly the temporal class.A discrimination test pins the disagreement in both directions: the analytics table accepts every temporal
min/maxpair this rule refuses, and the two agree on every otherFieldType. It is the assertion that stops someone simplifying the predicate back into a single call later.Population re-measurement, with a firing control
Measured on this branch (base
fe2b7554a, merged to2fd714f4f), over every tracked.ts/.json/.yml/.mdxfile excludingdist,node_modules, CHANGELOGs, generated bundles andcontent/docs/references/:The zero carries a firing positive control — the card's own example,
max(invoice_line.shipped_at)over adatetime, appended to the same scanner input:All three in-repo
min/maxroll-ups aggregatetask.estimate, anumber. On a population of 0 this adds a door, not a migration — no app in this tree reddens.Verification
Every reading below was taken at
986236bff, the final commit.pnpm --filter @objectstack/lint testpnpm --filter @objectstack/lint typechecktsc --noEmit+check:test-typecheck: OKpnpm --filter @objectstack/lint buildcheck-dts-emitted: 4/4pnpm --filter @objectstack/cli exec vitest run --project unit test/data-model-rules.test.ts test/score.test.tslintDataModel's existing tests and the metadata-eval rubric)pnpm --filter @objectstack/spec check:api-surfacepnpm --filter @objectstack/spec check:export-originspnpm lint(eslint . --no-inline-config, whole repo)pnpm check:nul-bytespnpm check:type-check-debtpnpm check:type-check-coveragenode scripts/pm/dispatch-gates.mjs --commandsderived set--ranreconciles 0 UNRUNpnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET) — see belowapi-surface / export-origins: zero movement, and no generated artifact under
packages/spec/is dirty. Fence 2 does not fire.check:dual-build-cjs-loadsis the one derived family this branch could not measure: it reads built output and nine packages unrelated to this diff (client-react, four connectors, two knowledge plugins,organizations,studio) have nodistin this worktree. Its own text says "This is NOT a pass: nothing was measured." It wants a whole-treepnpm build, which is CI's run — declared here as a narrowing, not read as a clearance. This diff adds no package entry point and moves noexports, so it cannot move that gate.The gate is not vacuous — ablation
The predicate was mutated to the analytics table's own
min/maxrow (numeric union temporal union boolean), which is what "simplifying it back intoisAggregateCompatibleWithFieldType" produces:b419e1c1toabaf144f.git checkout HEAD -- PATH, hash back tob419e1c1(byte-identical to the HEAD blob),git diff HEADempty, marker count 0.The mutation script carried
trap restore EXIT INT TERMthroughout; no ablation artifact remains in the diff.Changeset — both halves, measured on the deciding tree (
986236bff)Half 1 — is the changed PATH in
packages/lint's ownfiles[]? No.files[] = ["dist","README.md","CHANGELOG.md"]; 17 published entries.src/data-model-rules.tspublished: false. Discriminating negative controls, siblings the same mechanism does not ship:tsup.config.tsfalse,tsconfig.jsonfalse,src/index.tsfalse. Positive controls:README.mdtrue,CHANGELOG.mdtrue.Half 2 — is the changed TEXT in what the package actually publishes? Yes.
Fourteen published
dist/entries scanned:rollup/non-numeric-aggreganddist/index.cjs,dist/index.js)dist/index.cjs,dist/index.js)rollup/missing-summaryThe rule ships in the package's
dist, so it is a behaviour change for consumers of@objectstack/lintand ofos lint:.changeset/rollup-non-numeric-aggregand.md,@objectstack/lint: minor. Notskip-changeset.Governance
Re-run on the final file list, not recalled.
验收备注
Findings for the dispatching seat to file. None is folded into this PR.
AGGREGATE_FIELD_TYPE_COMPATIBILITYhas no consumer at all. Its module TSDoc says the table "is the contract both consumer legs execute — the compile-time refusal in the dataset compiler (No layer refuses an incoherent aggregate / field-type pair — a dataset measureavgover a datetime works on SQLite and errors on Postgres #16099) and the authoring-time lint rule — so the two cannot drift into two accounts of one pair." Measured repo-wide at986236bff, excludingdist:isAggregateCompatibleWithFieldTypeandAGGREGATE_FIELD_TYPE_COMPATIBILITYappear only in their own module, their own test, the api-surface snapshot and two changesets. Neither leg exists. Control:isIncoherentAggregate, cited in the same module's TSDoc, does have a production consumer (packages/lint/src/validate-functional-completeness.ts), so the grep is not blind. This is declared-not-enforced (Prime Directive chore: version packages #10's corollary) and a contract-text defect, corroborating this card rather than fixed by it. After this PR the table's only caller in the tree is this rule's discrimination test.summaryOperations.field'sdescribestates the contract in one of its two spellings and enforces neither. Inpackages/spec/src/data/field.zod.tsit reads "Field on child object to aggregate (ignored for count)", while the AI blueprint'sStrictSummaryOperationssays "Numeric field on the CHILD to aggregate".packages/spec/src/data/field.zod.tsis inside [finding]packages/spec/liveness/field.json: two livedisplayFieldnotes credit objectui with reading the snake_case spellings that objectui#7155 removed #15540's live claim, so this is reported, not written.sum/avgover a non-numeric child roll-up is unrefused too. This rule is scoped tomin/maxas dispatched. The aggregate table's own rows exclude those pairs, but per finding 1 nothing consults them, so the exclusion is not enforced anywhere. A different shape from this card, and it needs its own ruling rather than a widening here.Not measured, asserted nowhere: MySQL / Postgres insert behaviour when a non-numeric value reaches the float column. Neither engine was provisioned in this container. Nothing in the code, the tests or this body claims anything in either direction.
Not filed, noted only: the population scanner and the ablation script are one-shot instruments and were deliberately not left in the tree.
Generated by Claude Code
Generated by Claude Code