Skip to content

perf(seed,import): route bulk writes through the engine's batch insert path - #2680

Merged
os-zhuang merged 2 commits into
mainfrom
claude/seed-data-import-bulk-insert-7xw1uc
Jul 8, 2026
Merged

perf(seed,import): route bulk writes through the engine's batch insert path#2680
os-zhuang merged 2 commits into
mainfrom
claude/seed-data-import-bulk-insert-7xw1uc

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Summary

Fixes#2678.

The seed loader (defineDataset replay) and the data-import runner both wrote records one at a time through the single-record engine path, even though ObjectQL's engine already has an efficient batched branch (driver.bulkCreate + parent-deduplicated summary recompute) for the array form of insert(). Over a remote DB this meant N round-trips + N summary recomputes instead of ~N/batch, and a transient network error on any one row was silently dropped with no retry (the 2026-07-06 HotCRM first-boot incident).

  • New shared helperbulkWrite / withTransientRetry (packages/core/src/utils/bulk-write.ts): batches rows through a caller-supplied batch-write function, retries a whole-batch transient failure (network blip / timeout) with exponential backoff, and degrades to per-row writes (each itself retried) when a batch fails for a non-transient reason — so one bad row can't drop the other N-1.
  • SeedLoaderService.loadDataset() (packages/metadata-protocol/src/seed-loader.ts) buffers insert-mode records and flushes them in batches of 200 via the engine's array-form insert(). Datasets with a self-referencing field (e.g. employee.manager_id -> employee) keep the historical strictly-sequential per-record path, since a later record may need an earlier one's freshly-assigned id via insertedRecords — batching would defer that write past the point it's needed. Update-mode writes are wrapped in the same transient retry.
  • runImport() (packages/rest/src/import-runner.ts) buffers create-resolved rows and flushes them via protocol.createManyData() when the protocol supports it (falling back to the original per-row createData() call otherwise). Per-row result ordering is preserved via an index-addressed results array even though creates are now written out of their original per-row sequence.
  • Protocol.createManyData (packages/metadata-protocol/src/protocol.ts) now forwards context to engine.insert(), matching createData — previously it silently dropped tenant/security context, which is presumably why the issue called this endpoint "may be unwired".

Test plan

  • New unit tests for bulkWrite/withTransientRetry/defaultIsTransientError (packages/core/src/utils/bulk-write.test.ts): bulk-path write-count assertion, transient-retry, logical-error per-row degradation, index correlation across batches.
  • New integration-style tests for runImport()'s batching (packages/rest/src/import-runner-bulk.test.ts): batch write-count, transient retry, logical degrade, no-createManyData fallback, result ordering with interleaved updates.
  • Updated existing mocks/assertions in packages/runtime/src/seed-loader.test.ts and packages/runtime/src/http-dispatcher.test.ts that asserted the old N-calls-per-record behavior, now asserting the batched call.
  • pnpm test (via turbo) green across @objectstack/core, @objectstack/metadata-protocol, @objectstack/objectql, @objectstack/rest, @objectstack/runtime — 461+797+216+15+... tests passing, no regressions.
  • pnpm lint clean on all changed files.

Notes / follow-ups

  • Summary-recompute count is bounded by unique parents per batch (not across the whole load) — collapsing it to a single end-of-load pass is called out as optional in the issue and is a larger engine-level change; left out of scope here.
  • Import's findExisting/reference-resolution reads are unchanged (still per-row) — the issue's evidence and acceptance criteria are scoped to the write path.

🤖 Generated with Claude Code


Generated by Claude Code

…t path
Seed loader and data-import wrote records one at a time even though the
engine's array-form insert() already does the efficient thing (one
driver.bulkCreate round-trip + parent-deduplicated summary recompute).
Add a shared bulkWrite/withTransientRetry helper (@objectstack/core) that
batches writes, retries transient driver errors with backoff, and degrades
to per-row writes when a batch fails for a non-transient reason so one bad
row can't drop the rest. Wire it into SeedLoaderService.loadDataset() and
import-runner's runImport(), and fix Protocol.createManyData to forward
context to engine.insert() like createData already does.
Fixes#2678.
@vercel

vercelBot commented Jul 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJul 7, 2026 6:15pm

Request Review

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling size/l labels Jul 7, 2026
@github-actions

github-actionsBot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 6 package(s): @objectstack/core, @objectstack/metadata-protocol, @objectstack/objectql, @objectstack/driver-sql, @objectstack/rest, @objectstack/runtime.

37 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/actions-as-tools.mdx(via @objectstack/core)
  • content/docs/ai/knowledge-rag.mdx(via @objectstack/core)
  • content/docs/ai/natural-language-queries.mdx(via @objectstack/core)
  • content/docs/api/error-catalog.mdx(via @objectstack/rest)
  • content/docs/api/error-handling-server.mdx(via @objectstack/rest)
  • content/docs/api/index.mdx(via @objectstack/rest, @objectstack/runtime)
  • content/docs/api/wire-format.mdx(via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx(via @objectstack/runtime)
  • content/docs/automation/webhooks.mdx(via @objectstack/core)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/metadata-protocol, @objectstack/objectql)
  • content/docs/concepts/north-star.mdx(via packages/core, packages/runtime)
  • content/docs/data-modeling/drivers.mdx(via @objectstack/driver-sql, @objectstack/runtime)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/deployment/index.mdx(via @objectstack/runtime)
  • content/docs/deployment/migration-from-objectql.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx(via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx(via @objectstack/objectql, @objectstack/runtime)
  • content/docs/getting-started/glossary.mdx(via @objectstack/driver-sql)
  • content/docs/kernel/contracts/index.mdx(via @objectstack/core)
  • content/docs/kernel/runtime-services/examples.mdx(via @objectstack/core)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/kernel/services.mdx(via @objectstack/core, @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/runtime)
  • content/docs/permissions/authorization.mdx(via packages/rest)
  • content/docs/plugins/anatomy.mdx(via @objectstack/core, @objectstack/driver-sql)
  • content/docs/plugins/development.mdx(via @objectstack/core)
  • content/docs/plugins/index.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/rest)
  • content/docs/plugins/packages.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/driver-sql, @objectstack/rest, @objectstack/runtime)
  • content/docs/protocol/objectos/config-resolution.mdx(via @objectstack/core)
  • content/docs/protocol/objectos/http-protocol.mdx(via @objectstack/runtime)
  • content/docs/protocol/objectos/index.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/driver-sql, @objectstack/runtime)
  • content/docs/protocol/objectos/lifecycle.mdx(via @objectstack/core, @objectstack/driver-sql, @objectstack/runtime)
  • content/docs/protocol/objectos/plugin-spec.mdx(via @objectstack/core)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)
  • content/docs/releases/implementation-status.mdx(via @objectstack/core, @objectstack/objectql, @objectstack/driver-sql, @objectstack/rest, @objectstack/runtime)
  • content/docs/releases/v9.mdx(via @objectstack/objectql)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…) does
SqlDriver.bulkCreate() never ran the id/_id normalization create() applies,
so a row inserted via the array path without a driver-native id default
landed with id: NULL. This bulk path was rarely exercised before #2678
routed real seed/import data through it at scale, surfacing the gap (the
showcase invoice-line dogfood test failed: product/invoice references
resolved to NULL because the referenced object's bulk-inserted rows had no
id to record). bulkCreate() now assigns nanoid(DEFAULT_ID_LENGTH) per row
exactly like create() does.
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Investigated the CI failures from the first push:

Real regression (fixed, pushed in 6e3f21d):Dogfood Regression Gate / Test Core failed on showcase-invoice-seed-isolation.dogfood.test.ts — a seeded invoice line's product lookup resolved to null. Root cause: SqlDriver.bulkCreate() (packages/plugins/driver-sql/src/sql-driver.ts) never assigned a client-side id to a row missing one, unlike create() — so a bulk-inserted showcase_product row landed with id: NULL, and nothing downstream could resolve a reference to it. This bulk path was rarely exercised with real multi-row data before this PR; routing seed data through it at scale surfaced the gap. Fixed by mirroring create()'s id/_id normalization per row in bulkCreate(). Verified against a main-branch worktree that the test passes there today (i.e. this is not a pre-existing failure) and now passes on this branch too, along with the full driver-sql (248 tests) and dogfood (191 tests) suites.

Unrelated, pre-existing (not fixed here): CodeQL flagged 2 high-severity "polynomial regex" alerts in packages/objectql/src/validation/record-validator.ts and rule-validator.ts (EMAIL_RE). Confirmed via git diff main that neither file is touched by this PR's diff — these are latent findings in unrelated, unmodified code. Filed separately as #2681 rather than expanding this PR's scope.

Continuing to watch CI.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review July 8, 2026 05:56
@os-zhuang
os-zhuang merged commit 21420d9 into mainJul 8, 2026
15 of 16 checks passed
@os-zhuang
os-zhuang deleted the claude/seed-data-import-bulk-insert-7xw1uc branch July 8, 2026 05:57
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf/reliability: seed + data-import bypass the engine's bulk-insert path → N round-trips, N summary recomputes, no transient retry

2 participants

@os-zhuang@claude