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
100 changes: 100 additions & 0 deletions .changeset/data-driver-find-stream-retired.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
"@objectstack/spec": major
"@objectstack/driver-sql": major
"@objectstack/driver-memory": major
"@objectstack/driver-mongodb": major
---

refactor(spec,drivers)!: retire `IDataDriver.findStream` — a required method with no caller, whose two main implementations did the opposite of what it promised (#4484, ADR-0049 enforce-or-remove)

`findStream` was a **required** method on the driver contract — every driver and
every test double had to implement it — documented as the read

> Optimized for large datasets to avoid memory overflow.

Three things were true about it at once, and each is worse in the light of the
others.

**Nothing called it.** Not the query engine (there is no `stream` entry on it),
not REST export, not import, not any bulk-read path. Repo-wide, outside the
contract declaration and the three driver implementations, every single hit was
a test double — and roughly twenty of those satisfied the required method like
this:

```ts
findStream() { throw new Error('not implemented'); }
```

Twenty stubs that throw, across four packages, for years, and no test ever went
red. That is not an anecdote about test hygiene; it is the proof of absence. A
method whose every double throws is a method nothing reaches.

**Two of the three implementations inverted its one guarantee.** `SqlDriver` and
`InMemoryDriver` both did this:

```ts
const results = await this.find(object, query, options); // ← the entire result set
for (const row of results) yield row;
```

The whole table is resident in memory before the first `yield`. A caller who
believed the doc comment and reached for `findStream` precisely because a result
set was too large would have hit the overflow it existed to prevent, at exactly
the scale where it mattered. `SqlDriver` carried a `TODO: Use Knex .stream()`
admitting it.

**The one real implementation dropped a parameter.** `MongoDBDriver._findStream`
did walk a cursor — but it was the only read in that driver never routed through
`buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently
discarded `query.fields`. (#4459 unified `find`/`findOne` onto `buildFindOptions`
and recorded in its TSDoc that `_findStream` was left out. This removal subsumes
that divergence rather than fixing it — there is nothing left to fix it for.)

Rather than manufacture a caller to justify three implementations, the method is
retired. If a cursor-based read is wanted, it should arrive **with** the caller
that needs it, so the contract can be shaped by a real requirement instead of
being reverse-engineered from a doc comment nobody could test.

**Migration.**

| Wrote | Write instead |
| --- | --- |
| `for await (const row of driver.findStream(obj, q)) { … }` | page `driver.find(obj, { ...q, limit, offset })` in a loop |
| `findStream(…) { … }` on your own driver | delete the method (see below) |
| `findStream() { throw new Error('ni'); }` in a test double | delete the line |

Paging `find()` is not a downgrade from what `findStream` actually did: on SQL
and memory it is strictly better (bounded pages instead of one full
materialisation), and the paged read is the one with an **enforced** guarantee —
`IDataDriver.find` requires a total order across the whole walk, checked by the
shared `PAGINATION_CASES` / `PAGINATION_UNORDERED_CASES` fixtures in
`data/pagination-conformance.ts`. `findStream` never had a conformance case at
all.

**Driver authors: nothing breaks on you.** An implementation left in place still
compiles — an extra method is not an error on a class or a widened object — it is
simply never reached, so deleting it is cleanup you can do whenever. The break is
on the **caller** side: `driver.findStream(...)` no longer type-checks, and there
were no callers.

**No tombstone, deliberately.** The other v17 retirements tombstone their key so
authoring it fails loudly with a prescription. That would be noise here.
`DriverInterfaceSchema` describes a contract that code *implements*; nothing in
either repository ever ran a driver object through `.parse()`, so a
`retiredKey()` there would carry its prescription to no one. The channel that can
carry it is `tsc`, and `tsc` reports it where it is actionable — at a call site.
The key is removed from the schema and from `IDataDriver`, and the retirement is
registered as the `data-driver-find-stream-retired` semantic entry in the
protocol-17 chain step (ADR-0087 D3), so `spec-changes.json`, the generated
upgrade guide and the `spec_changes` MCP tool all carry it. There is no
`os migrate meta` step: a driver is code, never stack metadata, so the chain has
no source to rewrite.

**Left standing on purpose:** `DriverCapabilities.streaming`, the capability flag
whose only referent was this method. It has no readers either (and the values
written into it were already wrong — `SqlDriver` declared `streaming: false`
while implementing `findStream`, `InMemoryDriver` declared `true` for the
copy-everything version), but removing a key from the capabilities literal breaks
every driver that writes it, third-party included, and the same audit should
cover the other ~30 flags in one pass rather than one at a time. Tracked as
#4634.
1 change: 0 additions & 1 deletion docs/design/driver-turso.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ This does NOT require changes to existing client packages — it would be a new,
| `getPoolStats()` | 🟡 | Concurrency tracking (no traditional pool) |
| `execute()` | ✅ | `client.execute(sql, args)` |
| `find()` | ✅ | SQL SELECT with QueryAST→SQL compiler |
| `findStream()` | 🟡 | Cursor-based pagination (no native streaming) |
| `findOne()` | ✅ | `SELECT ... LIMIT 1` |
| `create()` | ✅ | `INSERT INTO ... RETURNING *` |
| `update()` | ✅ | `UPDATE ... WHERE id = ? RETURNING *` |
Expand Down
5 changes: 5 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ The datasource close-out also graduates the four legacy `datasource.config` spel

The `script` flow node converges on its one real path (#4343). It had four ways to name what it ran and only one of them ran anything: `config.actionType: 'email' | 'slack'` were logger-backed stubs that wrote a line, reported success and delivered nothing under any configuration — with `config.template` / `.recipients` / `.variables` feeding a message no channel ever sent; inline `config.script` was recognized and never executed (the built-in runtime has no server-side JS sandbox), so the node warned and no-op'd; and every other `actionType` value was shorthand for a registered-function name, a second spelling of `config.function`. All five keys are retired and `function` becomes required, which is also what finally made the contract PARSEABLE: while the legal key set depended on `actionType`, a flat parse would either reject valid shapes or wave everything through, so `script` (with `subflow`) now runs through the same execute-time contract parse #4277 gave the flat builtins. A shorthand `actionType` CONVERTS into `function` — that is what it meant — unless `function` is already set, in which case it was dead metadata the executor never reached. The other four are dropped outright: nothing read them, so there is no value to preserve, and rebuilding the intent is an authoring decision the tombstones prescribe per branch (a `notify` node for mail — it delivers through the messaging service, the in-app inbox by default and real email once `@objectstack/plugin-email` is installed; a `connector_action` with the Slack connector, or an `http` node posting to a webhook, for Slack; a registered function for an inline body). Retired from the load path for the same reason as the rest: absorbing `actionType: 'email'` silently would let an author keep believing the flow sends mail.

The same audit reaches the driver contract itself: `IDataDriver.findStream` is removed (#4484). It was REQUIRED — every driver and every test double had to implement it — and documented as the read "optimized for large datasets to avoid memory overflow", while two of its three implementations awaited `find()` for the whole result set and then yielded it row by row, reaching exactly the peak it promised to avoid; the third streamed for real but was the one read in that driver that skipped `buildFindOptions`, so it dropped `query.fields`. Nothing anywhere called it, which is why a contract method could carry an inverted guarantee for this long and why ~20 test doubles could satisfy it by throwing `not implemented`. Paged `find()` is the read that exists and is enforced (its total-order guarantee is checked by the shared pagination-conformance cases); a cursor-based read is worth building when a caller asks for one, which is the honest order. A TS/API surface, never stored — one semantic TODO for driver authors, no source rewrite, and no tombstone: `DriverInterfaceSchema` describes a contract that code IMPLEMENTS and nothing ever `.parse()`d a driver, so tsc is the only channel that could carry the prescription, and it carries it where it matters — at a call site.

### Mechanical (applied for you)

| Conversion | Surface | Change | Load window |
Expand Down Expand Up @@ -230,6 +232,9 @@ The `script` flow node converges on its one real path (#4343). It had four ways
- **`workflow-service-slot-retired`** — `CoreServiceName 'workflow' / IWorkflowService / WorkflowProtocol / discovery routes.workflow / RestApiRouteCategory workflow` → the live mechanisms the slot only ever pointed at: `state_machine` validation rules for record state machines, approval flow nodes on the approvals runtime (ADR-0019) for approvals, lifecycle hooks + `record_change` flows (service-automation) for record-triggered automation
- Why not automatic: The workflow slot was declared end to end and implemented nowhere: no code in either repository ever registered or resolved it (ADR-0115 Evidence 5 — the only touches were plugin-dev's retired stub probe and the generic discovery walk), no implementation of any WorkflowProtocol method ever existed, and no host ever mounted `/api/v1/workflow` (the pre-#3586 DEFAULT_DISPATCHER_ROUTES listed it among routes that never existed). Every part of it was ADR-0078's silently-inert declaration: a CoreServiceName nothing filled, a contract nothing implemented, a protocol nothing served, a discovery route field no builder could truthfully populate. These are TS/API surfaces and a discovery RESPONSE field — never stored in stack metadata, so there is no source for the chain to rewrite; consumers of the deleted types move their imports themselves. ADR-0049 / ADR-0078, #4451.
- Done when: No import of IWorkflowService, WorkflowProtocol or the Get/WorkflowState/Config/Transition types resolves; no code calls getService('workflow') or reads discovery `routes.workflow` / `services.workflow`; record state machines, approvals and record-triggered automation go through the replacement mechanisms. Discovery output on a default boot is unchanged (the slot was always reported unavailable; now it is simply absent).
- **`data-driver-find-stream-retired`** — `contracts.IDataDriver.findStream / data.DriverInterfaceSchema.findStream` → find() with limit/offset — the paged read whose determinism IS enforced (IDataDriver.find, data/pagination-conformance.ts)
- Why not automatic: `findStream` was a REQUIRED contract method documented as "optimized for large datasets to avoid memory overflow", and in two of its three implementations it delivered the opposite: `SqlDriver` and `InMemoryDriver` both awaited `find()` for the ENTIRE result set and then yielded it row by row, so the peak memory a caller was promised protection from was already reached before the first yield. The third (`MongoDBDriver._findStream`) did walk a cursor, but it was the one read path in that driver never routed through `buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently discarded `query.fields`. None of it was ever observed, because the method had NO caller in either repository: the engine exposes no stream entry, and the REST export, import and bulk-read paths all go through `find()`. The ~20 driver test doubles that existed only to satisfy a required method almost all threw `not implemented`, and nothing ever noticed — which is the proof, not the anecdote. Being REQUIRED, it also taxed every new driver and every test double with an implementation of a capability the platform does not have. Rather than build a caller to justify three implementations, the method is retired; a real cursor-based read should return WITH the caller that needs it (ADR-0049 enforce-or-remove). This is a TS/API contract surface — a driver is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone either: nothing ever ran a driver object through `DriverInterfaceSchema.parse()`, so a prescription there would have no one to reach. The enforced channel is tsc, and it points at callers. ADR-0049 / ADR-0078, #4484.
- Done when: No code calls `driver.findStream(...)`; large reads page through `find()` with `limit`/`offset` (which guarantees a total order across the whole walk) or go through the export surface. Drivers and test doubles no longer implement the method — one left behind still compiles and is simply never reached, so removing it is cleanup rather than a break, while a CALLER of it no longer type-checks.

---

Expand Down
1 change: 0 additions & 1 deletion packages/metadata/src/loaders/database-loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ function createMockDriver(): IDataDriver {
return Promise.resolve(null);
}),

findStream: vi.fn(),

create: vi.fn().mockImplementation((tableName: string, data: Record<string, unknown>) => {
const table = getTable(tableName);
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/datasource-mapping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const createMockDriver = (name: string) => ({
bulkUpdate: async () => [],
bulkDelete: async () => {},
execute: async () => ({}),
findStream: async function* () {},
upsert: async (obj: string, data: any) => ({ id: '1', ...data }),
beginTransaction: async () => ({}),
commit: async () => {},
Expand Down
2 changes: 0 additions & 2 deletions packages/objectql/src/engine-aggregate-having.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ function makeNativeDriver(rows: any[]) {
supports: {},
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find() { return rows.slice(); },
findStream() { throw new Error('ni'); },
async findOne() { return rows[0] ?? null; },
async create(_o: string, d: any) { return d; },
async update(_o: string, _id: string, d: any) { return d; },
Expand Down Expand Up @@ -66,7 +65,6 @@ function makeRawDriver(rows: any[]) {
supports: {},
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find() { return rows.slice(); },
findStream() { throw new Error('ni'); },
async findOne() { return rows[0] ?? null; },
async create(_o: string, d: any) { return d; },
async update(_o: string, _id: string, d: any) { return d; },
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-aggregate-timezone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ function makeBucketingDriver(rows: any[]) {
supports: { queryDateGranularity: { day: true, week: true, month: true, quarter: true, year: true } },
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find() { return rows.slice(); },
findStream() { throw new Error('ni'); },
async findOne() { return rows[0] ?? null; },
async create(_o: string, d: any) { return d; },
async update(_o: string, _id: string, d: any) { return d; },
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-ambient-transaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ function makeRecordingDriver() {
seen.find.push({ object, transaction: options?.transaction });
return Array.from(storeFor(object).values());
},
findStream() { throw new Error('not implemented'); },
async findOne(object: string) {
for (const r of storeFor(object).values()) return r;
return null;
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-audit-anchor-write.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ function makeMemoryDriver() {
async find(object: string, ast: any) {
return Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where));
},
findStream() { throw new Error('not implemented'); },
async findOne(object: string, ast: any) {
for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r;
return null;
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-autonumber-batch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ function makeDriver() {
name: 'memory', version: '0.0.0', supports: {},
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find(object: string) { return Array.from(storeFor(object).values()); },
findStream() { throw new Error('ni'); },
async findOne() { return null; },
async create(object: string, data: Record<string, unknown>) {
n += 1;
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-bulk-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ function makeDriver(opts: { bulkCreate?: (object: string, rows: any[]) => Promis
name: 'memory', version: '0.0.0', supports: {},
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find(object: string) { return Array.from(storeFor(object).values()); },
findStream() { throw new Error('ni'); },
async findOne() { return null; },
async create(object: string, data: Record<string, unknown>) {
n += 1;
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-cascade-delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ function makeMemoryDriver() {
name: 'memory', version: '0.0.0', supports: {},
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); },
findStream() { throw new Error('ns'); },
async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
async create(o: string, data: Record<string, unknown>) {
nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ function makeMemoryDriver() {
// audit's bounded-scan reporting untestable AND looser than production.
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
},
findStream() { throw new Error('not implemented'); },
async findOne(object: string, ast: any) {
for (const r of peek(object).values()) if (matches(r, ast?.where)) return r;
return null;
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-default-value-tokens.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ function makeMemoryDriver() {
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
async execute() { return null; },
async find(object: string) { return Array.from(storeFor(object).values()); },
findStream() { throw new Error('not implemented'); },
async findOne(object: string) { return storeFor(object).values().next().value ?? null; },
async create(object: string, data: Record<string, unknown>) {
nextId += 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ function makeDriver(name: string, connect: () => Promise<void>) {
async checkHealth() { return connected; },
async execute() { return null; },
async find() { return []; },
findStream() { throw new Error('ni'); },
async findOne() { return null; },
async create(_o: string, data: Record<string, unknown>) { return { id: 'r_1', ...data }; },
async update(_o: string, id: string, data: Record<string, unknown>) { return { ...data, id }; },
Expand Down
1 change: 0 additions & 1 deletion packages/objectql/src/engine-driver-health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ function makeDriver(name: string, checkHealth?: () => Promise<boolean>) {
async connect() {}, async disconnect() {},
async execute() { return null; },
async find() { return []; },
findStream() { throw new Error('ni'); },
async findOne() { return null; },
async create(_o: string, data: Record<string, unknown>) { return { id: 'r_1', ...data }; },
async update(_o: string, id: string, data: Record<string, unknown>) { return { ...data, id }; },
Expand Down
Loading
Loading