Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/strategy-context-aggregation-method-narrowed.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
"@objectstack/spec": minor
"@objectstack/service-analytics": patch
---

fix(spec): `StrategyContext.executeAggregate` `aggregations[].method` narrows from `string` to `AggregationFunction` (#12776)

<!-- adr-0087: registered strategy-context-aggregation-method-narrowed -->

**BREAKING** accept-set narrowing on a published contract, landing after the
v17.0.0 cut (the lockstep launch-window convention ships it as `minor`).

Two spec-declared surfaces described the same slot and disagreed about its
type: `IDataEngine.aggregate`'s `aggregations[].function` is the closed
six-value `AggregationFunction` enum, while the analytics strategy contract's
`StrategyContext.executeAggregate` declared the same value as
`aggregations[].method: string`. The analytics bridge renames one to the
other, so nothing on the analytics side of that seam was compile-checked
against the engine's vocabulary — a strategy author (very often an AI) got
no compile-time help and hit the bridge's runtime refusal instead.

FROM → TO:

- `aggregations[].method: string` →
`aggregations[].method: AggregationFunction`
(`'count' | 'sum' | 'avg' | 'min' | 'max' | 'count_distinct'`, the spec's
own enum from `@objectstack/spec/data`). One slot, one declaration.

Who breaks at compile time on upgrade:

- external CALLERS of `StrategyContext.executeAggregate` that fill `method`
with a value typed `string` (or a literal outside the six) — the values the
bridge already refused at runtime (#11833) now fail `tsc`.
- external IMPLEMENTORS of `StrategyContext` stay source-compatible: a
handler that accepts `method: string` accepts a superset and remains
assignable to the narrowed member.

The bridge's runtime parse-and-refuse (#11833) stays as defence in depth.
In-repo, `ObjectQLStrategy`'s aggregation locals now carry the enum
end-to-end (`@objectstack/service-analytics`, runtime behaviour unchanged —
the census measured every reachable producer already emitting enum-legal
values only).
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts';
import type { Cube } from '@objectstack/spec/data';
import type { AggregationFunction, Cube } from '@objectstack/spec/data';
// [#8220] The read-scope provenance mark: `withReadScope` below is one of the
// two merge boundaries that stamp it.
import { markFilterSubtreeProvenance } from '@objectstack/spec/data';
Expand DownExpand Up@@ -172,7 +172,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
// returns `null` for a filter that constrains nothing (an empty object),
// matching the engine's own "empty filter is vacuous" convention — so a
// vacuous measure filter adds no `filter` key rather than an empty one.
const aggregations: Array<{ field: string; method: string; alias: string; filter?: Record<string, unknown> }> = [];
const aggregations: Array<{ field: string; method: AggregationFunction; alias: string; filter?: Record<string, unknown> }> = [];
if (query.measures && query.measures.length > 0) {
for (const measure of query.measures) {
const { field, method } = this.resolveMeasureAggregation(cube, measure);
Expand DownExpand Up@@ -969,7 +969,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
private async executeCrossObject(
cube: Cube,
query: AnalyticsQuery,
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>,
plan: CrossObjectPlan,
ctx: StrategyContext,
Expand DownExpand Up@@ -1259,7 +1259,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
return member.includes('.') ? member.split('.')[1] : member;
}

private resolveMeasureAggregation(cube: Cube, measureName: string): { field: string; method: string } {
private resolveMeasureAggregation(cube: Cube, measureName: string): { field: string; method: AggregationFunction } {
const direct = this.lookupMember(cube, measureName, 'measure') as
| { sql: string; type: string }
| undefined;
Expand DownExpand Up@@ -1306,24 +1306,32 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
}
return {
field: direct.sql.replace(/^\$/, ''),
method: direct.type === 'count_distinct' ? 'count_distinct' : direct.type,
// The assertion, not a parse: for a CubeSchema-legal cube the type
// partition above leaves exactly the six `AggregationFunction` values.
// An enum-INVALID type (host drift, the comment above) still flows
// through unchecked ON PURPOSE — adding a method allowlist here would
// re-blame the caller with a 400 for OUR bug, so the cast keeps the
// compile-time contract (#12776) without changing that posture.
method: (direct.type === 'count_distinct' ? 'count_distinct' : direct.type) as AggregationFunction,
};
}
// Accept `${field}_${type}` aliases (e.g. 'amount_sum') for measures whose
// canonical name is just `${field}` (e.g. measure 'amount' of type 'sum').
// This matches the convention used by clients that build measure names
// from (field, function) pairs (e.g. the data-objectstack adapter).
const fieldName = measureName.includes('.') ? measureName.split('.')[1] : measureName;
const aggTypes = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'];
const aggTypes = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'] as const;
for (const type of aggTypes) {
const suffix = `_${type}`;
if (fieldName.endsWith(suffix)) {
const baseField = fieldName.slice(0, -suffix.length);
const candidate = cube.measures[baseField];
if (candidate && candidate.type === type) {
// `type` ranges over the six `AggregationFunction` literals and the
// guard just proved `candidate.type` equal to it (#12776) — no cast.
return {
field: candidate.sql.replace(/^\$/, ''),
method: candidate.type === 'count_distinct' ? 'count_distinct' : candidate.type,
method: type,
};
}
}
Expand Down
9 changes: 8 additions & 1 deletion packages/spec/src/contracts/analytics-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@

import type { AnalyticsQuery, Cube } from '../data/analytics.zod.js';
import type { FilterCondition } from '../data/filter.zod.js';
import type { AggregationFunction } from '../data/query.zod.js';
import type { PercentScale } from '../data/percent-scale.js';
import type { ExecutionContext } from '../kernel/execution-context.zod.js';
import type { Dataset } from '../ui/dataset.zod.js';
Expand DownExpand Up@@ -296,8 +297,14 @@ export interface StrategyContext {
* (`AggregationNodeSchema.filter`), which the engine honours on every
* driver by lowering in memory when the driver has no native
* conditional aggregation.
*
* `method` is the engine's own closed vocabulary
* (`AggregationFunction`, data/query.zod.ts) — the same slot
* `engine.aggregate`'s `aggregations[].function` declares. It was
* `string` until #12776; the bridge's runtime parse-and-refuse
* (#11833) stays as defence in depth behind this compile-time check.
*/
aggregations?: Array<{ field: string; method: string; alias: string; filter?: FilterCondition }>;
aggregations?: Array<{ field: string; method: AggregationFunction; alias: string; filter?: FilterCondition }>;
filter?: Record<string, unknown>;
/**
* Reference timezone (IANA name) for date bucketing (ADR-0053 Phase 2).
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import type { SemanticMigration } from '../../types.js';

export const entry: SemanticMigration = {
id: 'strategy-context-aggregation-method-narrowed',
surface: 'StrategyContext.executeAggregate aggregations[].method '
+ '(contracts/analytics-service.ts, exported from @objectstack/spec/contracts) '
+ '- the parameter type, declared as bare string',
replacement: 'AggregationFunction (count | sum | avg | min | max | count_distinct, '
+ 'data/query.zod.ts) - the same closed vocabulary IDataEngine.aggregate already '
+ 'declares for the identical slot (AggregationNodeSchema.function; the analytics '
+ 'bridge renames method to function and forwards). A caller filling method from a '
+ 'string-typed value narrows the value to the enum - typing it '
+ 'AggregationFunction, or parsing with the spec\'s own AggregationFunction zod '
+ 'enum where the value enters from data. Values outside the six were never '
+ 'served: the bridge has parsed-and-refused them at runtime since #11833, and '
+ 'that refusal stays as defence in depth',
reason:
'#12776, maintainer ruling 2026-08-28 (option A, census-first). Two spec-declared '
+ 'surfaces described the same value and disagreed about its type: '
+ 'IDataEngine.aggregate\'s aggregations[].function is the closed six-value '
+ 'AggregationFunction enum while StrategyContext.executeAggregate declared the '
+ 'same slot aggregations[].method: string, so nothing on the analytics side of '
+ 'that seam was compile-checked against the engine\'s vocabulary - an author, '
+ 'very often an AI (ADR-0033), writing an analytics strategy got no compile-time '
+ 'help and could carry any method name all the way to the bridge\'s runtime '
+ 'refusal. One slot now has one declaration. Bookkeeping: this is a TYPE '
+ 'narrowing on a runtime TS interface member - no authorable metadata key, no '
+ 'wire shape and no walked-shape def changed, so nothing lands in '
+ 'RETIRED_KEYS_BY_MAJOR / RETIRED_DEFS_BY_MAJOR and the surface ratchets are '
+ 'expected byte-identical. It is a SEMANTIC entry rather than a D2 conversion '
+ 'because there is no authored document or sys_metadata row for the chain to '
+ 'rewrite: the only consumers are TypeScript call sites, and the compile error '
+ 'is the channel that reaches them. In-repo census at the ruling (hard '
+ 'precondition, measured before the narrowing landed): every implementor and '
+ 'every call site filling method is legal under the enum - '
+ 'ObjectQLStrategy.resolveMeasureAggregation emits only the six post-#12209 '
+ 'refusal, the two literal producers write count, and every test fixture is '
+ 'implementor-side and stays assignable by contravariance.',
acceptanceCriteria:
'External implementors of StrategyContext stay source-compatible: a handler '
+ 'accepting method: string accepts a superset and remains assignable to the '
+ 'narrowed member. External callers filling method with a string-typed or '
+ 'out-of-vocabulary value fail tsc at the executeAggregate call site on upgrade; '
+ 'the fix is narrowing the value\'s type to AggregationFunction (parsing with '
+ 'the spec enum where it enters from data), never widening a local mirror of '
+ 'the contract. Runtime behaviour is unchanged: the bridge\'s #11833 '
+ 'parse-and-refuse accepts and rejects exactly the same sets before and after, '
+ 'and no stored metadata or document needs editing.',
};
47 changes: 47 additions & 0 deletions packages/spec/src/migrations/registry.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7128,6 +7128,53 @@ const step18: MigrationStep = {
+ 'of an envelope-level code; constructing an ApiError with a retired spelling '
+ 'fails `StandardErrorCode`/`ApiErrorSchema` parse rather than passing silently.',
},
{
id: 'strategy-context-aggregation-method-narrowed',
surface: 'StrategyContext.executeAggregate aggregations[].method '
+ '(contracts/analytics-service.ts, exported from @objectstack/spec/contracts) '
+ '- the parameter type, declared as bare string',
replacement: 'AggregationFunction (count | sum | avg | min | max | count_distinct, '
+ 'data/query.zod.ts) - the same closed vocabulary IDataEngine.aggregate already '
+ 'declares for the identical slot (AggregationNodeSchema.function; the analytics '
+ 'bridge renames method to function and forwards). A caller filling method from a '
+ 'string-typed value narrows the value to the enum - typing it '
+ 'AggregationFunction, or parsing with the spec\'s own AggregationFunction zod '
+ 'enum where the value enters from data. Values outside the six were never '
+ 'served: the bridge has parsed-and-refused them at runtime since #11833, and '
+ 'that refusal stays as defence in depth',
reason:
'#12776, maintainer ruling 2026-08-28 (option A, census-first). Two spec-declared '
+ 'surfaces described the same value and disagreed about its type: '
+ 'IDataEngine.aggregate\'s aggregations[].function is the closed six-value '
+ 'AggregationFunction enum while StrategyContext.executeAggregate declared the '
+ 'same slot aggregations[].method: string, so nothing on the analytics side of '
+ 'that seam was compile-checked against the engine\'s vocabulary - an author, '
+ 'very often an AI (ADR-0033), writing an analytics strategy got no compile-time '
+ 'help and could carry any method name all the way to the bridge\'s runtime '
+ 'refusal. One slot now has one declaration. Bookkeeping: this is a TYPE '
+ 'narrowing on a runtime TS interface member - no authorable metadata key, no '
+ 'wire shape and no walked-shape def changed, so nothing lands in '
+ 'RETIRED_KEYS_BY_MAJOR / RETIRED_DEFS_BY_MAJOR and the surface ratchets are '
+ 'expected byte-identical. It is a SEMANTIC entry rather than a D2 conversion '
+ 'because there is no authored document or sys_metadata row for the chain to '
+ 'rewrite: the only consumers are TypeScript call sites, and the compile error '
+ 'is the channel that reaches them. In-repo census at the ruling (hard '
+ 'precondition, measured before the narrowing landed): every implementor and '
+ 'every call site filling method is legal under the enum - '
+ 'ObjectQLStrategy.resolveMeasureAggregation emits only the six post-#12209 '
+ 'refusal, the two literal producers write count, and every test fixture is '
+ 'implementor-side and stays assignable by contravariance.',
acceptanceCriteria:
'External implementors of StrategyContext stay source-compatible: a handler '
+ 'accepting method: string accepts a superset and remains assignable to the '
+ 'narrowed member. External callers filling method with a string-typed or '
+ 'out-of-vocabulary value fail tsc at the executeAggregate call site on upgrade; '
+ 'the fix is narrowing the value\'s type to AggregationFunction (parsing with '
+ 'the spec enum where it enters from data), never widening a local mirror of '
+ 'the contract. Runtime behaviour is unchanged: the bridge\'s #11833 '
+ 'parse-and-refuse accepts and rejects exactly the same sets before and after, '
+ 'and no stored metadata or document needs editing.',
},
{
id: 'ui-cloud-connection-widgets-unknown-keys-refused',
surface: 'page `cloud-connection:panel` / `marketplace:installed-list` components — '
Expand Down
Loading