Skip to content

Commit c8be110

Browse files
os-litantclaude[bot]claude
authored
refactor(service-analytics): derive the auto-bridge's engine view from the declared contracts (#12777)
* refactor(service-analytics): derive the auto-bridge engine view from the declared contracts Replaces the consumer-local structural `DataEngineLike` in `service-analytics/src/plugin.ts` with the declared `IDataEngine` / `IObjectQLEngine` members - the #4251 B3 sweep pattern, and the second of the two sites named by #11833 (the first landed as PR #12011). The `aggregate` narrowing surfaced the mismatch the structural type hid: the local declaration typed `aggregations[].function` as `string` where the contract declares the six-value `AggregationFunction`. Closed by parsing with the spec enum itself at the forwarding site - not by widening back to `string` (which hid it) and not by a cast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 * test(service-analytics): pin the aggregate bridge's engine vocabulary Positive control (a declared function reaches the engine as `function`) plus the refusal (a method outside the engine's six never reaches the engine, and answers in the bare-Error/undeclared-500 tier rather than a 400 that would blame the caller for host drift). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 * chore: changeset for the analytics auto-bridge contract derivation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --------- Co-authored-by: claude[bot] <claude-bot@anthropic.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 21196cf commit c8be110

3 files changed

Lines changed: 274 additions & 92 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
refactor(service-analytics): derive the analytics auto-bridge's engine view from the declared contracts (#11833)
6+
7+
`plugin.ts` named the data engine through a consumer-local structural
8+
`DataEngineLike` — the second of the two sites #11833 records, after the
9+
datasource half that landed as PR #12011. It is now derived from the declared
10+
contracts: `IDataEngine.aggregate` / `execute?` /
11+
`resolveEffectiveDatasource?` / `getDriverForObject?` and
12+
`IObjectQLEngine.getObject`. Optionality is preserved exactly — `aggregate`
13+
required, everything else `Partial<>` — because these probes are the plugin's
14+
graceful-degradation seam.
15+
16+
**Why this is `patch` and not a type-only no-op.** Four of the five members
17+
substitute with no behaviour change. The fifth does not: the deleted structural
18+
type declared `aggregations[].function` as `string`, while the contract
19+
declares the six-value `AggregationFunction`. The bridge therefore forwarded
20+
whatever method string reached it. That forward is now parsed with the spec's
21+
own enum, so a method the engine contract does not declare is refused at the
22+
bridge — loudly, naming the aggregation and the legal vocabulary — instead of
23+
reaching the engine, where `driver-sql` blamed a `function` key the author
24+
never wrote and the in-memory evaluator answered `null` for every bucket under
25+
the author's own measure name.
26+
27+
No authored analytics can trigger the new refusal: the one reachable producer
28+
of a non-aggregate method — a custom-SQL measure (`AggregationMetricType`
29+
`number` / `string` / `boolean`) — is already refused earlier, caller-facing,
30+
by `ObjectQLStrategy.resolveMeasureAggregation` (#12209). What is left is host
31+
drift (a cube object registered without meeting `CubeSchema`), which is why
32+
the new refusal is a bare `Error` in the undeclared-500 tier rather than an
33+
ADR-0112 400 that would blame the caller for something they did not write.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#11833] The plugin's aggregate auto-bridge speaks the ENGINE's aggregate
5+
* vocabulary, and refuses anything else instead of forwarding it.
6+
*
7+
* `plugin.ts` used to name the engine through a consumer-local structural
8+
* `DataEngineLike` that declared `aggregations[].function` as `string`. The
9+
* declared contract (`IDataEngine.aggregate` →
10+
* `EngineAggregateOptions.aggregations[].function`) is the SIX-value
11+
* `AggregationFunction`. Nothing compiled the two against each other, so the
12+
* bridge forwarded whatever string reached it — and the engine then failed in
13+
* the two ways #12209 documents: `driver-sql` blaming a `function` key the
14+
* author never wrote, or the in-memory evaluator answering `null` for every
15+
* bucket under the author's own measure name (the #4157 class).
16+
*
17+
* Deriving the local view from the contract makes the forward a compile error;
18+
* these cases pin the RUNTIME half of that repair.
19+
*
20+
* ## Why this refusal is deliberately NOT in the ADR-0112 envelope
21+
*
22+
* The reachable producer of a non-aggregate method — a custom-SQL measure
23+
* (`AggregationMetricType` `number`/`string`/`boolean`) — is refused earlier
24+
* and caller-facing by `ObjectQLStrategy.resolveMeasureAggregation` (#12209,
25+
* `INVALID_FIELD` / 400). Anything still arriving at the bridge is host drift
26+
* (an unparsed cube object, our own drift), which `dataset-refusal.ts`'s module
27+
* header assigns to the bare-`Error`, undeclared-500 tier — the same tier it
28+
* assigns to `native-sql-strategy.ts`'s "measure … has unrecognised type". The
29+
* absence of a `code` is therefore asserted, not overlooked: enveloping this as
30+
* a 400 would tell the author to fix something they did not write.
31+
*/
32+
33+
import { describe, it, expect, vi } from 'vitest';
34+
import { AggregationFunction } from '@objectstack/spec/data';
35+
import type { Cube } from '@objectstack/spec/data';
36+
import type { AnalyticsService } from '../analytics-service.js';
37+
import { AnalyticsServicePlugin } from '../plugin.js';
38+
39+
type EngineAggregateCall = {
40+
object: string;
41+
aggregations?: Array<{ function: string; field?: string; alias: string }>;
42+
};
43+
44+
/**
45+
* Minimal `'data'` service: the one member the aggregate bridge requires.
46+
* `getObject` answers the schema so the source-field gates can stand.
47+
*/
48+
function fakeEngine(calls: EngineAggregateCall[], fields: Record<string, { type?: string }>) {
49+
return {
50+
getObject: (name: string) => (name === 'opportunity' ? { fields } : undefined),
51+
aggregate: async (object: string, options: EngineAggregateCall) => {
52+
calls.push({ object, aggregations: options.aggregations });
53+
return [{ region: 'west', total: 1 }];
54+
},
55+
};
56+
}
57+
58+
function fakePluginContext(services: Record<string, unknown>) {
59+
const registered: Record<string, unknown> = {};
60+
const warn = vi.fn();
61+
return {
62+
registered,
63+
ctx: {
64+
getService: (name: string) => services[name] ?? registered[name],
65+
registerService: (name: string, svc: unknown) => { registered[name] = svc; },
66+
replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
67+
logger: { info() {}, warn, error() {}, debug() {} },
68+
},
69+
};
70+
}
71+
72+
const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false });
73+
74+
/**
75+
* A cube carrying one measure of the given metric type.
76+
*
77+
* `type` is widened past `AggregationMetricType` on purpose: the drift case
78+
* below needs a cube that never met `CubeSchema`'s parse, which is exactly the
79+
* arrival path the refusal is tiered for. A parsed cube cannot carry it.
80+
*/
81+
const cubeWithMeasureType = (type: string): Cube => ({
82+
name: 'sales',
83+
title: 'Sales',
84+
sql: 'opportunity',
85+
measures: { revenue: { name: 'revenue', label: 'Revenue', type, sql: 'amount' } as Cube['measures'][string] },
86+
dimensions: { region: { name: 'region', label: 'Region', type: 'string', sql: 'region' } },
87+
public: false,
88+
});
89+
90+
async function analyticsVia(engine: unknown, cube: Cube): Promise<AnalyticsService> {
91+
const { ctx, registered } = fakePluginContext({ data: engine });
92+
await new AnalyticsServicePlugin({
93+
cubes: [cube],
94+
queryCapabilities: objectqlOnly,
95+
}).init(ctx as never);
96+
return registered.analytics as AnalyticsService;
97+
}
98+
99+
const selection = { cube: 'sales', dimensions: ['region'], measures: ['revenue'] };
100+
const schema = { region: { type: 'text' }, amount: { type: 'number' } };
101+
102+
describe('[#11833] the aggregate auto-bridge speaks the engine contract vocabulary', () => {
103+
it('forwards a declared aggregate function through to the engine', async () => {
104+
// Positive control: without this, the refusal case below could pass because
105+
// NOTHING reaches the engine, for reasons that have nothing to do with the
106+
// vocabulary.
107+
const calls: EngineAggregateCall[] = [];
108+
const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('sum'));
109+
110+
await service.query(selection as never);
111+
112+
expect(calls).toHaveLength(1);
113+
expect(calls[0].aggregations?.[0].function).toBe('sum');
114+
expect(AggregationFunction.options).toContain(calls[0].aggregations?.[0].function);
115+
});
116+
117+
it('refuses a method outside the engine vocabulary instead of forwarding it', async () => {
118+
// Host drift: a cube object registered without meeting `CubeSchema`, so its
119+
// `type` never faced the enum's parse. This is the arrival path the tiering
120+
// note above describes.
121+
const calls: EngineAggregateCall[] = [];
122+
const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('median'));
123+
124+
const err = await service.query(selection as never).then(() => null, (e: Error) => e);
125+
126+
expect(err).toBeInstanceOf(Error);
127+
// The wording IS the contract here: it must name the offending method, the
128+
// aggregation it belongs to, and the legal vocabulary.
129+
expect(err?.message).toContain('"median" is not one of the engine\'s aggregate functions');
130+
expect(err?.message).toContain('revenue');
131+
for (const fn of AggregationFunction.options) expect(err?.message).toContain(fn);
132+
// Undeclared-500 tier, deliberately: no ADR-0112 envelope on this family.
133+
expect((err as Error & { code?: string }).code).toBeUndefined();
134+
// The load-bearing half — the bad method never reached the engine, so no
135+
// driver got a chance to blame a `function` key nobody wrote and no bucket
136+
// came back silently null.
137+
expect(calls).toHaveLength(0);
138+
});
139+
});

0 commit comments

Comments
 (0)