Skip to content

Commit d028b37

Browse files
os-trumpclaude
andauthored
fix(spec): narrow StrategyContext.executeAggregate aggregations[].method to AggregationFunction (#12937)
* fix(spec): narrow StrategyContext.executeAggregate aggregations[].method to AggregationFunction Maintainer ruling 2026-08-28 (option A, census-first). The engine contract (IDataEngine.aggregate -> AggregationNodeSchema.function) and the analytics strategy contract described the same slot with two types: a closed six-value enum on one side, bare string on the other. One slot, one declaration: method now carries the spec's own AggregationFunction (count | sum | avg | min | max | count_distinct). The #11833 runtime parse-and-refuse in the bridge stays as defence in depth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4 * fix(service-analytics): carry AggregationFunction through ObjectQLStrategy's aggregation locals The spec narrowing surfaces exactly two TS2322s at the strategy's ctx.executeAggregate call sites (measured against a BASE-spec baseline: error sets differ by only these two). The local aggregations annotations and resolveMeasureAggregation's return type now carry the enum; the alias path proves it by the existing equality guard (no cast), the direct path asserts it with prose keeping the documented no-allowlist posture for host-drift cubes. Runtime behaviour unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4 * chore: breaking-grade changeset with FROM/TO for the method narrowing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4 * chore(spec): register the narrowing in the ADR-0087 ledger (semantic entry) The adr-0087 gate's own verdict: a changeset carrying a FROM->TO prescription must register its migration. Semantic entry (D3): a TS interface member has no authored document or sys_metadata row to rewrite, so the ledger entry plus the compile error are the upgrade channel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpRNHxWZgSUgVnFT9mQQo4 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b5a2398 commit d028b37

5 files changed

Lines changed: 163 additions & 8 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-analytics": patch
4+
---
5+
6+
fix(spec): `StrategyContext.executeAggregate` `aggregations[].method` narrows from `string` to `AggregationFunction` (#12776)
7+
8+
<!-- adr-0087: registered strategy-context-aggregation-method-narrowed -->
9+
10+
**BREAKING** accept-set narrowing on a published contract, landing after the
11+
v17.0.0 cut (the lockstep launch-window convention ships it as `minor`).
12+
13+
Two spec-declared surfaces described the same slot and disagreed about its
14+
type: `IDataEngine.aggregate`'s `aggregations[].function` is the closed
15+
six-value `AggregationFunction` enum, while the analytics strategy contract's
16+
`StrategyContext.executeAggregate` declared the same value as
17+
`aggregations[].method: string`. The analytics bridge renames one to the
18+
other, so nothing on the analytics side of that seam was compile-checked
19+
against the engine's vocabulary — a strategy author (very often an AI) got
20+
no compile-time help and hit the bridge's runtime refusal instead.
21+
22+
FROM → TO:
23+
24+
- `aggregations[].method: string`
25+
`aggregations[].method: AggregationFunction`
26+
(`'count' | 'sum' | 'avg' | 'min' | 'max' | 'count_distinct'`, the spec's
27+
own enum from `@objectstack/spec/data`). One slot, one declaration.
28+
29+
Who breaks at compile time on upgrade:
30+
31+
- external CALLERS of `StrategyContext.executeAggregate` that fill `method`
32+
with a value typed `string` (or a literal outside the six) — the values the
33+
bridge already refused at runtime (#11833) now fail `tsc`.
34+
- external IMPLEMENTORS of `StrategyContext` stay source-compatible: a
35+
handler that accepts `method: string` accepts a superset and remains
36+
assignable to the narrowed member.
37+
38+
The bridge's runtime parse-and-refuse (#11833) stays as defence in depth.
39+
In-repo, `ObjectQLStrategy`'s aggregation locals now carry the enum
40+
end-to-end (`@objectstack/service-analytics`, runtime behaviour unchanged —
41+
the census measured every reachable producer already emitting enum-legal
42+
values only).

packages/services/service-analytics/src/strategies/objectql-strategy.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import type { AnalyticsQuery, AnalyticsResult } from '@objectstack/spec/contracts';
4-
import type { Cube } from '@objectstack/spec/data';
4+
import type { AggregationFunction, Cube } from '@objectstack/spec/data';
55
// [#8220] The read-scope provenance mark: `withReadScope` below is one of the
66
// two merge boundaries that stamp it.
77
import { markFilterSubtreeProvenance } from '@objectstack/spec/data';
@@ -172,7 +172,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
172172
// returns `null` for a filter that constrains nothing (an empty object),
173173
// matching the engine's own "empty filter is vacuous" convention — so a
174174
// vacuous measure filter adds no `filter` key rather than an empty one.
175-
const aggregations: Array<{ field: string; method: string; alias: string; filter?: Record<string, unknown> }> = [];
175+
const aggregations: Array<{ field: string; method: AggregationFunction; alias: string; filter?: Record<string, unknown> }> = [];
176176
if (query.measures && query.measures.length > 0) {
177177
for (const measure of query.measures) {
178178
const { field, method } = this.resolveMeasureAggregation(cube, measure);
@@ -969,7 +969,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
969969
private async executeCrossObject(
970970
cube: Cube,
971971
query: AnalyticsQuery,
972-
aggregations: Array<{ field: string; method: string; alias: string; filter?: Record<string, unknown> }>,
972+
aggregations: Array<{ field: string; method: AggregationFunction; alias: string; filter?: Record<string, unknown> }>,
973973
filter: Record<string, unknown>,
974974
plan: CrossObjectPlan,
975975
ctx: StrategyContext,
@@ -1259,7 +1259,7 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
12591259
return member.includes('.') ? member.split('.')[1] : member;
12601260
}
12611261

1262-
private resolveMeasureAggregation(cube: Cube, measureName: string): { field: string; method: string } {
1262+
private resolveMeasureAggregation(cube: Cube, measureName: string): { field: string; method: AggregationFunction } {
12631263
const direct = this.lookupMember(cube, measureName, 'measure') as
12641264
| { sql: string; type: string }
12651265
| undefined;
@@ -1306,24 +1306,32 @@ export class ObjectQLStrategy implements AnalyticsStrategy {
13061306
}
13071307
return {
13081308
field: direct.sql.replace(/^\$/, ''),
1309-
method: direct.type === 'count_distinct' ? 'count_distinct' : direct.type,
1309+
// The assertion, not a parse: for a CubeSchema-legal cube the type
1310+
// partition above leaves exactly the six `AggregationFunction` values.
1311+
// An enum-INVALID type (host drift, the comment above) still flows
1312+
// through unchecked ON PURPOSE — adding a method allowlist here would
1313+
// re-blame the caller with a 400 for OUR bug, so the cast keeps the
1314+
// compile-time contract (#12776) without changing that posture.
1315+
method: (direct.type === 'count_distinct' ? 'count_distinct' : direct.type) as AggregationFunction,
13101316
};
13111317
}
13121318
// Accept `${field}_${type}` aliases (e.g. 'amount_sum') for measures whose
13131319
// canonical name is just `${field}` (e.g. measure 'amount' of type 'sum').
13141320
// This matches the convention used by clients that build measure names
13151321
// from (field, function) pairs (e.g. the data-objectstack adapter).
13161322
const fieldName = measureName.includes('.') ? measureName.split('.')[1] : measureName;
1317-
const aggTypes = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'];
1323+
const aggTypes = ['count', 'sum', 'avg', 'min', 'max', 'count_distinct'] as const;
13181324
for (const type of aggTypes) {
13191325
const suffix = `_${type}`;
13201326
if (fieldName.endsWith(suffix)) {
13211327
const baseField = fieldName.slice(0, -suffix.length);
13221328
const candidate = cube.measures[baseField];
13231329
if (candidate && candidate.type === type) {
1330+
// `type` ranges over the six `AggregationFunction` literals and the
1331+
// guard just proved `candidate.type` equal to it (#12776) — no cast.
13241332
return {
13251333
field: candidate.sql.replace(/^\$/, ''),
1326-
method: candidate.type === 'count_distinct' ? 'count_distinct' : candidate.type,
1334+
method: type,
13271335
};
13281336
}
13291337
}

packages/spec/src/contracts/analytics-service.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import type { AnalyticsQuery, Cube } from '../data/analytics.zod.js';
44
import type { FilterCondition } from '../data/filter.zod.js';
5+
import type { AggregationFunction } from '../data/query.zod.js';
56
import type { PercentScale } from '../data/percent-scale.js';
67
import type { ExecutionContext } from '../kernel/execution-context.zod.js';
78
import type { Dataset } from '../ui/dataset.zod.js';
@@ -296,8 +297,14 @@ export interface StrategyContext {
296297
* (`AggregationNodeSchema.filter`), which the engine honours on every
297298
* driver by lowering in memory when the driver has no native
298299
* conditional aggregation.
300+
*
301+
* `method` is the engine's own closed vocabulary
302+
* (`AggregationFunction`, data/query.zod.ts) — the same slot
303+
* `engine.aggregate`'s `aggregations[].function` declares. It was
304+
* `string` until #12776; the bridge's runtime parse-and-refuse
305+
* (#11833) stays as defence in depth behind this compile-time check.
299306
*/
300-
aggregations?: Array<{ field: string; method: string; alias: string; filter?: FilterCondition }>;
307+
aggregations?: Array<{ field: string; method: AggregationFunction; alias: string; filter?: FilterCondition }>;
301308
filter?: Record<string, unknown>;
302309
/**
303310
* Reference timezone (IANA name) for date bucketing (ADR-0053 Phase 2).
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { SemanticMigration } from '../../types.js';
4+
5+
export const entry: SemanticMigration = {
6+
id: 'strategy-context-aggregation-method-narrowed',
7+
surface: 'StrategyContext.executeAggregate aggregations[].method '
8+
+ '(contracts/analytics-service.ts, exported from @objectstack/spec/contracts) '
9+
+ '- the parameter type, declared as bare string',
10+
replacement: 'AggregationFunction (count | sum | avg | min | max | count_distinct, '
11+
+ 'data/query.zod.ts) - the same closed vocabulary IDataEngine.aggregate already '
12+
+ 'declares for the identical slot (AggregationNodeSchema.function; the analytics '
13+
+ 'bridge renames method to function and forwards). A caller filling method from a '
14+
+ 'string-typed value narrows the value to the enum - typing it '
15+
+ 'AggregationFunction, or parsing with the spec\'s own AggregationFunction zod '
16+
+ 'enum where the value enters from data. Values outside the six were never '
17+
+ 'served: the bridge has parsed-and-refused them at runtime since #11833, and '
18+
+ 'that refusal stays as defence in depth',
19+
reason:
20+
'#12776, maintainer ruling 2026-08-28 (option A, census-first). Two spec-declared '
21+
+ 'surfaces described the same value and disagreed about its type: '
22+
+ 'IDataEngine.aggregate\'s aggregations[].function is the closed six-value '
23+
+ 'AggregationFunction enum while StrategyContext.executeAggregate declared the '
24+
+ 'same slot aggregations[].method: string, so nothing on the analytics side of '
25+
+ 'that seam was compile-checked against the engine\'s vocabulary - an author, '
26+
+ 'very often an AI (ADR-0033), writing an analytics strategy got no compile-time '
27+
+ 'help and could carry any method name all the way to the bridge\'s runtime '
28+
+ 'refusal. One slot now has one declaration. Bookkeeping: this is a TYPE '
29+
+ 'narrowing on a runtime TS interface member - no authorable metadata key, no '
30+
+ 'wire shape and no walked-shape def changed, so nothing lands in '
31+
+ 'RETIRED_KEYS_BY_MAJOR / RETIRED_DEFS_BY_MAJOR and the surface ratchets are '
32+
+ 'expected byte-identical. It is a SEMANTIC entry rather than a D2 conversion '
33+
+ 'because there is no authored document or sys_metadata row for the chain to '
34+
+ 'rewrite: the only consumers are TypeScript call sites, and the compile error '
35+
+ 'is the channel that reaches them. In-repo census at the ruling (hard '
36+
+ 'precondition, measured before the narrowing landed): every implementor and '
37+
+ 'every call site filling method is legal under the enum - '
38+
+ 'ObjectQLStrategy.resolveMeasureAggregation emits only the six post-#12209 '
39+
+ 'refusal, the two literal producers write count, and every test fixture is '
40+
+ 'implementor-side and stays assignable by contravariance.',
41+
acceptanceCriteria:
42+
'External implementors of StrategyContext stay source-compatible: a handler '
43+
+ 'accepting method: string accepts a superset and remains assignable to the '
44+
+ 'narrowed member. External callers filling method with a string-typed or '
45+
+ 'out-of-vocabulary value fail tsc at the executeAggregate call site on upgrade; '
46+
+ 'the fix is narrowing the value\'s type to AggregationFunction (parsing with '
47+
+ 'the spec enum where it enters from data), never widening a local mirror of '
48+
+ 'the contract. Runtime behaviour is unchanged: the bridge\'s #11833 '
49+
+ 'parse-and-refuse accepts and rejects exactly the same sets before and after, '
50+
+ 'and no stored metadata or document needs editing.',
51+
};

packages/spec/src/migrations/registry.ts

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

0 commit comments

Comments
 (0)