Skip to content

Commit d3df3ba

Browse files
test(objectql): bulk-write-per-row-hooks runs under the 10 s budget (#23182 row 5) (#23779)
Part of #23182 (row 5) Row 5 of the test-budget card: `packages/objectql/src/bulk-write-per-row-hooks.test.ts`, project `local`, 16.1 s in the hourly battery's reading (run 38325245250). This PR is that file's fix and nothing else; the card stays open for its other rows. Test-only: one file, +37 / -15, no production file, no changeset (`skip-changeset` applied). ## Where the time went (measured, not guessed) The file's test bodies cost 3.1 to 4.0 s on a quiet local box, and the module line is the number the budget reads. Instrumenting `seedTasks` inside the real file gave, in file order, 453, 392, 636, 585 and 687 ms for the five calls that push 10 001 (or 10 000) rows through `engine.insert`: 2.75 s of 3.69 s, about 74 percent. Those five calls belong to the ceiling cases (section 4 and D6). The last three are slower than the first two as the heap climbs from 481 to 604 MB across them. The remaining cost is the case that admits a batch exactly at the ceiling (about 0.5 to 0.7 s: 10 000 rows, so 20 000 real per-row hook dispatches) and about 0.25 s for the other 58 cases. There is no sleep, no timer, no wait and no per-test kernel boot in this file; the cost is row volume through `engine.insert`. ## What changed The ceiling is judged on what the driver's `find` returns, so how those rows got into the stub store is not what the five cases pin. The stub driver gains a synchronous `seed(object, rows)` that stores the very rows `create()` stores (`create` and `seed` now share one `put`), and a helper `seedCeilingRows(driver, count, status)` lays down the matched set. The five ceiling cases seed through it instead of `engine.insert`. Every other seed in the file (2 to 4 rows) still goes through `engine.insert`. Why the rows are the same ones: the engine adds nothing to a row on insert (a two-row probe gave `{title, status, id}` from `engine.insert` and from the driver alike). One-off check, not committed: for 10 001 `todo` rows, 10 000 `todo` rows and 10 001 `stale` rows, `driver.find` after `engine.insert` and after `seedCeilingRows` returned deep-equal arrays, and `engine.count` read the same total. ## Before / after, wall of the file (module line, default reporter, file alone, `--maxWorkers=2`, shared box, three runs each) | reading | base file | this branch | |:--|:--|:--| | re-taken after a container restart, head eb84ae2 | 3149 / 3368 / 3426 ms | 963 / 871 / 837 ms | | before the restart | 3987 / 3905 / 3598 ms (and 3635 ms in a one-off) | 962 / 971 / 940 ms | | CI-shaped package run, before the restart: whole `local` project, 429 files, `--maxWorkers=8` on 4 cores, the base file copied under a temporary name into the same run | 8212 ms | 1780 ms | Same 59 tests, same titles, same order. Slowest case after: "ADMITS a batch exactly AT the ceiling, in both phases", 682 ms (its 20 000 dispatches are the behaviour it pins, so it cannot shrink without weakening the pin); the next slowest is 50 ms. The card's 16.1 s was the slowest of 20 CI runs, about 4.4 times this box's 3.6 s reading; the same ratio puts this branch near 4 s in CI. That is a projection, not a measurement: NOT MEASURED until the next refresh PR's list is read. The single-file run's own `Duration` line (12 to 17 s) is dominated by the cold import and transform phase (about 11 s), which is on no module line, the same shape rows 1 and 4 noted. ## Pins preserved No test deleted, no `.skip` / `.todo`, no timeout touched (the `60_000` on the at-ceiling case is unchanged), no project change, every assertion and every row count unchanged (10 001 and 10 000 still come from `MAX_BULK_PER_ROW_HOOK_ROWS`). Per-row hooks on bulk writes are still proven per row: the before and after hook counts at the ceiling are still asserted at 10 000 each. ## Ablation (on-disk proof via `scripts/ablation-replace.mjs`, wrap mode, restore proven: blob equals HEAD and `git diff HEAD` empty) Each leg ran the branch file and a temporary copy of the base file in one vitest invocation under the same mutation, and compared red sets by full test name. | leg | mutation | base file red | branch file red | red sets | |:--|:--|:--|:--|:--| | control | none | 0 / 59 | 0 / 59 | identical | | A | `engine.ts` ceiling check never refuses (`if (verdict.kind) return;`) | 3 / 59 | 3 / 59 | identical: the section 4 refusal, the D6 before-update refusal, the D6 before-delete refusal | | B | `engine.ts` ceiling check also refuses at the ceiling (a guard that refuses once `matched` reaches the limit) | 1 / 59 | 1 / 59 | identical: the at-ceiling admit case | | C | `engine.ts` predicate-update before-hooks never dispatched (`rows.length > 99999999`) | 13 / 59 | 13 / 59 | identical | | D | the new helper seeds one row short (`count - 1`) | n/a | 5 / 59 | the five ceiling cases, so none of them is satisfied by an empty or short set | The subject (`engine.ts`) is imported by relative source path, so there is no dist leg; the mutation reached disk (anchor 1 to 0, blob changed) and every restore was verified. ## Verification (head eb84ae2) - `pnpm --filter @objectstack/objectql typecheck`: exit 0, including `check:test-typecheck` (39 files / 233 errors / 64 pinned signatures, unchanged; 0 errors in this file; the file is in `tsconfig.test.json`'s program, which is the one that covers it, and not in the default `tsconfig.json` program). Re-taken after the final edit. - eslint `--no-inline-config` on the changed file: 0 errors, 0 warnings (1 file). - Whole `local` project of the package, before the restart, on this change: 429 files / 8222 tests passed (that count includes the temporary base copy of this file). - `node scripts/pm/dispatch-gates.mjs --commands`: 53 derived, 53 run, all exit 0; `--ran` reconciliation: 53 derived, 53 run, 0 NOT-MEASURED, 0 UNRUN. Beyond that, the 11 declared wide-population families and the 52 further artifact-roster families (one of the 53 roster entries is also in the commands list): 63 commands run, 59 exit 0. Four roster gates need a pull-request or workflow-event context and exited non-zero with their own "NOT MEASURED / NOT WIRED, no PR context" text, so they judged nothing in that run: `check-closing-target-claim`, `check-partof-closing-keyword`, `check-single-claim-paths`, `pm/check-governed-queue-guard`. `check-partof-closing-keyword` was then run on this body's own text (`PR_BODY` set): exit 0. The other three stay NOT MEASURED locally; CI supplies their context. `check:dual-build-cjs-loads` and `check:lean-entry-closure` first exited 3 (no dist); after a full workspace build (75 of 75 tasks) both exited 0. ## Acceptance notes - The `60_000` timeout on the at-ceiling case could now be lowered; it is left alone on purpose (the card forbids touching timeouts, and a lowered one is a separate decision). - `engine.insert` costs about 40 microseconds a row on a batch of 10 000 (about 0.4 s), growing as the heap fills. That is production cost, an optimisation and not a defect, so no card. - The pin for this row is the next refresh PR's list no longer naming the file; that read is not available yet. ## Readings by provenance A container restart killed the first run mid-flow (the worktree and commit survived). Re-taken after it: the before/after module-line runs in the first table row, typecheck, eslint, the full workspace build and the whole gate run on head eb84ae2. From before the restart and still valid because the head did not move: the first three-run set, the in-file seed instrumentation, the CI-shaped package run, the equivalence check and all ablation legs (their JSON and restore proofs are on disk). --- _Generated by [Claude Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent a69a40a commit d3df3ba

1 file changed

Lines changed: 37 additions & 15 deletions

File tree

‎packages/objectql/src/bulk-write-per-row-hooks.test.ts‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,10 @@ describe('[#5038] the matched row set is read ONCE', () => {
296296
describe('[#5038] an oversized matched set is refused, never silently downgraded', () => {
297297
it('rejects past the ceiling, naming the count, the limit and the routes out', async () => {
298298
const fired: string[] = [];
299-
const { engine } = await boot([hook('per_row', 'afterUpdate', () => { fired.push('x'); })]);
299+
const { engine, driver } = await boot([hook('per_row', 'afterUpdate', () => { fired.push('x'); })]);
300300

301301
const over = ObjectQL.MAX_BULK_PER_ROW_HOOK_ROWS + 1;
302-
await seedTasks(engine, Array.from({ length: over }, (_, i) => ({ title: `t${i}`, status: 'todo' })));
302+
seedCeilingRows(driver, over, 'todo');
303303

304304
const err = await engine
305305
.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any)
@@ -322,9 +322,9 @@ describe('[#5038] an oversized matched set is refused, never silently downgraded
322322
});
323323

324324
it('does not apply to an object with no after-hooks — a big batch still writes', async () => {
325-
const { engine } = await boot([]);
325+
const { engine, driver } = await boot([]);
326326
const over = ObjectQL.MAX_BULK_PER_ROW_HOOK_ROWS + 1;
327-
await seedTasks(engine, Array.from({ length: over }, (_, i) => ({ title: `t${i}`, status: 'todo' })));
327+
seedCeilingRows(driver, over, 'todo');
328328

329329
const affected = await engine.update(
330330
'task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any,
@@ -870,8 +870,8 @@ describe('[#5574 / D6] one ceiling, BOTH phases, checked before the first dispat
870870

871871
it('refuses an over-ceiling batch on the BEFORE phase — zero handlers, zero rows written', async () => {
872872
const fired: string[] = [];
873-
const { engine } = await boot([hook('per_row_before', 'beforeUpdate', () => { fired.push('x'); })]);
874-
await seedTasks(engine, Array.from({ length: over }, (_, i) => ({ title: `t${i}`, status: 'todo' })));
873+
const { engine, driver } = await boot([hook('per_row_before', 'beforeUpdate', () => { fired.push('x'); })]);
874+
seedCeilingRows(driver, over, 'todo');
875875

876876
const err = await engine
877877
.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } })
@@ -890,8 +890,8 @@ describe('[#5574 / D6] one ceiling, BOTH phases, checked before the first dispat
890890

891891
it('refuses an over-ceiling bulk DELETE on the before phase too', async () => {
892892
const fired: string[] = [];
893-
const { engine } = await boot([hook('per_row_before', 'beforeDelete', () => { fired.push('x'); })]);
894-
await seedTasks(engine, Array.from({ length: over }, (_, i) => ({ title: `t${i}`, status: 'stale' })));
893+
const { engine, driver } = await boot([hook('per_row_before', 'beforeDelete', () => { fired.push('x'); })]);
894+
seedCeilingRows(driver, over, 'stale');
895895

896896
const err = await engine
897897
.delete('task', { multi: true, where: { status: 'stale' } })
@@ -908,12 +908,12 @@ describe('[#5574 / D6] one ceiling, BOTH phases, checked before the first dispat
908908
// refuse honest batches, which is the failure nobody reports as a bug.
909909
const beforeFired: string[] = [];
910910
const afterFired: string[] = [];
911-
const { engine } = await boot([
911+
const { engine, driver } = await boot([
912912
hook('pre', 'beforeUpdate', () => { beforeFired.push('x'); }),
913913
hook('post', 'afterUpdate', () => { afterFired.push('x'); }),
914914
]);
915915
const at = MAX_BULK_PER_ROW_HOOK_ROWS;
916-
await seedTasks(engine, Array.from({ length: at }, (_, i) => ({ title: `t${i}`, status: 'todo' })));
916+
seedCeilingRows(driver, at, 'todo');
917917

918918
const affected = await engine.update(
919919
'task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any,
@@ -1186,6 +1186,24 @@ async function seedTasks(engine: ObjectQL, rows: Record<string, unknown>[]): Pro
11861186
return Array.isArray(written) ? written : [written];
11871187
}
11881188

1189+
/**
1190+
* The matched set for the ceiling cases (section 4, D6): `count` task rows, all
1191+
* in `status`, written straight into the stub driver's store.
1192+
*
1193+
* Why not `seedTasks`: the ceiling is judged on what the driver's `find`
1194+
* returns, so how those rows got into the store is not what these cases pin —
1195+
* and pushing 10 001 rows through `engine.insert` cost about 40 µs a row, five
1196+
* times over (about 3 s of this file's 3.6 s of wall, on a quiet box). The
1197+
* stub's `create` is the only thing the engine's insert path ends in, and the
1198+
* engine adds nothing to a row on the way (a seeded row and an `engine.insert`
1199+
* one are the same `{ title, status, id }`), so `seed` lays down the very rows
1200+
* `seedTasks` would. Small seeds elsewhere in this file still go through
1201+
* `engine.insert`.
1202+
*/
1203+
function seedCeilingRows(driver: any, count: number, status: string): void {
1204+
driver.seed('task', Array.from({ length: count }, (_, i) => ({ title: `t${i}`, status })));
1205+
}
1206+
11891207
function makeStubDriver(): any {
11901208
const stores = new Map<string, Map<string, Record<string, unknown>>>();
11911209
const storeFor = (o: string) => {
@@ -1194,6 +1212,11 @@ function makeStubDriver(): any {
11941212
return s;
11951213
};
11961214
let nextId = 0;
1215+
const put = (o: string, data: Record<string, unknown>) => {
1216+
nextId += 1;
1217+
const id = (data.id as string) ?? `r_${nextId}`;
1218+
const row = { ...data, id }; storeFor(o).set(id, row); return row;
1219+
};
11971220
const matches = (row: Record<string, unknown>, where: any): boolean => {
11981221
if (!where || typeof where !== 'object') return true;
11991222
for (const [k, v] of Object.entries(where)) {
@@ -1217,11 +1240,10 @@ function makeStubDriver(): any {
12171240
return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
12181241
},
12191242
async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
1220-
async create(o: string, data: Record<string, unknown>) {
1221-
nextId += 1;
1222-
const id = (data.id as string) ?? `r_${nextId}`;
1223-
const row = { ...data, id }; storeFor(o).set(id, row); return row;
1224-
},
1243+
async create(o: string, data: Record<string, unknown>) { return put(o, data); },
1244+
/** Store `rows` exactly as `create` would, skipping the engine's insert path
1245+
* (see `seedCeilingRows`). Synchronous: nothing here awaits. */
1246+
seed(o: string, rows: Record<string, unknown>[]) { for (const r of rows) put(o, r); },
12251247
async update(o: string, id: string, data: Record<string, unknown>) {
12261248
d.updateCalls += 1;
12271249
const s = storeFor(o); const cur = s.get(id); if (!cur) return null;

0 commit comments

Comments
 (0)