Skip to content

fix: harden the bulk-write path — retries, idempotency, contracts, and summary visibility (#3147–#3152)#3161

Merged
os-zhuang merged 8 commits into
mainfrom
claude/evaluate-issues-3147-5152-1naxe6
Jul 18, 2026
Merged

fix: harden the bulk-write path — retries, idempotency, contracts, and summary visibility (#3147–#3152)#3161
os-zhuang merged 8 commits into
mainfrom
claude/evaluate-issues-3147-5152-1naxe6

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Summary

Six related bugs surfaced by the #2678 bulk-write rework's dedicated test pass. The batched seed/import + engine.insert(array) path added at-least-once retry and per-row degradation, but lacked the idempotency, return-value contracts, and failure-visibility that safe retry/degradation requires. Each is fixed in its own commit; the branch is meant to be reviewed commit-by-commit.

Fixes #3147, #3148, #3149, #3150, #3151, #3152.

Commits

Commit Issue Change
fix(core): validate writeBatch result contract #3151 (core) bulkWrite guards writeBatch's return: a short / long / non-array result now throws ERR_BULK_RESULT_MISMATCH → per-row degradation, instead of backfilling record: undefined as phantom success. Documents at-least-once delivery.
fix(seed,rest,core): wrap bare writes in transient retry #3150 Wraps the two un-retried write points — seed writeRecord/resolveDeferredUpdates, import's no-createManyData fallback — in withTransientRetry. defaultIsTransientError short-circuits definitive logical signatures (constraint/validation/unique/…) to non-transient.
fix(rest): import resolveRef flushes pending creates #3148 A later row can reference an earlier same-file CREATE: on a same-object miss, resolveRef flushes the pending-create buffer and retries once. A bare miss is no longer negatively cached.
fix(objectql): reject short/non-array bulkCreate results #3151 (engine) engine.insert(array) throws ERR_BULK_RESULT_MISMATCH if the driver returns the wrong record count, rather than padding afterInsert contexts / caller results with undefined.
fix(core,seed,rest): idempotent batch retry via attempt-aware recheck #3149 Threads a 1-based attempt counter into writeBatch/writeOne; on attempt > 1 seed rechecks by externalId and import by matchFields, inserting only missing rows — so a commit-then-lost-response retry can't duplicate a batch. Pure-insert imports stay at-least-once by documented contract.
fix(objectql,seed,rest): retry + surface summary recompute failures #3147 recomputeSummaries retries each parent's aggregate+update with backoff and collects failures; on exhaustion, insert/update/delete throw SummaryRecomputeError (ERR_SUMMARY_RECOMPUTE) carrying the written records. Seed/import recover it to success + warning (no re-write), identified by code to avoid an import cycle.
fix(objectql): assign autonumbers after validation #3152 Autonumbers move after validation, so a batch that dies in validation (then degrades per-row) consumes no sequence value → no number-range gaps. The remaining beforeInsert double-execution on a degraded batch is documented as at-least-once hook semantics (mitigation, per the issue's allowance).

Design notes (cross-issue interactions)

Tests

New/extended, all green (vitest):

  • core bulk-write.test.ts: contract-guard (short/long/non-array → degrade), attempt propagation, mixed constraint+network → non-transient.
  • objectql engine-bulk-contract.test.ts, engine-summary-retry.test.ts, engine-autonumber-batch.test.ts + extended summary-rollup.
  • metadata-protocol seed-loader-retry.test.ts: self-ref retry, commit-then-lost-response no-dup, summary-failure recovery.
  • rest import-runner-selfref.test.ts, import-runner-idempotency.test.ts + extended import-runner-bulk.

Verification

  • pnpm build — 71/71 (type-checks across all packages).
  • pnpm test — 131/131 task suites pass (full monorepo).

🤖 Generated with Claude Code


Generated by Claude Code

claude added 7 commits July 18, 2026 03:17
bulkWrite correlated writeBatch's return to input rows purely by
position, with no check that it returned one record per row. A driver
that returned fewer rows silently backfilled `record: undefined` and
reported phantom successes; a longer return dropped records; a non-array
return marked the whole batch ok with no records.

Guard the return right after the batch write succeeds: a short / long /
non-array result now throws ERR_BULK_RESULT_MISMATCH, which falls into
the existing per-row degradation path (each row re-attempted via
writeOne) instead of being trusted. The error is thrown outside
withRetry and its message avoids any transient signature, so the batch
is never retried on it.

Also document bulkWrite's at-least-once delivery semantics in the module
header, ahead of the idempotency work in #3149.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…it logical errors (#3150)

Two write points were left out of the bulk-write transient-retry rework,
so a single network blip dropped the row with no retry — while adjacent
paths retried:

- seed-loader `writeRecord` (the self-referencing sequential path) and
  `resolveDeferredUpdates` called engine.insert/update bare; now wrapped
  in withTransientRetry, matching the batched update path.
- import-runner's no-createManyData fallback called p.createData bare;
  now wrapped, matching the update path (L352) and the bulkWrite batch
  path.

Also harden defaultIsTransientError: a message carrying a definitive
logical signature (constraint / validation / required / unique / not
null / out of range / not allowed) is now classified non-transient even
when a transient keyword also appears (e.g. `CHECK constraint failed:
network_zone`), so we don't burn retries on a row that fails identically
every time.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…ve cache (#3148)

Batched CREATE rows buffer into pendingCreates and flush later, but
resolveRef only queried the DB — so a later row referencing an earlier
same-file CREATE always missed with reference_not_found and was dropped.
refCache compounded it by caching the empty miss, so even after a
progress-checkpoint flush landed the row, the name stayed pinned to the
cached miss.

resolveRef now, on a same-object miss with a non-empty buffer, flushes
pending creates and retries the lookup once (the buffered rows are all
earlier than the current one, so the flush is safe and idempotent). A
bare miss is no longer cached — only a definitive id/ambiguous verdict
is — so a name that misses early can still resolve once a later flush
creates it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
engine.insert(array) built afterInsert contexts by index over
rowHookContexts, so a driver bulkCreate that returned fewer records than
input rows padded the tail with `undefined` — afterInsert hooks ran with
`ctx.result === undefined` and the engine returned undefined entries,
corrupting seed externalId→id maps and import undo logs downstream.

Guard the driver result before the afterInsert loop: a batch whose
result is not an array of exactly rows.length records now throws
ERR_BULK_RESULT_MISMATCH, so the caller sees a real failure. Every
in-repo driver already returns one-per-row in order; this defends against
third-party drivers. The single-record path is untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…-key recheck (#3149)

bulkWrite's transient retry is at-least-once: when a driver commits a
batch but its response is lost (turso's fetch-based transport fails after
commit), the retry re-inserted the whole batch — 2 input rows became 4
stored rows, all reported success, each firing record-change and summary
recompute twice.

Rather than a generic dedup inside bulkWrite, thread a 1-based `attempt`
counter into writeBatch/writeOne and let each caller enforce batch-level
idempotency with its own natural key:

- core: withRetry passes the attempt number; writeBatch/writeOne gain a
  `{ attempt }` context (backward compatible — callers may ignore it).
- seed: on attempt > 1, recheck existing rows by externalId (reusing
  loadExistingRecords) and insert only the missing ones; reassemble the
  per-row result in order.
- import: on attempt > 1 with matchFields, recheck by matchFields and
  create only the missing rows. A short createManyData return is now
  surfaced as a failed batch (so degradation, which also rechecks, runs)
  instead of being padded. A pure-insert import (no matchFields) has no
  natural key and stays at-least-once by documented contract.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…#3147)

recomputeSummaries wrapped each parent's aggregate+update in a try/catch
that only warned — a transient turso blip on the parent summary write
left the value silently stale, and a child batch insert still reported
full success.

Engine: retry each parent recompute with transient backoff
(withTransientRetry, injectable via engine.summaryRetryOptions); collect
per-parent failures so one bad parent doesn't abort the rest. When any
remain after retries, insert/update/delete throw SummaryRecomputeError
(code ERR_SUMMARY_RECOMPUTE) AFTER the realtime publish — the records
ARE written, so the error carries them on `written`.

Callers (identified by `code`, no objectql import — that would cycle):
seed and import recover ERR_SUMMARY_RECOMPUTE to success using `written`
rather than re-writing (which would duplicate), logging a warning; import
marks the affected rows `SUMMARY_RECOMPUTE_FAILED` while keeping them
created/updated. A direct API caller simply sees the thrown error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…-once hooks (#3152)

In the batch insert path, beforeInsert hooks and autonumber assignment
ran before per-row validation. When bulkWrite degrades a batch to per-row
(or retries it), the whole engine.insert re-runs, so a batch that dies in
validation had already consumed an autonumber sequence value for every
good row — leaving number-range gaps — and re-fired side-effecting
beforeInsert hooks on the retry.

Move autonumber assignment to after validation: a doomed attempt now
consumes no sequence value (required-validation already exempts
autonumber fields, and a driver-owned autonumber assigns nothing here, so
no validation rule can depend on the value — the reorder is safe). The
remaining beforeInsert double-execution on a degraded batch is documented
as at-least-once hook semantics on engine.insert (hooks must be
idempotent); root-causing it needs a partial-success engine API, out of
scope here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
@vercel

vercel Bot commented Jul 18, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jul 18, 2026 4:11am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/core, @objectstack/metadata-protocol, @objectstack/objectql, @objectstack/rest.

31 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)
  • 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)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/core, @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • 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)
  • content/docs/permissions/authorization.mdx (via packages/core)
  • content/docs/plugins/anatomy.mdx (via @objectstack/core)
  • 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/rest)
  • content/docs/protocol/kernel/config-resolution.mdx (via @objectstack/core)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/core, @objectstack/objectql)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/core)
  • content/docs/protocol/kernel/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/rest)
  • content/docs/releases/v12.mdx (via @objectstack/rest)
  • content/docs/releases/v15.mdx (via @objectstack/core)
  • 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.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling labels Jul 18, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review July 18, 2026 04:28
@os-zhuang
os-zhuang merged commit e057f42 into main Jul 18, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/evaluate-issues-3147-5152-1naxe6 branch July 18, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(objectql): 汇总重算失败仅 warn——子记录批量插入报成功,父 summary 静默过期且无重试

2 participants