Skip to content

Commit 986236b

Browse files
committed
docs(lint): state the aggregate table's role exactly
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
1 parent 0f5ea11 commit 986236b

3 files changed

Lines changed: 10 additions & 4 deletions

File tree

.changeset/rollup-non-numeric-aggregand.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
- **The accept set** is the numeric class union the boolean class, read from `NUMERIC_VALUE_TYPES` and `BOOLEAN_VALUE_TYPES` rather than typed out. The first is the set that DEFINES the criterion — it is the membership `valueSchemaFor` consults to answer `z.number().finite()`, so a type joining it moves the value contract and this door together. The second is admitted on the authority of the `min(flag)=0` / `max(flag)=1` ruling pinned by the spec's own `AGGREGATION_CASES` (#11152): the answer is a number, so it fits.
1010
- **It is NOT `isAggregateCompatibleWithFieldType`.** That table deliberately accepts `min`/`max` over 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 accept the very declaration this rule exists to refuse. The two questions look alike and are not — "can every backend give one answer" versus "does that answer fit the column this roll-up is stored into" — so this predicate is that table's `min`/`max` row narrowed by exactly the temporal class, and a test pins the disagreement.
11-
- **Scope.** `min`/`max` only. `count` reads no value off the field; `sum`/`avg` over a non-numeric child is a different shape, one the aggregate table already refuses, and is judged elsewhere.
11+
- **Scope.** `min`/`max` only. `count` reads no value off the field; `sum`/`avg` over a non-numeric child is a different shape, whose accept set the aggregate table's own rows already exclude, and is not widened into here.
1212
- **Silent where it cannot resolve.** An unknown child object, a field the child does not declare, or a field with no declared type produce no finding — the aggregate table's own consumer tier ("a consumer that cannot resolve a field's type must NOT call the predicate with a guess"). A partially-loaded model cannot draw a false refusal.
1313

1414
No export moves: the rule id is an inline literal inside the already-exported `lintDataModel`, beside `rollup/missing-summary`. Measured across this repository, no declaration trips the new refusal — all three `min`/`max` roll-ups aggregate a `number` child field — so this adds a door rather than migrating anything.

packages/lint/src/data-model-rules.summary-rollup.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,10 @@ describe('rollup/non-numeric-aggregand — stays silent where it cannot resolve,
169169
expect(findings(model('datetime', 'count'))).toEqual([]);
170170
});
171171

172-
// `sum` / `avg` over a non-numeric child is a different shape (the analytics
173-
// table already refuses those pairs) and is deliberately not this rule's.
172+
// `sum` / `avg` over a non-numeric child is a different shape, whose accept
173+
// set the analytics table's own rows already exclude, and is deliberately not
174+
// this rule's. (Nothing in this tree consults that table — an open gap
175+
// reported with this change, not one this rule silently absorbs.)
174176
it('does not fire for sum or avg', () => {
175177
expect(findings(model('datetime', 'sum'))).toEqual([]);
176178
expect(findings(model('datetime', 'avg'))).toEqual([]);

packages/lint/src/data-model-rules.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,11 @@ export function lintDataModel(objects: any[]): LintIssue[] {
648648
//
649649
// Scoped to `min`/`max` deliberately: `count` ignores the field
650650
// entirely, and `sum`/`avg` over a non-numeric child is a different
651-
// shape — one the analytics table already refuses — judged elsewhere.
651+
// shape, whose accept set the analytics table's own rows already
652+
// EXCLUDE. Measured on this tree, nothing consults that table on the
653+
// roll-up door (or anywhere else), so that is an open gap, not a
654+
// refusal this rule may lean on — and widening here to cover it would
655+
// be a second account of a pair the table already rules on.
652656
if (type === 'summary') {
653657
const ops = def.summaryOperations;
654658
const fn = ops?.function;

0 commit comments

Comments
 (0)