fix(service-analytics): a draft-preview min/max answers the operand's own type instead of 0

[os-warren]

Fixes #16203

The draft-preview evaluator assumed every measure is numeric and every dimension is a string. Both halves are produced by evaluateAnalyticsQueryOverRows (packages/services/service-analytics/src/preview-evaluator.ts) before any descriptor pass runs, so no enrichment could reach either — and the severe half is a wrong value, not a descriptor gap.

Driven, not read — the two-instance harness

One dataset, one row set, two AnalyticsService instances differing in exactly one config key (draftRowsResolver), so a difference between the two responses is a difference the preview evaluator caused. The LIVE half is not a model of an engine: it is NativeSQLStrategy's generated SQL executed on a real SQLite (sql.js) whose table is seeded from the same rows the resolver hands the preview, with date stored as TEXT (this platform's canonical storage form, ADR-0053 D-B).

Measured on main @ 159dbad23, before the fix (12 pins, 7 red / 5 green):

measure (dataset)	live	preview BEFORE	preview AFTER
latest_spend — max over a date	'2026-05-12'	0	'2026-05-12'
earliest_spend — min over a date	'2026-04-21'	0	'2026-04-21'
first_payer — min over text	'ann'	0	'ann'
last_payer — max over text	'bob'	0	'bob'
min/max over a group whose operand is null throughout	null	0	null
max over a BSON Date (mongo storage form)	the instant	1785110400000 (epoch leak)	the Date
distinct_payers — count_distinct over text	2	3 (a row count)	2
latest_per_expense — derived ratio over latest_spend	null	0	null
fields[].type of a date dimension column	'time'	'string'	'time'
min_amount / max_amount over a number	500 / 1200	500 / 1200	500 / 1200
expense_count, total_amount, avg_amount	unchanged	unchanged	unchanged
sum_spent_on / avg_spent_on — the temporal pair	a different number	0	0 (untouched)

Per aggregate — the whole closed vocabulary, every member answered

AggregationFunction (packages/spec/src/data/query.zod.ts) is CLOSED, so this has a finite answer; the sibling ruling for the DESCRIPTOR half of the same question (measure-result-type.ts, #15768 / #16101) answers it identically:

aggregate	verdict	what changed
count	a row count — numeric whatever it counted	nothing
count_distinct	a cardinality — numeric	the arm was UNREACHABLE: the switch spelled it countDistinct, a word no producer mints (dataset-compiler copies the spec's count_distinct through), so it fell to the numeric default and answered a row count under the author's measure name. Now spelled as the spec spells it. ⛔ Still numeric — typing it otherwise would be a new bug.
sum	numeric arithmetic	nothing
avg	numeric arithmetic	nothing
min	a value of the operand's own type	the fix
max	a value of the operand's own type	the fix

min/max now return the winning operand itself, ordered by this file's shared compare — so an ISO date orders as a date, a BSON Date orders as its instant against wire text, and text orders the way MIN(text_col) orders on a SQL face — with a numeric arm (numericOperand) so a numeric column authored as text ('800') still orders numerically instead of lexicographically. cross-object-rebucket.ts settled the identical question for the recombination path (#3797) the same way: the value these two pick is a value OF the column, so it has to come back in the shape the row carried; coercing it to a number is what produced the epoch leak in the table above.

A group whose operand is null throughout now answers null rather than 0, which is emptyGroupValueFor's ruling in @objectstack/spec/data — "averaging, minimising or maximising no rows is undefined … and must stay null rather than be flattened to a zero that reads as a real measurement".

The ordering arm is deliberately NOT folded into compare itself: that primitive also decides where filtering and order, whose comparand comes from the query rather than from a sibling row, so widening it would move populations this card never measured.

⛔ What is deliberately left unchanged

• sum / avg over a TEMPORAL operand. There is no defined answer, and the two faces do not agree on one: the preview drops every non-finite operand and answers the identity 0, while SQLite applies numeric affinity to the TEXT column and sums the leading years. Neither number is a date and no layer refuses the pair — which is No layer refuses an incoherent aggregate / field-type pair — a dataset measure avg over a datetime works on SQLite and errors on Postgres #16099's subject, an open needs-user-decision card, not this one's to pre-empt. The pin below asserts today's preview value and only that the two faces disagree, so it is a statement about the missing refusal rather than a pin on a dialect. It passed before this change and passes after: the pair is measurably untouched.
• Derived measures. computeDerived still coerces with Number() and answers null for a non-finite operand — this change does not reach that function, it changes what the function READS. A derived ratio over a temporal min/max now sees a date instead of the spurious 0, so it answers null on the preview path exactly as it already did on the live one. Measured both ways in the table above.
• A MEASURE column's fields[].type. It stays the 'number' every producer in the platform mints, live faces included. Correcting it is measureResultType's one rule applied at the ADR-0021 descriptor pass (A dataset measure over a datetime is typed number in the analytics response, and a metric tile ignores the format style it is handed — measured on 17.3.0 #15768 / fix(analytics): a dataset measure's result type stops contradicting its own value — min/max over a temporal field is time, not number #16101); a second copy here would be two implementations free to drift, over a question this producer cannot answer anyway (it holds the cube, not the source object's declared field types). ⛔ The descriptor is not made to lie and is not withheld: after the enrichment reaches this path, latest_spend is described as type: 'time' and the value beside it is now the date.

The pins, and the population each covers

New file: packages/services/service-analytics/src/__tests__/preview-aggregate-operand-type.test.ts (12 cases).

• min/max over temporal — one date column, two groups, both bounds (min and max), asserted against the literal AND against the live path.
• min/max over text — the text / select / lookup population, which is A min/max over a text / select / lookup field is still described as type: "number" in the analytics response #16098's population on the live path and arrives here through the same defect.
• min/max over numbers — the control that must NOT move: same numbers, still typeof 'number'. Green before and after.
• min/max over an all-null group — the emptyGroupValueFor boundary, driven through the evaluator directly.
• max over a BSON Date — Field.datetime's storage form on driver-mongodb (ADR-0053 D-E2), the population compare's Date arm exists for.
• count / count_distinct / sum / avg — the rest of the vocabulary, each against the live path over the same rows.
• sum/avg over temporal — the deliberately-unchanged pair.
• derived — one ratio over numeric operands, one over a temporal max.
• fields[].type — a date dimension, a string dimension, and a measure column, each compared with the live producer's answer.

Reverse verification — direction predicted before running

Restoring preview-evaluator.ts to 159dbad23 and keeping the pins must go RED on exactly the seven cases above and leave the five controls green. Ordinary direction, no inversion: the change narrows no rule and removes no limb that another gate reads, so nothing downstream can gain a finding from it.

Measured, committed first (4df5628e0), mutation proven on disk by blob delta (2edec665… HEAD vs cbe2c62d… mutated) plus anchored counts (extremumOf 0, Math.min 1), restored under trap … EXIT INT TERM against an absolute path, restore proven by blob equality with HEAD and an empty git diff HEAD:

Tests  7 failed | 5 passed (12)

No rebuild leg is owed and here is why: the pins import ../preview-evaluator.js and ../analytics-service.js — relative, same-package specifiers vitest compiles from src/, so the mutated bytes are in the module graph directly and nothing in this ablation resolves through a package dist/. A green ablation would have been the danger; this one is red on the predicted seven.

Verification

All at head 00e99d9d7, each exit code captured immediately after a single redirected command, never through a pipe.

• pnpm --filter @objectstack/service-analytics test — 95 files / 2050 tests passed, exit 0.
• pnpm --filter @objectstack/service-analytics typecheck — exit 0. tsc --noEmit --listFiles confirms both edited files are in the program (1 hit each), so the green covers them rather than skipping them.
• Gate family derived mechanically (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack), re-derived on the final head — identical 55-family list; 55 runnable families plus the 4 artifact-roster gates whose roster sits under a path this change touches = 59 commands, all exit 0. Two needed their prerequisite built first (check:dual-build-cjs-loads, check:type-check-debt); the full closure was built (turbo run build --filter='./packages/*' --filter='./packages/*/*', 71/71) and both then answered 0 — check:type-check-debt re-measured 12 ledger entries, none above its recorded number.
• eslint --no-inline-config — the whole repo, not a narrowing: 6195 files, 0 errors, 0 warnings, exit 0.

Out-of-scope findings, filed rather than absorbed

Both were measured on the same two-instance harness, both live in this file, and neither is this card's defect class (this one is about the operand's TYPE; both of those are about NULLs):

• The draft-preview evaluator's count over a FIELD counts rows, nulls included — live counts the non-null values, so the same measure answers 2 there and 1 on the engine #16218 — count over a FIELD counts rows including nulls on the preview path (2), while every SQL face lowers COUNT("payer") and answers 1. The identical defect was measured and cleared on driver-mongodb by the AGGREGATION_CASES enrolment.
• The draft-preview evaluator answers 0 for an avg over a group with no numeric values — emptyGroupValueFor rules that null, and the live path answers null #16219 — avg over a group with no numeric values answers 0 on the preview path while the live path answers null, contradicting emptyGroupValueFor's explicit ruling. The min/max half of that same : 0 idiom moved with this card because min/max are its subject; avg is not, and moving it is a value change that deserves its own review.

Collision fence

analytics-service.ts (#16192), text-match-sql.ts / like-pattern.ts (#16028) and measure-result-type.ts (#16098) are untouched — the diff is preview-evaluator.ts, one new test file and one changeset. #16192 is not addressed here and #16098 remains open.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 6 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/ai/natural-language-queries.mdx (via count_distinct (literal, a string literal in aggregate))
• content/docs/data-modeling/queries.mdx (via count_distinct (literal, a string literal in aggregate))
• content/docs/kernel/contracts/data-engine.mdx (via count_distinct (literal, a string literal in aggregate))
• content/docs/protocol/objectql/query-syntax.mdx (via count_distinct (literal, a string literal in aggregate))
• content/docs/ui/dashboards.mdx (via count_distinct (literal, a string literal in aggregate))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v15.mdx (via count_distinct (literal, a string literal in aggregate))
• content/docs/releases/v17.mdx (via count_distinct (literal, a string literal in aggregate))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 1 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1ebde49536b431dc891b4bbe1a144b638c6205ae → packageMentionDocs.

Which tree this was computed on

This run read content/docs from cb886b5dd986b6c74621078a79c9b27f453486f5 — the merge of head 00e99d9d7561f8c744d0c098f3bc3de0b26aa340 into base 1ebde49536b431dc891b4bbe1a144b638c6205ae, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cb886b5dd986b6c74621078a79c9b27f453486f5 && git checkout cb886b5dd986b6c74621078a79c9b27f453486f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1ebde49536b431dc891b4bbe1a144b638c6205ae 00e99d9d7561f8c744d0c098f3bc3de0b26aa340 && git checkout -B drift-repro 1ebde49536b431dc891b4bbe1a144b638c6205ae && git merge --no-ff 00e99d9d7561f8c744d0c098f3bc3de0b26aa340

node scripts/docs-audit/affected-docs.mjs --json 1ebde49536b431dc891b4bbe1a144b638c6205ae

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 1ebde49536b431dc891b4bbe1a144b638c6205ae → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-warren]

PM verification — PASS

Measured at head 00e99d9d7, independently of the report. Everything below is a reading I took myself, not a re-statement of the body.

Fence — held, by a LOCAL merge-base diff

⚠️ Not GitHub's file list: this branch's base is a merge-base, so GitHub can compare against a stale one. git merge-base = 159dbad23 — the same commit the body says it measured on.

.changeset/analytics-preview-min-max-operand-type.md      |  25 ++
.../__tests__/preview-aggregate-operand-type.test.ts      | 332 +++++++++++++++
packages/services/service-analytics/src/preview-evaluator.ts | 133 ++++++++-
3 files changed, 480 insertions(+), 10 deletions(-)

analytics-service.ts (#16192), text-match-sql.ts / like-pattern.ts (#16028), measure-result-type.ts (#16098) — untouched. Three concurrent PRs in this package, no overlap.

The unreachable arm — confirmed, and confirmed unreachable from any validated cube

The pre-fix blob (cbe2c62df) spelled it countDistinct at three sites (a doc line, and the two live arms at :159 / :166). The question that matters is not whether the word was there but whether anything could ever reach it, and that has a closed answer:

• AggregationFunction (spec/data/query.zod.ts:159) = count · sum · avg · min · max · count_distinct — 6 members, no countDistinct.
• AggregationMetricType (spec/data/analytics.zod.ts:23) = those 6 plus number / string / boolean (custom-SQL) — 9 members, still no countDistinct.
• dataset-compiler.ts:180 — return m.aggregate as Metric['type'], a verbatim pass-through of the first vocabulary into metric.type, which is what the switch reads.

So the arm was unreachable from the dataset path and from a hand-authored cube (zod rejects the spelling in both enums). It was dead, and the measure fell to the numeric default and answered a row count under the author's count_distinct name. ⭐ Enumerating the closed vocabulary is what found it — sampling could not have. That is exactly why the brief required enumeration, and it is the second time this session that walking a closed set surfaced a defect nobody had filed.

Also checked the arm that moved out from under count: the old nested if (metricType === 'countDistinct') sat inside count || field === '*' and fell through to rows.length; the new code returns rows.length directly. Same answer for a '*' operand — no regression hidden in the un-nesting.

The emptyGroupValueFor ruling — read, not cited

spec/data/aggregation-policy.ts:52 returns 0 for count / count_distinct / sum and undefined for everything else — "averaging, minimising or maximising no rows is undefined … and must stay null rather than be flattened to a zero that reads as a real measurement." The min/max → null change is that ruling applied. And it is why #16219 is a real finding, not a nit: avg over a group with no numeric values still answers 0 in this file, against the same function's explicit word. Correctly filed rather than absorbed — min/max are this card's subject, avg is a value change that deserves its own review.

The dimension type — the same expression, not a parallel one

native-sql-strategy.ts:1157 and objectql-strategy.ts:1772 both do fields.push({ name: dim, type: d?.type || 'string' }). The fix mints String(cube.dimensions?.[d]?.type || 'string') — the same expression against the same source, so a date dimension is 'time' on both faces rather than two implementations free to drift.

What was deliberately NOT changed — verified measurably unchanged, not merely asserted

• A MEASURE column keeps type: 'number'. ⭐ This is the constraint I set on dispatch: the descriptor must be neither made to lie nor withheld. It is neither — the value beside it is now the date, and correcting the descriptor stays measureResultType's one rule at the ADR-0021 pass (A dataset measure over a datetime is typed number in the analytics response, and a metric tile ignores the format style it is handed — measured on 17.3.0 #15768 / fix(analytics): a dataset measure's result type stops contradicting its own value — min/max over a temporal field is time, not number #16101). A second copy here would be a drift surface over a question this producer cannot answer (it holds the cube, not the source object's field types).
• sum/avg over a temporal operand. The pin (test:280) asserts today's preview value and only that the two faces disagree — expect(live).not.toBe(preview) — never SQLite's figure. That is a statement about the missing refusal, which is No layer refuses an incoherent aggregate / field-type pair — a dataset measure avg over a datetime works on SQLite and errors on Postgres #16099's open subject, not a pin on a dialect. Green before and after.

Scope note on the body

The per-aggregate table cites AggregationFunction (6 members); the switch actually reads AggregationMetricType (9). Not a gap — the compiler pass-through above means only the 6 reach this path, and the in-source comment does answer the other three (number/string/boolean left on the historical numeric default, as custom-SQL metrics the dataset path never mints). Recording it so a reviewer doesn't read the 6-row table as the whole surface.

Contained blast radius

aggregate() is module-private with a single call site (:339, row[m] = …), so widening its return from number to unknown cannot reach a caller outside this file. Only analytics-service.ts:57 imports this module, and it imports evaluateAnalyticsQueryOverRows, whose signature is unchanged.

Harness

12 cases, and the live half is not a model: sql.js initialised for real, seeded from the same ROWS the resolver hands the preview, two AnalyticsService instances differing in exactly one config key (draftRowsResolver). A difference between the two responses is therefore a difference this evaluator caused.

Verdict: PASS. Moving to needs:contract-review — opening a PR is not submitting it, and the contract review is the bar here, not CI-green.

---

Generated by Claude Code

[os-zhuang]

Contract review (clause ②) — PASS · landing held on the pair declaration — PR #16235 at head 00e99d9d (Fixes #16203)

Reviewed by the director seat at tier (claude-fable-5-1, session session_01TezFG8ZMrNH6n5VTNpPpdH), 2026-09-06 07:52Z; taken here because the domain:services seat's tier fuse is blown.

Clause ② answer: no surface move; conformance limb yes — the preview producer now answers what the live producer answers. No exported symbol or signature moves on @objectstack/service-analytics's published .d.ts: aggregate / extremumOf / compareOperands / numericOperand are module-private, and evaluateAnalyticsQueryOverRows's declared shape is unchanged (rows were already Record<string, unknown>). What moves is the wire VALUE on one path of POST /analytics/dataset/query (preview=draft): min/max return the operand in its own type instead of 0, an all-null group answers null per emptyGroupValueFor, count_distinct answers a cardinality (the arm was unreachable — spelled countDistinct, a word no producer mints, so it fell to the numeric default), and a dimension column is typed from the cube dimension. Each is the preview path being brought onto the answer the live path already gives over the same rows — measured on a real SQLite in the test, not modelled. Defect repair, not a contract move.

Fences held, read on the diff: sum/avg over a temporal operand left exactly as it was and pinned as "the two faces disagree" (not a dialect pin) — #16099's box, not pre-empted; computeDerived untouched; a measure column's fields[].type keeps the producer's number (one rule, measureResultType, applied once downstream — correct, and now latest_spend is described time and carries the date on both paths); compare itself not widened. count's un-nesting gives the same answer for a '*' operand.

Tests read (preview-aggregate-operand-type.test.ts, 332 lines): one fixture, two services differing in draftRowsResolver, live half executed on sql.js; the whole closed AggregationFunction vocabulary answered; BSON-Date instant ordering; the all-null group through the evaluator directly; ablation 7 red / 5 green on the predicted set.

Changeset: @objectstack/service-analytics: minor. Accepted: the PR grades on the wire value moving for existing queries; the fixed group makes the level moot in effect, and minor is the conservative reading rather than a wrong one. CI at 00e99d9d: 31 success · 6 skipped · 0 failing. Governed-merge audit on the 3 paths: 0 hits.

Landing — held on one thing

--pair 16235 exits 4: card #16203's claim has no Claim: / Clause-②: spelling. Same fix as the five cards the seat already corrected this hour. needs:contract-review comes off this PR now (card never carried it). On pair exit 0 the next director pass flips ready-for-review + auto-merge (squash).

---

Generated by Claude Code