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
38 changes: 38 additions & 0 deletions .changeset/driver-conformance-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
"@objectstack/driver-sqlite-wasm": patch
---

test(drivers): the "held to by a gate" claim now has a gate behind it (#4363)

Three changesets — filter combinator semantics (#3774), temporal storage form
(ADR-0053), deterministic paged reads (objectui#3106 / #4363) — each introduced
a shared case-set in `@objectstack/spec/data` with some version of the claim
that a future driver "is held to this by a gate rather than by remembering it".

There was no gate. The case-sets are exports sitting in a package; nothing
obliged a driver to import them. Measured on `main`, the matrix had three holes:
`driver-sqlite-wasm` ran neither pagination case-set, and neither it nor
`driver-mongodb` ran the filter-logic one — including a hole in the very
case-set whose changeset made the claim.

`scripts/check-driver-conformance.mjs` (`pnpm check:driver-conformance`, wired
into lint.yml's required job) makes the hole the failure. Every
(driver × case-set) cell is covered — some file under the package's `src/`
imports *and drives* the case-set's marker export — or carries a measured
DEBT/EXEMPT entry, reconciled in both directions. A third direction, CLASSIFIED,
holds the other end: a new `*-conformance.ts` fixture nobody classified fails
the run rather than starting life uncovered, which is the direction that
actually rots (#4203). It caught an unclassified `TEMPORAL_TIME_CASES` on its
first run.

`driver-sqlite-wasm` gains the pagination suite the gate found missing. It
inherits `SqlDriver`'s ORDER BY construction, so nothing is re-implemented —
what the suite pins is that the clause survives a different *engine*: this
driver swaps knex's transport for a custom sql.js dialect that compiles,
executes and marshals every row through its own path, and a dialect that
reordered or dropped the trailing `ORDER BY id` would fail in no other suite.

The two filter-logic holes are ledgered as DEBT rather than fixed here, with
their reasons printed on every run and tracked in #4405. The mongodb row is the
substantive one: `translateFilter` is an independent FilterCondition backend —
the fifth, and the one #3774 never enrolled when it counted "the four".
55 changes: 55 additions & 0 deletions .changeset/keyset-batch-walks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
"@objectstack/types": patch
"@objectstack/objectql": patch
"@objectstack/service-storage": patch
"@objectstack/plugin-approvals": patch
"@objectstack/plugin-pinyin-search": patch
---

fix(batch): the background walks seek instead of counting, so they stop skipping rows (#4363)

#4363 made a single paged read a partition of its result set. It could not make
a *walk* one: seven background scans paged with a growing `offset` while writing
to the very rows they were reading, and an offset counts into a set those writes
are changing. Rows slide past the cursor and are never visited.

That is not a slow page in any of these — it is a wrong answer wearing the shape
of a clean run:

- **`rebuildApproverIndex`** built its desired state by walking
`sys_approval_request WHERE status = 'pending'` with no `orderBy` at all, then
**deleted** every index row that state did not explain. A skipped request
meant an approver silently dropped from someone's queue. (The loop beside it
ordered by `created_at` — not unique, so its pages were never a partition
either.)
- **`verifyFileReferences`** decides which files nothing references. A record it
never visits is reported as an unreferenced file.
- **`backfillFileReferences`** and the **pinyin companion backfill** rewrite
each row they read, so their own writes were shifting the set out from under
the cursor. Records were left unconverted and unsearchable by a run that
reported success.
- **`scanValueShapes`** exists to vouch that no stored value is off-shape, and
it opens a migration gate on that evidence.

All of them now go through `keysetWalk` (`@objectstack/types`): order by a
unique key, and seek past the last one instead of counting from the start. A
row's key does not move when the row is updated, and cannot be shifted when
another is deleted, so the walk is stable under exactly the mutation these
functions perform. It is also O(n) rather than O(n²/page) — measured on
Postgres over 2M rows, deep pages cost ~1.1 s by offset against ~0.09 s by seek.

One deliberate non-conversion: the REST **export** stream keeps its offset. It
honors a caller-chosen sort, and a keyset walk would have to re-order the export
by `id` to seek — changing what the user asked for to fix a cost. Its pages are
already a partition since #4363; only the depth cost remains.

`keysetWalk` merges the cursor with `$and` rather than spreading it into the
caller's filter, so a walk whose own `where` constrains the key column
(`{ id: { $in: [...] } }`) keeps that constraint instead of having it silently
overwritten. When a `max` cap is set it reads one row beyond the cap to tell
"the cap stopped us" from "the source ended exactly there" — without that, a
walk that read everything still reports `truncated`, and a caller acting on it
goes looking for rows that were never withheld.

The storage suites' fake engines now **throw** on an `offset` instead of serving
one, so the conversion is pinned rather than merely passing.
13 changes: 13 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,19 @@ jobs:
- name: Check every package is type-check covered or ledgered
run: pnpm check:type-check-coverage

# Conformance meta-gate (#4363): `spec/src/data/*-conformance.ts` holds the
# shared case-sets that exist so the independent drivers answer ONE
# standard, and three separate changesets claimed a future driver "is held
# to this by a gate rather than by remembering it". There was no gate —
# nothing obliged a driver to import them — and the matrix had three holes,
# one of them in the case-set whose changeset made the claim. Same shape as
# the ratchet above: every (driver × case-set) cell is covered or carries a
# measured DEBT/EXEMPT entry, reconciled in both directions, plus a
# CLASSIFIED direction so a NEW shared fixture cannot start life uncovered.
# Reads source files only; no build, sub-second.
- name: Check every driver runs the shared conformance cases
run: pnpm check:driver-conformance

- name: Type check (@objectstack/spec)
run: pnpm --filter @objectstack/spec exec tsc --noEmit

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"check:release-notes": "node scripts/check-release-notes.mjs",
"check:node-version": "node scripts/check-node-version.mjs",
"check:published-files": "node scripts/check-published-files.mjs --self-test && node scripts/check-published-files.mjs",
"check:type-check-coverage": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs"
"check:type-check-coverage": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs",
"check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs"
},
"keywords": [
"objectstack",
Expand Down
58 changes: 28 additions & 30 deletions packages/objectql/src/validation/scan-value-shapes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
* fact the validator does not recognise.
*/

import { keysetWalk } from '@objectstack/types';

import {
valueShapeViolation,
isScannableValueShapeField,
Expand Down Expand Up @@ -125,31 +127,21 @@ export async function scanValueShapes(

for (const object of scannedObjects) {
const fields = scannableFieldsOf(engine, object);
let offset = 0;

for (;;) {
let page: Array<Record<string, unknown>>;
try {
page = await engine.find(object, {
fields: ['id', ...fields.map((f) => f.name)],
limit: SCAN_PAGE_SIZE,
offset,
context: { ...SYSTEM_CTX },
});
} catch (err) {
// An object we cannot read is not an object we may vouch for. Record it
// AND truncate: the gate's claim is about all the data, so a hole in
// the evidence has to close it.
logger.warn(
`[value-shape] scan: cannot read ${object} (${(err as Error)?.message ?? err}) — ` +
'reported as unreadable; the gate stays closed',
);
unreadableObjects.push(object);
truncated = true;
break;
}
if (!page || page.length === 0) break;
// Seek by `id` (#4363) rather than counting from the start. This scan
// exists to vouch that no stored value is off-shape, and an offset walk
// cannot promise it visited every row — the same reason an unreadable
// object below closes the gate rather than being skipped quietly.
const walk = keysetWalk<Record<string, unknown>>(
(q) => engine.find(object, {
...q,
fields: ['id', ...fields.map((f) => f.name)],
context: { ...SYSTEM_CTX },
}),
{ pageSize: SCAN_PAGE_SIZE, max: maxPerObject },
);

try {
for await (const page of walk.pages()) {
for (const record of page) {
scannedRecords++;
for (const { name, def } of fields) {
Expand All @@ -173,14 +165,20 @@ export async function scanValueShapes(
}
}
}

offset += page.length;
if (page.length < SCAN_PAGE_SIZE) break;
if (offset >= maxPerObject) {
truncated = true;
break;
}
} catch (err) {
// An object we cannot read is not an object we may vouch for. Record it
// AND truncate: the gate's claim is about all the data, so a hole in
// the evidence has to close it.
logger.warn(
`[value-shape] scan: cannot read ${object} (${(err as Error)?.message ?? err}) — ` +
'reported as unreadable; the gate stays closed',
);
unreadableObjects.push(object);
truncated = true;
continue;
}
if (walk.truncated) truncated = true;
}

const list = [...findings.values()].sort((a, b) => b.count - a.count);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Deterministic paged reads for the wasm driver (objectui#3106,
* objectstack#4363) — the contract on `IDataDriver.find`, run against the
* shared `@objectstack/spec/data` cases like every other driver.
*
* `SqliteWasmDriver extends SqlDriver`, so the ORDER BY is *built* by inherited
* code and nothing here re-implements it. What this pins is the other half:
* the clause has to survive a different **engine**. This driver swaps knex's
* transport for a custom sql.js dialect (`Client_WasmSqlite`), which compiles
* and executes the statement and marshals every row back through its own path.
* A dialect that reordered, dropped or mis-bound the trailing `ORDER BY id`
* would produce exactly the failure the contract exists to rule out — full
* pages, real rows, one served twice and one never served — and it would fail
* in no other suite in the repo.
*
* That is the same reason the temporal suite next door exists: inheritance
* makes it easy to assume the behavior comes along for free, and the dialect
* seam is precisely where it might not.
*/

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import {
PAGINATION_ALL_IDS,
PAGINATION_CASES,
PAGINATION_ROWS,
PAGINATION_UNORDERED_CASES,
} from '@objectstack/spec/data';
import { SqliteWasmDriver } from './index.js';

describe('driver-sqlite-wasm — paged reads are a partition of the result set', () => {
let driver: SqliteWasmDriver;

beforeAll(async () => {
driver = new SqliteWasmDriver({ filename: ':memory:' });
await driver.initObjects([
{
name: 'ticket',
fields: {
status: { type: 'string' },
rank: { type: 'integer' },
name: { type: 'string' },
},
},
]);
for (const row of PAGINATION_ROWS) {
await driver.create('ticket', { ...row }, { bypassTenantAudit: true } as any);
}
});

afterAll(async () => {
await driver.disconnect();
});

const walk = async (pageSize: number, orderBy?: ReadonlyArray<{ field: string; order: 'asc' | 'desc' }>) => {
const seen: string[] = [];
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += pageSize) {
const page = await driver.find(
'ticket',
{ ...(orderBy ? { orderBy: [...orderBy] } : {}), limit: pageSize, offset } as any,
{ bypassTenantAudit: true } as any,
);
seen.push(...page.map((r: any) => String(r.id)));
}
return seen;
};

for (const testCase of PAGINATION_CASES) {
it(`visits every row exactly once — ${testCase.name}`, async () => {
const seen = await walk(testCase.pageSize, testCase.orderBy);
expect(seen).toHaveLength(PAGINATION_ALL_IDS.length);
expect(new Set(seen).size).toBe(PAGINATION_ALL_IDS.length);
expect([...seen].sort()).toEqual([...PAGINATION_ALL_IDS].sort());
});

it(`page boundaries are invisible — ${testCase.name}`, async () => {
const paged = await walk(testCase.pageSize, testCase.orderBy);
const whole = await driver.find(
'ticket',
{ orderBy: [...testCase.orderBy] } as any,
{ bypassTenantAudit: true } as any,
);
expect(paged).toEqual(whole.map((r: any) => String(r.id)));
});
}

for (const testCase of PAGINATION_UNORDERED_CASES) {
it(`visits every row exactly once with NO orderBy at all — ${testCase.name}`, async () => {
const seen = await walk(testCase.pageSize);
expect(seen).toHaveLength(PAGINATION_ALL_IDS.length);
expect(new Set(seen).size).toBe(PAGINATION_ALL_IDS.length);
expect([...seen].sort()).toEqual([...PAGINATION_ALL_IDS].sort());
});

it(`walks an unsorted read in id order — ${testCase.name}`, async () => {
// The fixture's ids are shuffled relative to insertion order, so this
// distinguishes "the inherited ORDER BY reached the wasm engine" from
// "sql.js happened to hand back insertion order".
expect(await walk(testCase.pageSize)).toEqual([...PAGINATION_ALL_IDS].sort());
});
}

it('leaves an UNPAGED unordered read alone — no sort is imposed on a caller who asked for none', async () => {
const rows = await driver.find('ticket', {} as any, { bypassTenantAudit: true } as any);
expect(rows.map((r: any) => r.id)).toEqual(PAGINATION_ROWS.map((r) => r.id));
});
});
3 changes: 2 additions & 1 deletion packages/plugins/plugin-approvals/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"@objectstack/formula": "workspace:*",
"@objectstack/metadata-core": "workspace:*",
"@objectstack/platform-objects": "workspace:*",
"@objectstack/spec": "workspace:*"
"@objectstack/spec": "workspace:*",
"@objectstack/types": "workspace:*"
},
"devDependencies": {
"@objectstack/objectql": "workspace:*",
Expand Down
43 changes: 24 additions & 19 deletions packages/plugins/plugin-approvals/src/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type ApprovalNodeConfig,
} from '@objectstack/spec/automation';
import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formula';
import { keysetWalk } from '@objectstack/types';
import {
ADMIN_FULL_ACCESS,
ORGANIZATION_ADMIN_GRANTS,
Expand Down Expand Up @@ -3112,37 +3113,41 @@ export class ApprovalService implements IApprovalService {
* the number of *pending* requests, not the request history.
*/
async rebuildApproverIndex(): Promise<{ requests: number; inserted: number; deleted: number }> {
// Desired state: every pending request's CSV entries.
// Both walks seek by `id` rather than counting from the start (#4363).
// The desired-state walk fed the deletion pass below, so a row it skipped
// was not a slow page — it was an approver silently dropped from someone's
// queue. An offset walk over `status = 'pending'` has no such guarantee:
// it slices an arrangement nothing holds steady, and this method is itself
// a writer against the same tables.
const desired = new Map<string, { approvers: Set<string>; org: string | null }>();
const PAGE = 500;
for (let offset = 0; ; offset += PAGE) {
const batch = await this.engine.find('sys_approval_request', {
where: { status: 'pending' },
const requests = keysetWalk<Record<string, any>>(
(q) => this.engine.find('sys_approval_request', {
...q,
fields: ['id', 'pending_approvers', 'organization_id'],
limit: PAGE, offset, context: SYSTEM_CTX,
});
const rows: any[] = Array.isArray(batch) ? batch : [];
context: SYSTEM_CTX,
}),
{ where: { status: 'pending' }, pageSize: PAGE },
);
for await (const rows of requests.pages()) {
for (const r of rows) {
desired.set(String(r.id), {
approvers: new Set(csvSplit(r.pending_approvers)),
org: r.organization_id ?? null,
});
}
if (rows.length < PAGE) break;
}

// Current state: read the whole index first (bounded by the live work
// queue), THEN mutate — deleting while paginating would shift the cursor.
// Current state: read the whole index first, THEN mutate. The seek makes
// that separation a belt rather than the only brace — `created_at` is not
// unique, so the previous ORDER BY could not make these pages a partition
// even before the deletes below shifted the rows underneath them.
const indexRows: any[] = [];
for (let offset = 0; ; offset += PAGE) {
const batch = await this.engine.find('sys_approval_approver', {
orderBy: [{ field: 'created_at', order: 'asc' }],
limit: PAGE, offset, context: SYSTEM_CTX,
});
const rows: any[] = Array.isArray(batch) ? batch : [];
indexRows.push(...rows);
if (rows.length < PAGE) break;
}
const index = keysetWalk<Record<string, any>>(
(q) => this.engine.find('sys_approval_approver', { ...q, context: SYSTEM_CTX }),
{ pageSize: PAGE },
);
for await (const rows of index.pages()) indexRows.push(...rows);
let inserted = 0; let deleted = 0;
const seen = new Map<string, Set<string>>();
for (const row of indexRows) {
Expand Down
Loading
Loading