fix(service-analytics): compile a measure's field and filter, on both doors - #10411
Conversation
… doors
A dataset measure declares `aggregate`, `field` and `filter`; the compiled SQL
used only `aggregate`.
1. `{ aggregate: 'count', field: 'x' }` emitted `COUNT(*)`. The wrapper table
took the resolved column and discarded it, so a measure asking how many rows
carry a value counted every row it was handed.
2. `/api/v1/analytics/query` dropped every per-measure `filter`, and the
dataset's definition-level `filter` with it. That door addresses the
registered Cube directly; both filters live beside the cube in the dataset
registry, and only `DatasetExecutor` — the dashboard's door — ever read them.
One cube and one set of measure names answered two different numbers
depending on which door the caller came in.
`count` now takes its column (`*` still counts rows — it is the compiler's "no
field declared" spelling), and a new conditional-aggregate table lowers a
measure filter to a portable `CASE WHEN` (not `FILTER (WHERE …)`, which MySQL
lacks). The dataset scope reaches the strategy through `getDatasetScope` on the
context the analytics package builds for its own strategies — same shape and
same registry as the neighbouring `getAllowedRelationships`, and no spec edit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
… filters do not name Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 1 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 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 f4e64efd71c2b643c93f9bb87a1001b3ab5edffa && git checkout f4e64efd71c2b643c93f9bb87a1001b3ab5edffa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e502a6a8ebafaee434a14481a45494a1dd4958c9 69143f891cfc53ae5dca740e3f3673ae51538d64 && git checkout -B drift-repro e502a6a8ebafaee434a14481a45494a1dd4958c9 && git merge --no-ff 69143f891cfc53ae5dca740e3f3673ae51538d64
node scripts/docs-audit/affected-docs.mjs --json e502a6a8ebafaee434a14481a45494a1dd4958c9
|
Fixes #10298
A dataset measure declares three things —
aggregate,fieldandfilter— and the compiled SQL used only the first. Both defects the card measured are that one gap, and this repairs it in the strategy that produced the statements the card quoted.The premise, re-established on this branch before anything changed
Both defects reproduce byte-for-byte against
origin/main(8012960), compiling the card's own cubes throughAnalyticsService.generateSql:That is the card's reported SQL, reproduced.
premise_still_valid: true.The fork triage named — not reached
Triage ruled that if per-measure
filterturned out to be genuinely unsupported on the strict wrapper, the 400-naming-the-measure variant would be a contract change and forks to a decision. That determination was made before writing the fix, and it lands on the other side: the filter is supportable here with no change to what the endpoint accepts or rejects. The endpoint's accept/reject set is untouched; only the arithmetic moves, from "what the compiler happened to emit" to "what the author declared".Clause-②: no.Two facts settled it.
MetricSchemainpackages/spechas declaredfilterson a metric since #4001 — the cube model has always had room for per-metric filtering, it simply had no consumer. And the compiled dataset already carries both filters in the registry beside its Cube; nothing was missing but the wire between them.The fix
countcompiles its column.AGGREGATE_SQL['count']took the resolved column and discarded it. It now wraps that column —COUNT(resolved_by_article)— when the measure names a field, and keepsCOUNT(*)when it does not:sql: '*'is the compiler's own "no field declared" spelling, so the star must go on counting rows.The dataset's filters reach the strict door.
compileDatasetsplits a dataset into the half a Cube can express and the half it cannot: the definition-levelfilter, and each measure's own scopedfilter. OnlyDatasetExecutor— the dashboard's door — ever read the second half.POST /api/v1/analytics/queryaddresses the registered Cube directly and never touches the executor, so it answered unfiltered aggregates under the author's measure names.That half now reaches the strategy through
getDatasetScope(cubeName)on the context the analytics package builds for its own strategies — the same shape, the same registry and the same "cannot answer, do not block" tiering as thegetAllowedRelationshipshook directly above it. A per-measure filter becomes a conditional aggregate rather than aWHEREconjunct, because one statement carries several measures and aWHEREwould narrow all of them; the definition-level filter becomes a plain conjunct, because narrowing the whole statement is exactly what it means.CASE WHEN, notFILTER (WHERE …):FILTERis Postgres and SQLite ≥ 3.30 only — MySQL has never had it — and this strategy hand-compiles one statement for whichever SQL driver owns the object. A portable conditional aggregate is the only form that cannot answer a syntax error on one supported driver and a number on another.No
packages/specedit. The hook is declared inservice-analytics's ownstrategies/types.tsas an extension of the spec'sStrategyContext. Nothing about it is an authorable surface — no metadata key, no wire shape, no error code — so widening the published contract would have bought nothing.What the SQL looks like now
Comparands are bound, in the order their placeholders appear: the SELECT list precedes the WHERE clause and
$nis positional, so each measure filter is compiled inside the SELECT loop. A filter compiled anywhere else would misalign every later bind — the pin asserts theparamsarray, not just the text.In scope beyond the two measured defects, and why
The card names the per-measure
filter; the definition-levelfilterwas dropped on the same door by the same mechanism, and is repaired here rather than filed. Two reasons, both load-bearing.It is the card's own acceptance criterion: "the same cube and the same measure names answer two different numbers depending on which door you come in." For any dataset that declares an intrinsic
filter, fixing only the measure filters leaves the two doors still disagreeing — so the card could not be closed without it.And it is mechanical, with its correct shape already pinned by evidence:
DatasetExecutor.runMeasurePasscombinescompiled.filteras the base filter of every pass. The sibling behaviour dictates the shape; nothing here is a judgement call.Evidence it was broken, measured the same way as the two above:
Both doors, on a real database
Shape assertions cannot tell a fix from plausible-looking SQL, so the last block runs both doors against a real SQLite (
sql.js, the pure-WASM enginedriver-sqlitself falls back to) over the card's own ground truth: 24 opportunities, 8 won, 5 lost, won revenue 1,290,000, grand total 5,632,500 — the number the broken door answered forwon_amount.The API door now answers 24 / 8 / 5 / 1,290,000, and
queryDatasetanswers identically, measure for measure, both ungrouped and grouped.The grouped leg groups by owner, never by
stage. Grouping by the very column a measure filters on makes the filtered and unfiltered aggregates coincide inside the matching group, so an assertion there passes with the filter dropped. That is not a hypothetical: the first draft of that test grouped bystage, and the ablation below is what caught it — the fix was fine, the test was asleep.Reverse verification
Predicted signature, written before the run: neutering both halves in
native-sql-strategy.ts(revertcountto the arity-zeroCOUNT(*)lambda, forcedatasetScopetoundefined) turns 8 of the 11 new tests red, the three survivors beingCOUNT(*)for a fieldless count, the manifest-cube no-op, and the fixture self-check; the lockstep pin stays green because it reads the two tables rather than the call site.Observed, exactly:
Tests 8 failed | 12 passed (20),Test Files 1 failed | 1 passed (2), with the card's own wrong numbers back in the failure text —Rebuild: none is required between the edit and the run, argued from the files. Both the ablated module and the test resolve through relative specifiers inside one package (
../analytics-service.js,../strategies/native-sql-strategy.js), so vitest transformssrc/directly;service-analyticsships no vitest config and the repo has no alias table redirecting them. The onlyexports-map resolution in the file is@objectstack/spec, which this change does not touch and which was built before the first measurement. The ablation was therefore read off the edited source in both legs.Restore is byte-identical, not merely "reverted":
and the restore leg was re-run green (
Tests 20 passed (20)) rather than assumed.The vocabulary stays in lockstep
aggregation-lockstep.test.tsgains one pin: the conditional table's keys must equal the plain table's. An aggregate added to one and not the other would not fail — it would silently drop the author's filter and answer the unfiltered number under the filtered measure's name, which is this card's defect returning through a new door.Tests
pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2→Test Files 78 passed (78),Tests 1734 passed (1734)params.Gate union
Derived with
node scripts/pm/dispatch-gates.mjs(no paths passed — it reads the change set from the merge base itself), then run after the final commit on a clean worktree, at69143f891. Nine path-matched families plus the five the test-file convention moves, andcheck:nul-bytes. Every exit code captured before any pipe; each family's own verdict line quoted:check:changeset-gate-self-tests✓ check-changeset-no-major --self-test: 116 assertions …check:objectui-changeset✓ objectui-range --self-test: all checks passedcheck:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none newcheck:test-source-aliascheck-test-source-alias OK — 72 packages with tests scannedcheck:type-source-resolutioncheck-type-source-resolution OK — 76 packages with a tsconfig.json scannedcheck-adr-0087-registration.mjs✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)check-changeset-no-major.mjs✓ This diff introduces no \major` bump.`check-empty-changeset.mjs✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)docs-audit/check-affected-docs.mjs✓ affected-docs self-test: 262 cases pass.(its remaining output is the standing route-ledger census, not a verdict about this diff)check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none newcheck:type-check-coveragecheck-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger (436 frozen raw errors), 1 exemptcheck:type-check-debt--re-measure; 436 frozen raw errors, unchanged —service-analytics's ledger entry stays at 10 and no entry was raisedcheck:engine-double-contractcheck-engine-double-contract: OK — 338 pinned, 133 in the DEBT ledger, 2 exempt.check:where-matcher✓ where-matcher conformance holds: 266 matcher(s) discovered … none newcheck:nul-bytescheck-nul-bytes: OK (scanned 6116 text file(s) … no raw ASCII control bytes)The debt gate was run against a fully built workspace closure (
turbo run build --filter=./packages/* --filter=./packages/*/*, 70/70 successful) — on an unbuilt tree it refuses, and that refusal is a precondition, not a pass.Not addressed here
Two neighbouring gaps were measured and filed as their own issues rather than widened into this PR. Neither is fixed by this branch:
/api/v1/analytics/querystill drops per-measure and dataset-levelfilteron the ObjectQL path —engine.aggregatereceives no filter at all #10413 — the same declarations are still dropped on the ObjectQL path (objectqlAggregatedrivers), whereengine.aggregatehas no per-aggregation filter to lower into. Repairing it is either a contract widening or a second fan-out, which is a decision this card does not own.MetricSchema.filtersis an authorable per-metric filter with zero consumers — a hand-authored cube'sfilters: [{ sql }]is parsed and dropped #10414 —MetricSchema.filters, the spec's own per-metric filter surface, parses and is consumed nowhere. It carries a raw SQL string, which is why this PR reached for a structured sidecar instead; whether to enforce it or retire it under ADR-0049 is apackages/speccall.Generated by Claude Code