lint: refuse a dataset measure whose aggregate its field type cannot carry - #19138
os-elon-musk merged 4 commits into
Conversation
… cannot carry The authoring-time leg of the aggregate x field-type contract (director ruling, decision batch #59: "both legs, table in spec"). A dataset measure pairs an `aggregate` with a `field`; `AGGREGATE_FIELD_TYPE_COMPATIBILITY` in `@objectstack/spec` says which pairs every backend can answer identically, and until now nothing in the authoring path read it: `avg` over a `datetime` field validated clean, shipped, and became either a plausible wrong number (SQLite coerces the canonical UTC text and returns the average YEAR) or a query-time failure (PostgreSQL has no such function), decided by the deployment rather than by the document. `validateDatasetMeasureAggregates` walks `datasets[].measures[]`, resolves the field's declared type on the object graph lint already indexes, and refuses the pair when `isAggregateCompatibleWithFieldType` says no. The verdict is the shared predicate's on every pair — no second table here — and the message names the aggregate, the field, its declared type and the accepted set, with the way out computed from the same table. Silent wherever the type cannot be resolved rather than guessing: an unresolvable base object, a dangling field path (that is `dataset-field-unknown`'s finding), an untyped leaf, a non-string in either position, and an aggregate outside the table's own vocabulary. It reaches further than the compile leg in one direction only: a dotted `relationship.field` reference, whose leaf type authoring time can read and the compile leg's base-object field metadata cannot. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh
… with its changeset The rule reference gains the row and the worked example beside the dataset-axis section that already covers measures, so an author reading about chart axes finds the one about the measure itself. The changeset carries the migration the refusal prescribes, per refused class, and the ADR-0087 disposition: the two semantic entries that register this surface already exist. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh
…taset-measure-aggregate-field-type-lint
…nd re-derive the printed rule count Two obligations the local gates named, both mechanical consequences of the new registry entry: - `check:doc-authoring` refuses a tracker id inside a runtime string, since the operators and generated surfaces that read one cannot resolve it. The reason string says what the differential does instead of citing where it was ruled; the adjacent comment, which only a source reader sees, keeps the citation. - `check:docs-transcript-drift` derives the author-time rule count from the registry and found four CLI transcripts printing the old one. A registry entry moves that number, so the four move with it. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 4 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 16e494b74487164bce01fad4504386b24488d8a9 && git checkout 16e494b74487164bce01fad4504386b24488d8a9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ee5812a5e3931b64037cb46e255649a9da0e7b74 e1851367078379be69a0b80d82f2f25fe69c534d && git checkout -B drift-repro ee5812a5e3931b64037cb46e255649a9da0e7b74 && git merge --no-ff e1851367078379be69a0b80d82f2f25fe69c534d
node scripts/docs-audit/affected-docs.mjs --json ee5812a5e3931b64037cb46e255649a9da0e7b74
|
Contract reviewServed-tier: 75/75 Isolated at-tier reviewer for PR #19138 / card #16354. Tier reading: grep of my OWN transcript ( ① Derived judgmentsDeclaration
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Record written 2026-09-18T23:00Z; every reading above carries its own time or tree. Generated by Claude Code |
Seat disposition — contract review PASS, both carriers cleared, and one number in the body is wrongSeat Review of record: comment 5737203251, Carriers cleared, with provenance. This seat ran
|
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
aggregatewith afield;AGGREGATE_FIELD_TYPE_COMPATIBILITYin@objectstack/specdeclares which of those pairs every backend answers the same way, and nothing in the authoring path read it. New gating rulemeasure-aggregate-field-type-refusedinpackages/lint/src/validate-dataset-measure-aggregates.ts, registered inAUTHORING_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 tableos lint/os validate/os buildall run).The sample (author-written metadata, three measures over one object):
c70581bc8, pristine worktree)avg(closed_at)—datetimeerroravg(units)—numbersum(win_rate)—percentmeasure-aggregate-incoherent,warningerror, beside that same warning⇒ accept/reject flips on author-written metadata. Contract surface.
BEFORE arm — taken on the pristine worktree (
git status --porcelainprinted 0 lines) at the branch point, through the package source, with@objectstack/specfreshly built in this worktree beforehand:AFTER arm — same probe, unchanged, on this branch; run twice, through the source and through a freshly built
dist, with identical verdicts:Build provenance, because a stale
distreads exactly like a real reading. Both arms resolve@objectstack/specthrough its build.packages/spec/distwas absent in this fresh worktree and was built here (pnpm --filter '@objectstack/lint^...' build, run twice: once before the BEFORE arm, once after mergingmain);packages/lint/distwas built for the dist arm. Nothing was served from a turbo cache: noturboprocess was in either pipeline —pnpm --filter … buildinvokes each package's own script directly, andgrep -ciE 'cache hit|turbo|FULL TURBO'over the build logs returns 0. The specdisttimestamp 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):avgover adatetimefield fires: oneerroratdatasets[0].measures[0].aggregate, message pinned to name the aggregate, the field, its type and every accepted type.avgover anumberfield is silent. Generalised rather than left as one case:avg/sumover the whole numeric class, all four arithmetic and order aggregates over the boolean class (maintainer ruling [finding]AGGREGATION_ROWShas no boolean column, so the cross-driver aggregation conformance family cannot see a boolean aggregand on any face #11152),min/maxover the temporal class, andcount/count_distinctover every declaredFieldTypeare each asserted silent.sumoverpercentfires — the pairanalytics-service.tsalready 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×percentis advised and accepted by the table,avg×datetimeis 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
isAggregateCompatibleWithFieldTypefor every aggregate × every declaredFieldType(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
hint:
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 closedAggregationFunctionvocabulary. Each is a test.It reaches further than the compile leg in exactly one direction: a dotted
relationship.fieldreference. 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 currentmaininto this branch (the merge landed sibling work inside this package), and again after the last commit.pnpm --filter @objectstack/lint typecheck— clean, includingcheck:test-typecheckover the test layer.pnpm --filter '@objectstack/lint^...' buildandpnpm --filter @objectstack/lint build— clean, dts emitted.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, speccheck:docsandcheck:yaml-examples, and the lint package's own two doc gates. All green.check:doc-authoringrefused a tracker id inside the new entry's runtimesurfaceReasonstring, andcheck:docs-transcript-driftderives the author-time rule count from the registry and found four CLI transcripts still printing the old one (45 → 46).eslint --no-inline-config --format jsonover 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 oneeslint.config.mjs, which "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) 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 ownfiles: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']blocks minus theirignores; the repo-wide run belongs to CI.check:type-check-debtreported 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 siblingcheck:type-check-coveragedid run and passed.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:
datasetmetadata type is not gated at the runtime publish door at all.datasetis a registered type withallowRuntimeCreate: trueandsupportsOverlay: true, butTYPE_TO_STACK_KEYinruntime-gate.tsmaps nodatasetrow, 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 declaressurfaces: cliwith 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 readingstack.datasetsat once.service-analytics. The compile leg builds its message from two local helpers;@objectstack/lintcannot 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/specbeside the table would make both legs read one sentence.sum×percentis reported by both this rule (error, contract) andmeasure-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 anAUTHORING_RULESentry — both widening tells against anodeclaration — so the honest value isyes, andyes (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 dispositionnot-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