fix(service-analytics): narrow the executeAggregate config mirrors' aggregations[].method to AggregationFunction (#13361)

#12776 narrowed StrategyContext.executeAggregate's aggregations[].method from
`string` to the six-value AggregationFunction enum, but this package's own
consumer-local mirrors of that slot kept declaring `string`, so a custom-bridge
author still typed their handler against the open vocabulary.

Narrows all three sites the tree carries: the AnalyticsServicePluginOptions
mirror (plugin.ts), the AnalyticsServiceConfig twin (analytics-service.ts) and
parseEngineAggregateFunction's parameter, and repairs the two comments that
explained the auto-bridge's runtime parse by pointing at the old `method:
string` declaration. The parse itself is untouched: with both ends of the
`method` -> `function` rename now declaring the same enum, it is defence in
depth behind a compile-time check rather than the only check.

Type-only; no runtime change.


Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-elon

.changeset/analytics-config-mirrors-aggregation-method-narrowed.md

@@ -0,0 +1,40 @@
+---
+"@objectstack/service-analytics": patch
+---
+
+fix(service-analytics): the consumer-local `executeAggregate` config mirrors narrow `aggregations[].method` to `AggregationFunction` (#12940)
+
+#12776 narrowed the contract — `StrategyContext.executeAggregate`'s
+`aggregations[].method` went from `string` to the six-value
+`AggregationFunction` — but this package's two CONSUMER-LOCAL config mirrors
+of that same slot kept declaring `string`, so the compile-time vocabulary the
+narrowing bought for strategy authors stopped at the package boundary and
+never reached the people who write a custom bridge.
+
+FROM → TO, at all three sites the tree carries (the card enumerated two):
+
+- `AnalyticsServicePluginOptions.executeAggregate` (`plugin.ts`) —
+  `aggregations[].method: string` → `AggregationFunction`. This is the
+  declaration an app author's own `executeAggregate` bridge is typed against.
+- `AnalyticsServiceConfig.executeAggregate` (`analytics-service.ts`) — the
+  same narrowing on the config twin whose own comment says it is kept in
+  lockstep with `StrategyContext.executeAggregate`; that claim is true again,
+  and now names the member so the next drift is visible.
+- `parseEngineAggregateFunction`'s `method` parameter (`plugin.ts`), the
+  auto-bridge's runtime parse — narrowed for the same reason: it was the
+  third place a reader was told this vocabulary is open.
+
+Who breaks at compile time on upgrade: CALLERS that fill `method` with a
+value typed `string` (or a literal outside the six) when invoking one of
+these bridges — the values the bridge already refused at runtime (#11833).
+IMPLEMENTORS are source-compatible: a handler that accepts `method: string`
+accepts a superset and stays assignable to the narrowed member (parameter
+contravariance), which is why the ~nine test doubles in this package that
+declare their own `{ field, method: string, alias }` mirrors still compile
+untouched.
+
+No runtime change. The auto-bridge's runtime parse-and-refuse (#11833) stays
+exactly where it was — with both ends of the `method` → `function` rename now
+declaring the same enum, it is defence in depth behind a compile-time check
+rather than the only check, and the two comments that explained it by
+pointing at the old `method: string` declaration say so instead.

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

@@ -7,7 +7,7 @@ import type {
   CubeMeta,
   DatasetSelection,
 } from '@objectstack/spec/contracts';
-import { percentScaleOf, type Cube, type FilterCondition } from '@objectstack/spec/data';
+import { percentScaleOf, type AggregationFunction, type Cube, type FilterCondition } from '@objectstack/spec/data';
 import type { ExecutionContext } from '@objectstack/spec/kernel';
 import type { Dataset } from '@objectstack/spec/ui';
 // [#6761] The ONE shared `I18nLabel → string` resolver (#6765, maintainer
@@ -390,15 +390,23 @@ export interface AnalyticsServiceConfig {
   executeAggregate?: (objectName: string, options: {
     groupBy?: string[];
     /**
-     * Per-aggregation `filter` (#10576, the #10413 phase-2 contract field) —
-     * kept in lockstep with `StrategyContext.executeAggregate`
-     * (`packages/spec/src/contracts/analytics-service.ts`), which this local
-     * type otherwise mirrors. A bridge that reconstructs the aggregation
-     * entries (as `AnalyticsServicePlugin`'s auto-bridge does, to rename
-     * `method` → the engine's `function`) MUST forward this field or a
-     * measure-scoped filter `ObjectQLStrategy` lowers never reaches storage.
+     * The local mirror of `StrategyContext.executeAggregate`'s aggregation
+     * entries (`packages/spec/src/contracts/analytics-service.ts`), kept in
+     * lockstep with it member by member. The two that lockstep is load-bearing
+     * for:
+     *
+     * - `filter` (#10576, the #10413 phase-2 contract field) — a bridge that
+     *   reconstructs the aggregation entries (as `AnalyticsServicePlugin`'s
+     *   auto-bridge does, to rename `method` → the engine's `function`) MUST
+     *   forward this field or a measure-scoped filter `ObjectQLStrategy`
+     *   lowers never reaches storage.
+     * - `method` is the spec's OWN six-value `AggregationFunction`, not
+     *   `string`: #12776 narrowed the contract, #12940 brought this mirror
+     *   back into line. Widening it here again would not be a local matter —
+     *   a bridge author types their handler against THIS declaration, so what
+     *   they would get is a vocabulary the contract no longer has.
      */
-    aggregations?: Array<{ field: string; method: string; alias: string; filter?: Record<string, unknown> }>;
+    aggregations?: Array<{ field: string; method: AggregationFunction; alias: string; filter?: Record<string, unknown> }>;
     filter?: Record<string, unknown>;
     /** Reference timezone (IANA) for date bucketing — ADR-0053 Phase 2. */
     timezone?: string;

packages/services/service-analytics/src/plugin.ts

@@ -67,18 +67,31 @@ type TemporalDriverSurface = Pick<
 >;
 
 /**
- * Narrow a strategy-supplied aggregation `method` to the engine contract's
- * `AggregationFunction`, refusing anything else.
+ * Re-parse a bridge-supplied aggregation `method` as the engine contract's
+ * `AggregationFunction` before it is forwarded as `function`, refusing
+ * anything else.
  *
- * The two sides genuinely differ: `IDataEngine.aggregate`'s
- * `aggregations[].function` is the six-value enum, while the analytics
- * strategy contract that feeds this bridge declares `aggregations[].method` as
- * `string`. Parsing with the spec's OWN enum keeps a single vocabulary — no
- * local literal list to drift, and `AggregationFunction`'s error map already
- * knows the retired `array_agg` / `string_agg` spellings.
+ * Both sides now declare the same six-value enum: `IDataEngine.aggregate`'s
+ * `aggregations[].function`, and — since #12776 — the analytics strategy
+ * contract (`StrategyContext.executeAggregate`) plus the two consumer-local
+ * config mirrors this package keeps in lockstep with it (#12940). So this
+ * parse is DEFENCE IN DEPTH behind a compile-time check, not the only check
+ * (#11833).
+ *
+ * That is a reason to keep it, not to delete it. Types are erased: a
+ * JavaScript app supplying its own `executeAggregate`, or host drift arriving
+ * through a cube object that never met `CubeSchema`'s parse (the path
+ * `aggregate-bridge-function-vocabulary.test.ts` drives end to end), still
+ * reaches this seam carrying a method the engine does not declare. What the
+ * refusal buys is in `plugin.ts`'s forward below and in #12209: the engine is
+ * never handed a `function` no driver declares.
+ *
+ * Parsing with the spec's OWN enum keeps a single vocabulary — no local
+ * literal list to drift, and `AggregationFunction`'s error map already knows
+ * the retired `array_agg` / `string_agg` spellings.
  */
 function parseEngineAggregateFunction(
-  method: string,
+  method: AggregationFunction,
   alias: string,
 ): NonNullable<Parameters<IDataEngine['aggregate']>[1]['aggregations']>[number]['function'] {
   const parsed = AggregationFunction.safeParse(method);
@@ -116,13 +129,23 @@ export interface AnalyticsServicePluginOptions {
   executeAggregate?: (objectName: string, options: {
     groupBy?: string[];
     /**
-     * Per-aggregation `filter` (#10576, the #10413 contract field) — a
-     * CUSTOM bridge (an app author's own `executeAggregate`, as opposed to
-     * the auto-bridge below) MUST forward it to the real engine the same way
-     * the auto-bridge does, or a measure-scoped filter this plugin lowers
-     * onto the aggregation silently never reaches storage.
+     * The CUSTOM bridge's view of the aggregation entries — an app author's
+     * own `executeAggregate`, as opposed to the auto-bridge below. Mirrors
+     * `StrategyContext.executeAggregate`
+     * (`packages/spec/src/contracts/analytics-service.ts`) and must stay in
+     * lockstep with it; the two members that lockstep is load-bearing for:
+     *
+     * - `filter` (#10576, the #10413 contract field) — a custom bridge MUST
+     *   forward it to the real engine the same way the auto-bridge does, or a
+     *   measure-scoped filter this plugin lowers onto the aggregation
+     *   silently never reaches storage.
+     * - `method` is the spec's OWN six-value `AggregationFunction`, not
+     *   `string`: #12776 narrowed the contract, #12940 brought this mirror
+     *   back into line. This is the declaration a custom-bridge author types
+     *   their handler against, so it is where the compile-time vocabulary
+     *   #12776 bought for strategy authors reaches them too.
      */
-    aggregations?: Array<{ field: string; method: string; alias: string; filter?: Record<string, unknown> }>;
+    aggregations?: Array<{ field: string; method: AggregationFunction; alias: string; filter?: Record<string, unknown> }>;
     filter?: Record<string, unknown>;
     /** Reference timezone (IANA) for date bucketing — ADR-0053 Phase 2. */
     timezone?: string;
@@ -288,15 +311,24 @@ export class AnalyticsServicePlugin implements Plugin {
           // vacuous-filter convention.
           aggregations: aggregations?.map((a) => ({
             // [#11833] `function` is the engine contract's SIX-value
-            // `AggregationFunction`, while this bridge's own input declares
-            // `method: string` (`StrategyContext.executeAggregate`, spec
-            // `contracts/analytics-service.ts:300`). Narrowing the engine side
-            // to the contract turned that forward into a compile error — the
+            // `AggregationFunction`. This bridge's own input declared
+            // `method: string` when that history was written
+            // (`StrategyContext.executeAggregate`, spec
+            // `contracts/analytics-service.ts`), so the two ends of this
+            // rename spoke different vocabularies: narrowing the engine side
+            // to the contract turned the forward into a compile error — the
             // correct signal, and the one the deleted structural type hid by
             // declaring `function: string` on both sides.
             //
-            // Closed by PARSING with the spec enum itself rather than by
-            // widening back to `string` (what hid it) or casting past it
+            // Since #12776 (contract) and #12940 (this plugin's own config
+            // mirror above), BOTH ends declare the enum, so the rename is
+            // enum-to-enum and the parse below is defence in depth behind a
+            // compile-time check rather than the only check — see
+            // `parseEngineAggregateFunction` for why erased types still leave
+            // it load-bearing.
+            //
+            // It was closed by PARSING with the spec enum itself rather than
+            // by widening back to `string` (what hid it) or casting past it
             // (which keeps the hole and adds a lie). `AggregationFunction` is
             // the same schema `AggregationNodeSchema.function` is built from,
             // so there is one vocabulary, and its own error map already