Repository navigation
Commit 4ad3ce5
fix(lint, service-analytics): a total over a formula field names the stored-number remedy, not a count (#24505)
Fixes #24426
Clause-②: no
## What changed
A dataset measure (or an authored cube measure) that takes `sum` / `avg`
/ `min` / `max` over a `formula` field is still refused. Only the
prescription changed. It used to name a count as the way out:
> Either point "sum" at a field of an accepted type, or aggregate
"leave_days" with one its `formula` type accepts: count, count_distinct.
A count read as a total is the plausible wrong number from
objectstack-ai/cloud#2861: "Total Leave Days" showed 3 records against a
true total of 10 days. The hint now gives the spec's remedy first and
names the count only as a different question:
> The field is computed on read and stores no value: store the value as
a number field (kept by a hook) and aggregate that. `count` /
`count_distinct` answer a different question: how many records.
### One source
- **Home: `@objectstack/spec/data`, in
`aggregate-field-type-compatibility.ts` (the table module).** It exports
`VIRTUAL_FIELD_AGGREGATE_REMEDY` (the sentence) and
`virtualFieldAggregateRemedy(aggregate, fieldType)`. The function
returns the sentence for a REFUSED `sum`/`avg`/`min`/`max` over a type
in `SEARCH_VIRTUAL_TYPES` (today `formula`, the drivers' storage rule).
It returns `undefined` for every other pair. Like the predicate, it
fails closed on non-string input. The class is not a new list: it reuses
`SEARCH_VIRTUAL_TYPES`, and a spec test holds every member refused on
all four value-reading rows.
- **Why there and not beside the semantic migration entry (the claim's
expected spot).** Both consumers already import that module
(`isAggregateCompatibleWithFieldType`,
`AGGREGATE_FIELD_TYPE_COMPATIBILITY`). Its TSDoc is where the module
says formula is refused because it is virtual. An extra export in a
`migrations/entries/semantic/*.ts` file would sit inside the
generated-registry tree, which holds `SemanticMigration` entries only.
Dependency direction checked: `@objectstack/lint` and
`@objectstack/service-analytics` both list `@objectstack/spec` as
`workspace:*`. Spec imports neither of them.
- **Readers:**
- lint `measure-aggregate-field-type-refused` hint
(`validate-dataset-measure-aggregates.ts`, dataset and cube legs);
- the dataset compile leg's `REMEDY_BY_SOURCE_CLASS`
(`dataset-compiler.ts`), which now asks the spec first;
- `os explain measure-aggregate-field-type-refused`
(`rule-explanations.ts`, that entry only). This module imports nothing
by design, so the entry writes the sentence out and a lint test holds it
equal to the constant. I also dropped "(a computed column)" from the
existing stored-numeric sentence, because a formula IS a computed
column, so that wording pointed the reader the wrong way.
- **Unchanged:** every other refused pair keeps its own words. That
includes `count_distinct` over a JSON-stored type and over a `multiple:
true` declaration ("… declaration accepts: count."), the temporal
sentence, the min/max sort sentence, and the non-numeric sentence. No
pair moved between accepted and refused.
## Tests (at `3ecaba7595`, after merging `origin/main`, which already
contains #24474)
- `@objectstack/lint` `validate-dataset-measure-aggregates.test.ts`. New
describe block:
- `sum` over a formula: hint `=== VIRTUAL_FIELD_AGGREGATE_REMEDY`,
contains "store the value as a number field", the remedy comes before
the `count` mention, and no "accepts: count";
- `avg`/`min`/`max` read the same sentence;
- `os explain` contains the sentence verbatim;
- controls: `sum` × `text` keeps "type accepts: count, count_distinct.",
`count_distinct` × `select` `multiple: true` keeps "declaration accepts:
count.", `count_distinct` × `json` keeps "type accepts: count.", and
`count` × formula fires nothing.
- The explain pin "STORED as a numeric field (a computed column)" is
flipped to "STORED as a numeric field and aggregated as one" plus the
remedy sentence. The suite's existing 200-character `fix:` line pin now
covers the formula hint too.
- `@objectstack/service-analytics`
`aggregate-nontemporal-measure-refusal.test.ts`: `sum`/`avg`/`min`/`max`
× `formula` gives `DATASET_INVALID` / 400 with no SQL emitted, and the
message contains the spec sentence and neither the non-numeric nor the
sort sentence. Control: `sum` × `text` keeps "For a non-numeric field".
- `@objectstack/spec` `aggregate-field-type-compatibility.test.ts`: the
four aggregates over formula return the sentence, `undefined` everywhere
else (accepted pairs and stored classes), fail-closed shape cases, one
line of at most 200 characters, and the virtual class refused on all
four rows.
### Package runs
- At `3ecaba7595` (merged head): `@objectstack/spec` `pnpm test`, 699
files / 21281 passed; spec `test:repo` was run before the merge, 70
files / 1162 passed; the three targeted files above, 76 + 49 + 28
passed.
- Before the merge (branch head `b3a054b0af` plus its changeset and
regen commits): `@objectstack/lint` `pnpm test`, 145 files / 6870
passed; `@objectstack/service-analytics` `pnpm test`, 183 files / 4498
passed, 262 skipped; `typecheck` green for lint, service-analytics and
spec.
- Gates: `node scripts/pm/dispatch-gates.mjs --commands` at `3ecaba7595`
derived 91 commands. All 91 were run, every one exited 0, and `--ran`
reports 91 run, 0 NOT-MEASURED, 0 UNRUN. `check:api-surface` and
`check:export-origins` went red on the new export as expected and were
regenerated (`gen:api-surface`, `gen:export-origins`): two added names,
nothing removed.
### Reverse verification (fix committed first;
`scripts/ablation-replace.mjs`, which restores to HEAD and checks `git
diff HEAD` is empty)
- Deleted the lint line `virtualFieldAggregateRemedy(aggregate,
fieldType) ??` (anchor 1 → 0, blob e0f8418 → cd6b974fa3a7). Two
lint cases went red with Received "Either point "sum" at a field of an
accepted type, or aggregate "measured" with one its `formula` type
accepts: count, count_distinct." The explain-equality case stayed green,
as expected: that text does not depend on the line. Restored: blob ==
HEAD.
- Deleted the compile-leg line `if (virtual !== undefined) return
virtual;` (blob 170605d → 68caccc65865). The new compile case went
red with the old "For a non-numeric field, `count` accepts every type …"
remedy. Restored: blob == HEAD.
- Both subjects are imported from `src/` by relative path, so the
ablations needed no `dist/` rebuild.
## Acceptance notes
- **Same family, other doors, not changed here (outside the claim's file
surface).** For a formula, these still name a count as the way out: the
analytics door's cube-query refusal
(`service-analytics/src/cube-measure-field-type-door.ts`, "… aggregate a
field of one of those types, or count the rows with count.", `400
INVALID_FIELD`) and the engine's `aggregate` door
(`objectql/src/aggregate-field-type-door.ts`, the same clause). Both can
read `virtualFieldAggregateRemedy` the same way. Reported for the seat
to place; not measured over HTTP in this run.
- **The divergence sentence is still worded for a stored column.** For a
formula, the compile leg's divergence sentence still says "derives a
NUMBER from the stored values, so over a `formula` column …". The lint
message says the same thing ("the number would depend on the SQL
dialect"). For a virtual field the real reason is that no column exists.
The wording is inaccurate but not misdirecting, since the remedy line
now names the fix. Noted only.
- **`count_distinct` × `formula`.** The table accepts it, and the shared
sentence names it as "a different question". Whether every backend can
actually lower a `count_distinct` over a column that is never emitted is
not measured here.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 3ae0b77 commit 4ad3ce5
10 files changed
Lines changed: 221 additions & 7 deletions
File tree
- .changeset
- packages
- lint/src
- services/service-analytics/src
- __tests__
- spec
- api-surface
- export-origins
- src/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1890 | 1890 | | |
1891 | 1891 | | |
1892 | 1892 | | |
1893 | | - | |
1894 | | - | |
| 1893 | + | |
| 1894 | + | |
| 1895 | + | |
| 1896 | + | |
| 1897 | + | |
| 1898 | + | |
| 1899 | + | |
| 1900 | + | |
| 1901 | + | |
1895 | 1902 | | |
1896 | 1903 | | |
1897 | 1904 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
432 | 433 | | |
433 | 434 | | |
434 | 435 | | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
435 | 484 | | |
436 | 485 | | |
437 | 486 | | |
| |||
1271 | 1320 | | |
1272 | 1321 | | |
1273 | 1322 | | |
1274 | | - | |
| 1323 | + | |
| 1324 | + | |
1275 | 1325 | | |
1276 | 1326 | | |
1277 | 1327 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
199 | 199 | | |
200 | 200 | | |
201 | 201 | | |
| 202 | + | |
202 | 203 | | |
203 | 204 | | |
204 | 205 | | |
| |||
417 | 418 | | |
418 | 419 | | |
419 | 420 | | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
420 | 427 | | |
421 | 428 | | |
422 | 429 | | |
| |||
441 | 448 | | |
442 | 449 | | |
443 | 450 | | |
| 451 | + | |
444 | 452 | | |
445 | 453 | | |
446 | 454 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| 60 | + | |
60 | 61 | | |
61 | 62 | | |
62 | 63 | | |
| |||
242 | 243 | | |
243 | 244 | | |
244 | 245 | | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
245 | 268 | | |
246 | 269 | | |
247 | 270 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
271 | 272 | | |
272 | 273 | | |
273 | 274 | | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
274 | 279 | | |
275 | | - | |
276 | | - | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
277 | 284 | | |
278 | 285 | | |
279 | 286 | | |
| |||
288 | 295 | | |
289 | 296 | | |
290 | 297 | | |
| 298 | + | |
291 | 299 | | |
292 | 300 | | |
293 | 301 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
804 | 804 | | |
805 | 805 | | |
806 | 806 | | |
| 807 | + | |
807 | 808 | | |
808 | 809 | | |
809 | 810 | | |
| |||
994 | 995 | | |
995 | 996 | | |
996 | 997 | | |
997 | | - | |
| 998 | + | |
| 999 | + | |
998 | 1000 | | |
999 | 1001 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
790 | 790 | | |
791 | 791 | | |
792 | 792 | | |
| 793 | + | |
793 | 794 | | |
794 | 795 | | |
795 | 796 | | |
| |||
980 | 981 | | |
981 | 982 | | |
982 | 983 | | |
983 | | - | |
| 984 | + | |
| 985 | + | |
984 | 986 | | |
985 | 987 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
34 | 35 | | |
35 | 36 | | |
| 37 | + | |
36 | 38 | | |
| 39 | + | |
37 | 40 | | |
38 | 41 | | |
39 | 42 | | |
| |||
244 | 247 | | |
245 | 248 | | |
246 | 249 | | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
0 commit comments