Skip to content

Commit e89fa92

Browse files
os-justinclaude
andauthored
feat(spec): declare aggregate? on IDataDriver with the signature the engine calls (#15384)
The engine dispatches native aggregation by presence (typeof driver.aggregate === 'function') and calls driver.aggregate(object, ast, options), but IDataDriver never declared the member, so a custom driver's aggregate was checked in neither direction: swapped arguments or a non-row result compiled clean and surfaced only after the engine's having filter matched nothing. Declared optional, matching the presence test, with the row shape aggregation-conformance pins. All four in-repo implementations (memory, mongodb, sql, turso remote transport via TursoDriver) already satisfy it; no driver, capability bit, or engine cast changes. The recording double in engine-filter-array-lowering.test.ts drops its local RecordingDriver extension and annotates against IDataDriver directly. Claude-Session: https://claude.ai/code/session_01H2oQebDDxYKfWZusyd8GXk Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent bcb6a17 commit e89fa92

3 files changed

Lines changed: 53 additions & 10 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
`IDataDriver` now declares `aggregate?` — the one engine-reached driver verb that had no signature to match against.
6+
7+
The engine has always dispatched native aggregation by presence (`typeof driver.aggregate === 'function'`) and called `driver.aggregate(object, query, options)`, but the interface never spelled the member, so a custom driver's `aggregate` was checked in neither direction: swapped arguments or a non-row result compiled clean and surfaced only after the engine's `having` filter silently matched nothing. The member is declared optional, matching the presence test — a driver without native aggregation omits it and stays conformant, served by the `find()` + in-memory fallback.
8+
9+
Additive: every in-repo driver already satisfies the declared signature (`(object: string, query: DriverQuery, options?: DriverOptions) => Promise<Record<string, unknown>[]>`); a wider parameter union or a looser return type stays assignable. What is newly refused is a wrong argument order or a non-array result. No `DriverCapabilities` bit is added — presence remains the capability test, as `data/driver.zod.ts` rules.

packages/objectql/src/engine-filter-array-lowering.test.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
import { describe, it, expect, beforeEach } from 'vitest';
2929
import type {
30-
DriverOptions,
3130
EngineAggregateOptions,
3231
EngineCountOptions,
3332
EngineQueryOptions,
@@ -90,15 +89,14 @@ interface SeenRead { ast: DriverQuery }
9089
* the annotation THIS declaration is what fails when a member is added or a
9190
* signature moves.
9291
*
93-
* The one extension: `aggregate` is NOT on `IDataDriver`. The engine reaches it
94-
* by duck-typing (`typeof drv.aggregate === 'function'`, `engine.ts`), so it is
95-
* declared here explicitly — the pin exercises that verb and must keep
96-
* witnessing it. ⚠️ Declared here does NOT make it contractual; it records that
97-
* this double answers a call the interface does not describe.
92+
* `aggregate` included: the engine reaches it by duck-typing
93+
* (`typeof drv.aggregate === 'function'`, `engine.ts`), and until #14345 the
94+
* interface did not declare it, so this file carried a local extension for
95+
* the one verb. `IDataDriver.aggregate?` now spells the signature the engine
96+
* calls, so the double's `aggregate` is checked by the same annotation as
97+
* every other verb — no local extension, nothing this double answers that the
98+
* contract does not describe.
9899
*/
99-
interface RecordingDriver extends IDataDriver {
100-
aggregate(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>;
101-
}
102100

103101
/**
104102
* A verb the pin does not exercise, present only because `IDataDriver` requires
@@ -152,7 +150,7 @@ function makeRecordingDriver() {
152150
const out = [...rows.values()].filter((r) => matches(r, ast?.where));
153151
return typeof ast?.limit === 'number' && ast.limit > 0 ? out.slice(0, ast.limit) : out;
154152
};
155-
const driver: RecordingDriver = {
153+
const driver: IDataDriver = {
156154
name: 'recording', version: '0.0.0', supports: {},
157155
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
158156
async find(_o: string, ast: DriverQuery) { reads.push({ ast }); return run(ast); },

packages/spec/src/contracts/data-driver.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,42 @@ export interface IDataDriver {
213213
*/
214214
count(object: string, query?: DriverQuery, options?: DriverOptions): Promise<number>;
215215

216+
/**
217+
* Native aggregation pushdown: execute `query.groupBy` / `query.aggregations`
218+
* inside the store and return the ALREADY-GROUPED rows.
219+
*
220+
* What the engine passes (objectql `engine.ts`, `ObjectQL.aggregate`): the
221+
* object name; the full query AST it built for the read (`where`, `groupBy`,
222+
* `aggregations`, `orderBy`, `limit`, ...) — the same {@link DriverQuery}
223+
* `find()` receives, so a value that satisfies `find` satisfies this; and the
224+
* per-call {@link DriverOptions}. Presence IS the capability test: the engine
225+
* dispatches on `typeof driver.aggregate === 'function'` — the rule
226+
* `data/driver.zod.ts` states for `DriverCapabilities`, and why there is
227+
* deliberately no `queryAggregations` bit — and otherwise falls back to
228+
* `find()` plus in-memory grouping. Two things the engine keeps for itself
229+
* even when it dispatches here: `having` is applied AFTER this call, over the
230+
* returned rows (#4286); and `aggregations[].filter` (#10576) or a non-UTC
231+
* `timezone` with date bucketing (ADR-0053) force the in-memory path, so a
232+
* driver never sees either on this road today.
233+
*
234+
* What a driver returns: one row per group, keyed by the `groupBy` field
235+
* names and by each aggregation's `alias`, with the per-function value
236+
* semantics `data/aggregation-conformance.ts` pins for every face that
237+
* lowers this vocabulary. The engine's `having` filter reads exactly those
238+
* keys, so a mis-keyed row does not error — it silently fails to match.
239+
*
240+
* Declared for the reason `introspectSchema` gives below: the engine reached
241+
* this verb by duck-typing with no signature to mis-match against, so
242+
* `aggregate(query, object)` with the arguments swapped, or a non-row
243+
* result, compiled clean and surfaced only downstream of `having`. Optional,
244+
* matching the presence test — a driver without native aggregation
245+
* (driver-rest today) omits the member and stays conformant, served by the
246+
* fallback. A wider parameter union (driver-memory also accepts a raw
247+
* pipeline) or a looser return type stays assignable; what is refused is a
248+
* wrong argument order or a non-array result.
249+
*/
250+
aggregate?(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>;
251+
216252
// ===========================================================================
217253
// Bulk Operations
218254
// ===========================================================================

0 commit comments

Comments
 (0)