Repository navigation
Commit c565813
feat(spec,analytics): a dataset answer's measure column states its aggregate, labelled or not (fields[].aggregate) (#22021)
Fixes #21995
Clause-②: yes (widening: a new member on a published response schema;
@objectstack/spec changeset at least minor)
Dev report for this PR: session `session_01GV6oYwgc1kWiUCb1YaprQ7` (PM
loop round 2, `domain:spec` seat 2), branch
`claude/issue-21995-measure-column-aggregate`, base `1bc6ca1d`.
## What this changes
A dataset answer's measure column now states its aggregate, whether or
not the author labelled the measure:
- **Spec** (`packages/spec/src/api/analytics.zod.ts`): one new closed
member on `AnalyticsResultResponseSchema.data.fields[]`, `aggregate:
AggregationFunction.optional()`. The `AnalyticsResult` contract
(`packages/spec/src/contracts/analytics-service.ts`) gains the same
member, so the compile-time binding between the two
(`AnalyticsResultMatchesContract`) holds.
- **Producer**
(`packages/services/service-analytics/src/analytics-service.ts`,
`enrichResultColumns`): one line beside the `builtinAggregate` line,
read off the same dataset measure. `enrichResultColumns` runs on both
the live query and the draft-data preview, so the two answers agree by
construction.
- **Where it is absent:** dimension columns; derived measures (guarded
with `!m.derived`, because the compiler ignores a stray `aggregate`
written beside `derived`); and the cube query answer (`POST
/analytics/query`), which never passes through `enrichResultColumns`.
The `.describe()` says exactly this.
- **Unchanged:** `builtinAggregate` keeps its label-only meaning, and
every existing pin on it stays green. No authoring key is added.
- **Docs:** `content/docs/api/data-api.mdx` listed "the optional members
`AnalyticsResultResponseSchema` declares". That list would be false
after this change, so it now names `aggregate` too and says how it
differs from `builtinAggregate`. See Acceptance notes: this file is
outside the claim's declared surface.
- **Changeset:** `.changeset/21995-measure-column-aggregate.md`, with
`@objectstack/spec` and `@objectstack/service-analytics` both `minor`.
It says what a renderer can now read.
What a renderer reads: on `POST /analytics/dataset/query`, a measure
column such as `{ name: 'task_count', type: 'number', label: 'Tasks',
aggregate: 'count' }`. objectstack-ai/objectui#11681 is the consumer
half: once a release carries this, it derives integer ticks from it.
## Mechanism readings (PM hypotheses H1 to H4, measured at `1bc6ca1d`)
- **H1, holds with one refinement.** `enrichResultColumns` is the only
writer of descriptor keys on a dataset answer's `fields[]`. It has two
call sites: the draft preview (`analytics-service.ts:2504`) and the live
return (`:2761`). The column ENTRIES themselves are minted upstream as
`{ name, type: 'number' }` by the strategies and by `DatasetExecutor`
(`dataset-executor.ts:1166` for `__compare`, `:1205` for derived,
`:1333`). The degraded "backing object unavailable" exit returns
`fields: []`, so it has no column to describe. `queryDataset` has a
single implementation repo-wide. The measure's `aggregate` is in hand at
the `builtinAggregate` line (`:2866`).
- **H2, the schema is shared, and the describe is truthful without a
second producer change.** `AnalyticsResultResponseSchema` is the
response schema of `POST /analytics/query`
(`plugin-rest-api.zod.ts:1324`), and through the `AnalyticsResult`
binding it is also the dataset answer's shape. The cube door writes
`fields[]` through the strategies, `withDeclaredMeasureFormats` and
`withMeasureResultTypes`, and none of them writes `builtinAggregate` or
the new member. So the describe states that the member is absent on a
cube query answer. A pin calls `AnalyticsService.query()` and asserts
that absence.
- **H3, holds, with one edge.** A derived measure's column is minted at
`dataset-executor.ts:1205` and has no aggregate. The edge:
`DatasetSchema` accepts `aggregate` beside `derived`, and the compiler
ignores it (`dataset-compiler.ts:704`). So the new member checks
`!m.derived` rather than relying on `aggregate` being absent, and a pin
covers that case.
- **H4, holds.** The REST route ends `res.json(result)`
(`rest-server.ts:11319`). No REST source change is needed.
`packages/rest/src/analytics-routes.test.ts` passes (14 tests).
## Tests
All on HEAD `908f4f02`, or on `91e05bd9` where noted. The commit between
them touches only the `.mdx` and the changeset.
-
`packages/services/service-analytics/src/__tests__/measure-column-aggregate.test.ts`
(new, 7 tests, each run on BOTH the live and preview paths):
- a labelled `count` measure states `count`, and an unlabelled one still
carries `builtinAggregate`;
- a `sum` over a currency field states `sum`;
- the preview path states the same as the live path, column for column;
- absent on a dimension column and on a derived measure;
- absent on a derived measure that also declares a stray `aggregate`;
- a `__compare` column states its measure's aggregate;
- absent on a cube `query()` answer.
- `preview-column-enrichment.test.ts`: `aggregate` was added to its
key-for-key live/preview parity descriptor.
- `packages/spec/src/api/analytics.test.ts`: the member parses beside a
`label`. Off-enum `total` is refused at `data.fields.0.aggregate` with
issue code `invalid_value`.
- `packages/spec/src/contracts/analytics-service.test.ts`: the member
types on a labelled column. Off-enum is a compile error
(`@ts-expect-error`, compiled by `check:test-typecheck`).
- Readings:
- spec, 2 files: 42 passed (at `91e05bd9`);
- service-analytics full suite: 178 files, 4410 passed, 262 skipped;
- rest `analytics-routes.test.ts`: 14 passed;
- `typecheck` for `@objectstack/spec` (tsc, scripts-typecheck,
test-typecheck) and for `@objectstack/service-analytics`: exit 0.
service-analytics resolves `@objectstack/spec` types from `dist/`, so
its compiling `f.aggregate` shows it read the rebuilt `.d.ts`.
**Reverse verification.** The direction was predicted before running.
The fix was committed first. The mutation went through
`scripts/ablation-replace.mjs`: the anchor hit 1 time and then 0, the
blob changed, and the restore was proven by blob equal to HEAD and an
empty `git diff HEAD`. The subject is imported from `src/` (relative
import), so no dist leg applies.
1. Delete the producer line. Predicted: positive `aggregate` assertions
go red, absences and every `builtinAggregate` assertion stay green.
Observed: 4 failed and 47 passed across the 3 files. The red ones were
labelled-count, sum, stray-aggregate (its control leg) and `__compare`.
2. Delete only the `!m.derived` guard. Predicted: only the
stray-aggregate case goes red. Observed: 1 failed and 50 passed (`live:
expected 'sum' to be undefined`).
## Gates
These were derived by `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` on HEAD `908f4f02`, from a 9-path
change set measured against merge base `1bc6ca1d`.
- **109 commands derived, 108 run, 0 unrun.** `--ran` reconciliation
exits 0.
- **107 exited 0 on the first pass.**
- `check:api-surface`: "public API surface + factory signatures
unchanged".
- `check:docs`: "226 generated files in sync".
- `check:authorable-surface`: green.
- `check-adr-0087-registration`: no declared-breaking changeset.
- `check-changeset-no-major`: no major.
- `check:nul-bytes`, `check:docs-spec-enumerations`,
`check:doc-authoring`, `check:cross-package-test-inputs`,
`check:test-source-alias`: OK.
- **`pnpm --filter @objectstack/spec run check:skill-examples`** first
exited 3 (PREREQUISITE NOT MET, no `client-react` dist). After a turbo
build of `@objectstack/client-react` it was re-run and exited 0 ("262
prose examples type-check").
- **NOT MEASURED: `pnpm check:dual-build-cjs-loads`** (exit 3). Reason:
it reads every package's built output, and 44 packages had no `dist/`
here. A whole-repo build is CI's.
- **Run beyond the derived set**, from the PM's lead list (roster
families): `check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`, and spec `check:error-code-provenance`. All
exited 0.
- **`pnpm --filter @objectstack/spec check:generated`:** all 15
artifacts up to date. The `fields` row in
`content/docs/references/api/analytics.mdx` is truncated, so no
generated artifact moves.
- **ESLint, a declared narrowing.**
- Command: `eslint --no-inline-config --format json` over the 7 touched
`.ts` files.
- Result: 7 files reported, 0 errors, 0 warnings. No "file ignored"
warning, so all 7 are inside the config's linted population.
- Why the narrowing excludes nothing: `eslint.config.mjs` enables no
type-aware linting (no `parserOptions.project`, no `projectService`), so
this diff cannot move a verdict on any untouched file.
- The `.md` and `.mdx` files are outside ESLint's `files` globs.
- Repo-wide `pnpm lint` is CI's.
## Acceptance notes
- **File surface.** `content/docs/api/data-api.mdx` is not on the
claim's declared surface. It was edited because its sentence naming "the
optional members `AnalyticsResultResponseSchema` declares" becomes false
with this change. The claim's file list wants that path added.
- **`builtinAggregate` on a derived measure with a stray `aggregate`**
(noted, not filed).
- The producer writes `builtinAggregate` with the stray value on such a
column when it is unlabelled. `builtinAggregate`'s own describe says it
is absent on derived columns.
- `DatasetSchema` accepts `aggregate` beside `derived` at the REST
door's parse, and the compiler then ignores it.
- No real producer writes that shape: zero co-declarations in
`examples/**` and in non-test `packages/**` sources.
- Refusing `aggregate` beside `derived` at the schema would close both
this and the `!m.derived` guard's reason to exist. That is a contract
tightening, outside this card.
- **The cube door does not state `aggregate`.** It is declared absent
there. A cube measure's `type` is its aggregate, so stating it there
would be one write beside `withMeasureResultTypes`. No measured consumer
pulls it; the dashboards on this card read dataset answers.
- **Branch is behind `origin/main`.** It is 2 commits behind
(`1fb274e6`, touching rest/runtime, metadata-protocol, `.claude`).
Neither touches a path here, so `main` was not merged.
- **Contract review.** The `Clause-②` contract review is owed, as the
claim records.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01GV6oYwgc1kWiUCb1YaprQ7)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8caa131 commit c565813
9 files changed
Lines changed: 345 additions & 7 deletions
File tree
- .changeset
- content/docs/api
- packages
- services/service-analytics/src
- __tests__
- spec/src
- api
- contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
461 | 461 | | |
462 | 462 | | |
463 | 463 | | |
464 | | - | |
| 464 | + | |
465 | 465 | | |
466 | 466 | | |
467 | 467 | | |
| |||
485 | 485 | | |
486 | 486 | | |
487 | 487 | | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
488 | 495 | | |
489 | 496 | | |
490 | 497 | | |
| |||
Lines changed: 225 additions & 0 deletions
| 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 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 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 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
51 | 52 | | |
52 | 53 | | |
53 | 54 | | |
| |||
238 | 239 | | |
239 | 240 | | |
240 | 241 | | |
241 | | - | |
| 242 | + | |
242 | 243 | | |
243 | 244 | | |
244 | | - | |
| 245 | + | |
245 | 246 | | |
246 | 247 | | |
247 | 248 | | |
| |||
Lines changed: 14 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2492 | 2492 | | |
2493 | 2493 | | |
2494 | 2494 | | |
2495 | | - | |
2496 | | - | |
| 2495 | + | |
| 2496 | + | |
2497 | 2497 | | |
2498 | 2498 | | |
2499 | 2499 | | |
| |||
2777 | 2777 | | |
2778 | 2778 | | |
2779 | 2779 | | |
2780 | | - | |
2781 | | - | |
| 2780 | + | |
| 2781 | + | |
2782 | 2782 | | |
2783 | 2783 | | |
2784 | 2784 | | |
| |||
2864 | 2864 | | |
2865 | 2865 | | |
2866 | 2866 | | |
| 2867 | + | |
| 2868 | + | |
| 2869 | + | |
| 2870 | + | |
| 2871 | + | |
| 2872 | + | |
| 2873 | + | |
| 2874 | + | |
| 2875 | + | |
| 2876 | + | |
2867 | 2877 | | |
2868 | 2878 | | |
2869 | 2879 | | |
| |||
0 commit comments