fix(analytics): a new spec aggregate can no longer silently return a row count (#4153)

Track C item 4 of objectui#2945 — "AggregationFunction: three places in
lockstep". They agreed only by coincidence:

  AggregationFunction (spec)      8 members
  UNSUPPORTED_AGGREGATES          2 rejected at compile
  native-sql-strategy switch      6 cases + `default: COUNT(*)`

8 − 2 = 6 = the six cases, today. Add a ninth to the spec and it passes the
compiler's gate (not in UNSUPPORTED), is advertised as supported by that
gate's error message (a hand-written prose list — a third copy of the
vocabulary), reaches the switch, matches nothing, and falls to
`default: COUNT(*)`. The author asks for a median and gets a row count: no
error, no log, wrong figures on a dashboard. Same shape as #3948, in the
analytics SQL builder.

Derivation plus a guard, no behaviour change:
- the switch becomes AGGREGATE_SQL, a table whose coverage is assertable;
- the error message's prose becomes SUPPORTED_AGGREGATES, derived as
  AggregationFunction.options minus UNSUPPORTED_AGGREGATES;
- aggregation-lockstep.test.ts asserts the arithmetic in both directions.

Verified by adding a hypothetical `median` to the spec: three assertions now
fail naming it, including "these would fall through to the COUNT(*) fallback
and return a row count". The same edit was green before.

Same six aggregates, same six SQL expressions, same fallback for everything
else.

Reported not fixed (recorded in the strategy's doc comment): the fallback is
also reached by a `number`/`string`/`boolean` metric — a custom SQL
expression per AggregationMetricType — whose expression is replaced by a row
count. Datasets cannot produce one, so it needs a hand-authored Cube;
emitting `col` instead is a behavioural change deserving its own tests.

service-analytics: 420 tests / 32 files green; tsc clean in the touched files.

Refs objectstack-ai/objectui#2945, #3948

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/aggregation-vocabulary-lockstep.md

@@ -0,0 +1,56 @@
+---
+"@objectstack/service-analytics": patch
+---
+
+fix(analytics): a new spec aggregate can no longer silently return a row count
+
+Track C item 4 of objectstack-ai/objectui#2945 — *"`AggregationFunction`: three
+places in lockstep"*. They agreed only by coincidence, and the failure mode when
+they stopped agreeing was silent wrong numbers.
+
+The three:
+
+1. `AggregationFunction` (`@objectstack/spec/data`) — eight members, what an
+   author may declare as a dataset measure's `aggregate`.
+2. `UNSUPPORTED_AGGREGATES` (`dataset-compiler.ts`) — `array_agg`/`string_agg`,
+   rejected at compile time with a clear error.
+3. The aggregate `switch` in `native-sql-strategy.ts` — six cases, then
+   `default: return 'COUNT(*)'`.
+
+8 − 2 = 6 = the six cases, today. Add a ninth member to the spec — `median`,
+`percentile`, anything — and it would:
+
+- pass the compiler's gate, since it is not in `UNSUPPORTED_AGGREGATES`;
+- be **advertised as supported** by that gate's error message, which listed
+  `count, sum, avg, min, max, count_distinct` as hand-written prose — a third
+  copy of the vocabulary;
+- reach the strategy's `switch`, match no case, and fall to
+  `default: COUNT(*)`.
+
+The author asks for a median and gets a row count. No error, no log, wrong
+figures on a dashboard — the same silent-wrong-answer shape as the filter
+operators in #3948, in the analytics SQL builder.
+
+**The fix is derivation plus a guard, with no behaviour change.** The `switch`
+becomes `AGGREGATE_SQL`, a table whose coverage is assertable; the error
+message's prose list becomes `SUPPORTED_AGGREGATES`, derived as
+`AggregationFunction.options` minus `UNSUPPORTED_AGGREGATES`; and
+`aggregation-lockstep.test.ts` asserts the arithmetic — the lowered set equals
+the admitted set, every spec member is either lowered or explicitly rejected,
+nothing is both, and the rejection list names only aggregates the spec has.
+
+Verified by adding a hypothetical `median` to the spec, which now fails three
+assertions naming it, including *"these would fall through to the COUNT(*)
+fallback and return a row count"*. Before this change the same edit was green.
+
+Nothing is narrowed and no SQL changes: the same six aggregates lower to the
+same six expressions, and the `COUNT(*)` fallback still catches everything else.
+
+**Reported, not fixed:** that fallback is also reached by a measure whose `type`
+is `number`/`string`/`boolean` — a custom SQL *expression*, per
+`AggregationMetricType` — whose expression is then replaced by a row count.
+Datasets cannot produce one (`aggregateToMetricType` only ever returns an
+`AggregationFunction` member), so it is reachable only from a hand-authored
+Cube. Emitting `col` instead is a behavioural change in an analytics SQL path
+and deserves its own change with its own tests; the strategy's doc comment now
+records it.

packages/services/service-analytics/src/aggregation-lockstep.test.ts

@@ -0,0 +1,89 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * One aggregate vocabulary, three places that must agree (objectui#2945 Track C).
+ *
+ *   1. `AggregationFunction` (`@objectstack/spec/data`) — what an author may
+ *      declare as a dataset measure's `aggregate`.
+ *   2. `UNSUPPORTED_AGGREGATES` (`dataset-compiler.ts`) — the ones v1 rejects at
+ *      compile time, with a clear error.
+ *   3. `AGGREGATE_SQL` (`native-sql-strategy.ts`) — the ones it can lower to SQL.
+ *
+ * (2) and (3) are complements of each other over (1), and nothing enforced it.
+ * The strategy used to `switch` with `default: return 'COUNT(*)'`, so an
+ * aggregate added to the spec would pass the compiler's gate, be advertised as
+ * supported by its error message, and then return **a row count in place of the
+ * number the author asked for** — no error, no log, wrong analytics.
+ *
+ * These tests make that arithmetic explicit, so growing the vocabulary fails
+ * here instead of shipping a silently wrong figure.
+ */
+import { describe, it, expect } from 'vitest';
+import { AggregationFunction } from '@objectstack/spec/data';
+import { UNSUPPORTED_AGGREGATES, SUPPORTED_AGGREGATES } from './dataset-compiler.js';
+import { SUPPORTED_AGGREGATE_SQL_KEYS } from './strategies/native-sql-strategy.js';
+
+describe('aggregate vocabulary lockstep', () => {
+  it('the strategy lowers exactly the aggregates the compiler admits', () => {
+    expect([...SUPPORTED_AGGREGATE_SQL_KEYS].sort()).toEqual([...SUPPORTED_AGGREGATES].sort());
+  });
+
+  it('every spec aggregate is either lowered or explicitly rejected', () => {
+    const lowered = new Set(SUPPORTED_AGGREGATE_SQL_KEYS);
+    const unhandled = AggregationFunction.options.filter(
+      (a: string) => !lowered.has(a) && !UNSUPPORTED_AGGREGATES.has(a),
+    );
+    expect(
+      unhandled,
+      'these would fall through to the COUNT(*) fallback and return a row count',
+    ).toEqual([]);
+  });
+
+  it('nothing is both rejected and lowered', () => {
+    const both = SUPPORTED_AGGREGATE_SQL_KEYS.filter((a) => UNSUPPORTED_AGGREGATES.has(a));
+    expect(both).toEqual([]);
+  });
+
+  it('the rejection list names only aggregates the spec actually has', () => {
+    // A stale entry here silently *widens* what v1 claims to support: the name
+    // is subtracted from SUPPORTED_AGGREGATES for nothing.
+    const stray = [...UNSUPPORTED_AGGREGATES].filter(
+      (a) => !(AggregationFunction.options as string[]).includes(a),
+    );
+    expect(stray).toEqual([]);
+  });
+
+  it('the two halves partition the vocabulary', () => {
+    expect(SUPPORTED_AGGREGATES.length + UNSUPPORTED_AGGREGATES.size)
+      .toBe(AggregationFunction.options.length);
+  });
+
+  it('records the current split, so a vocabulary change is visible in review', () => {
+    expect([...SUPPORTED_AGGREGATES].sort())
+      .toEqual(['avg', 'count', 'count_distinct', 'max', 'min', 'sum']);
+    expect([...UNSUPPORTED_AGGREGATES].sort()).toEqual(['array_agg', 'string_agg']);
+  });
+});
+
+describe('the compiler error message is derived, not restated', () => {
+  it('names every supported aggregate, and none of the unsupported ones', async () => {
+    const { compileDataset } = await import('./dataset-compiler.js');
+    let message = '';
+    try {
+      compileDataset({
+        name: 'agg_probe',
+        object: 'showcase_task',
+        dimensions: [{ name: 'status', field: 'status', type: 'string' }],
+        measures: [{ name: 'names', field: 'title', aggregate: 'string_agg' }],
+      } as never);
+    } catch (e) {
+      message = (e as Error).message;
+    }
+    expect(message).toContain('string_agg');
+    for (const a of SUPPORTED_AGGREGATES) {
+      expect(message, `error message omits supported aggregate "${a}"`).toContain(a);
+    }
+    // The prose list it replaced would have kept claiming these are supported.
+    expect(message).not.toContain('array_agg,');
+  });
+});

packages/services/service-analytics/src/dataset-compiler.ts

@@ -1,6 +1,7 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
 import type { Cube, Metric, Dimension as CubeDimension, CubeJoin } from '@objectstack/spec/data';
+import { AggregationFunction } from '@objectstack/spec/data';
 import type { Dataset, DatasetMeasure, DatasetDimension } from '@objectstack/spec/ui';
 import type { FilterCondition } from '@objectstack/spec/data';
 
@@ -21,7 +22,20 @@ import type { FilterCondition } from '@objectstack/spec/data';
  */
 
 /** Operators v1 does NOT compile to the Cube SQL switch — surfaced as a clear error. */
-const UNSUPPORTED_AGGREGATES = new Set(['array_agg', 'string_agg']);
+export const UNSUPPORTED_AGGREGATES = new Set(['array_agg', 'string_agg']);
+
+/**
+ * What v1 *can* lower — derived from the spec's vocabulary rather than restated.
+ *
+ * The list used to be hand-written prose inside the error message below, which
+ * made it a third copy of one vocabulary (after `AggregationFunction` and the
+ * `native-sql-strategy` switch) with nothing keeping the three in step. An
+ * aggregate added to the spec would have passed this gate, been reported as
+ * supported by that message, and then hit the strategy's `default` — returning
+ * a row count in place of the requested number. objectui#2945.
+ */
+export const SUPPORTED_AGGREGATES: string[] = AggregationFunction.options
+  .filter((a: string) => !UNSUPPORTED_AGGREGATES.has(a));
 
 export interface DerivedMeasureSpec {
   name: string;
@@ -82,7 +96,7 @@ function aggregateToMetricType(m: DatasetMeasure): Metric['type'] {
   if (UNSUPPORTED_AGGREGATES.has(m.aggregate)) {
     throw new Error(
       `[dataset-compiler] measure "${m.name}" uses aggregate "${m.aggregate}" which is ` +
-      `not supported by the v1 dataset runtime (supported: count, sum, avg, min, max, count_distinct).`,
+      `not supported by the v1 dataset runtime (supported: ${SUPPORTED_AGGREGATES.join(', ')}).`,
     );
   }
   return m.aggregate as Metric['type'];

packages/services/service-analytics/src/strategies/native-sql-strategy.ts

@@ -7,12 +7,49 @@ import { normalizeAnalyticsFilters, coerceFilterValueForSql } from './filter-nor
 import { compileScopedFilterToSql } from '../read-scope-sql.js';
 import { nextUtcCalendarDay } from '@objectstack/core';
 
+/**
+ * The SQL wrapper for each aggregate a measure's `type` can name.
+ *
+ * A table rather than a `switch` so its coverage is *assertable*: the aggregate
+ * vocabulary lives in `@objectstack/spec` (`AggregationFunction`), the dataset
+ * compiler subtracts the two it cannot lower (`array_agg`, `string_agg`), and
+ * `aggregation-lockstep.test.ts` checks that what remains is exactly the keys
+ * below. A `switch` gave that no purchase — the missing case fell to
+ * `default: COUNT(*)`, so an aggregate the spec grew would have returned a row
+ * count instead of the number the author asked for, silently. objectui#2945.
+ *
+ * Non-aggregate metric types (`number`/`string`/`boolean` — custom SQL
+ * expressions, `AggregationMetricType` in `data/analytics.zod.ts`) are
+ * deliberately absent, and keep the caller's existing fallback rather than
+ * changing behaviour here; see the note at {@link NativeSQLStrategy}.
+ */
+const AGGREGATE_SQL: Record<string, (col: string) => string> = {
+  'count': () => 'COUNT(*)',
+  'sum': (col) => `SUM(${col})`,
+  'avg': (col) => `AVG(${col})`,
+  'min': (col) => `MIN(${col})`,
+  'max': (col) => `MAX(${col})`,
+  'count_distinct': (col) => `COUNT(DISTINCT ${col})`,
+};
+
+/** Exported for the lockstep guard — the aggregates this strategy can lower. */
+export const SUPPORTED_AGGREGATE_SQL_KEYS = Object.keys(AGGREGATE_SQL);
+
 /**
  * NativeSQLStrategy — Priority 1
  *
  * Pushes the analytics query down to the database as a native SQL statement.
  * This is the most efficient path and is preferred whenever the backing driver
  * supports raw SQL execution (e.g. Postgres, MySQL, SQLite).
+ *
+ * Known gap, unchanged by the lockstep work and reported separately: a measure
+ * whose `type` is `number`/`string`/`boolean` — a custom SQL *expression*, not
+ * an aggregate — also lands on the `COUNT(*)` fallback in
+ * `resolveMeasureSql`, so its expression is replaced by a row count. Datasets
+ * cannot produce such a measure (`aggregateToMetricType` only ever returns an
+ * `AggregationFunction` member), so this is reachable only from a hand-authored
+ * Cube. Left as-is on purpose: emitting `col` instead is a behavioural change
+ * in an analytics SQL path, and deserves its own change with its own tests.
  */
 export class NativeSQLStrategy implements AnalyticsStrategy {
   readonly name = 'NativeSQLStrategy';
@@ -370,15 +407,8 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
     const col = measure.sql === '*'
       ? '*'
       : this.qualifyAndRegisterJoin(measure.sql, parentTable, joins, cube);
-    switch (measure.type) {
-      case 'count': return 'COUNT(*)';
-      case 'sum': return `SUM(${col})`;
-      case 'avg': return `AVG(${col})`;
-      case 'min': return `MIN(${col})`;
-      case 'max': return `MAX(${col})`;
-      case 'count_distinct': return `COUNT(DISTINCT ${col})`;
-      default: return `COUNT(*)`;
-    }
+    const wrap = AGGREGATE_SQL[measure.type];
+    return wrap ? wrap(col) : `COUNT(*)`;
   }
 
   private resolveFieldSql(