fix: harden the bulk-write path — retries, idempotency, contracts, and summary visibility (#3147–#3152)#3161
Merged
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 31 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
os-zhuang
marked this pull request as ready for review
July 18, 2026 04:28
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
fix(core): validate writeBatch result contractbulkWriteguardswriteBatch's return: a short / long / non-array result now throwsERR_BULK_RESULT_MISMATCH→ per-row degradation, instead of backfillingrecord: undefinedas phantom success. Documents at-least-once delivery.fix(seed,rest,core): wrap bare writes in transient retrywriteRecord/resolveDeferredUpdates, import's no-createManyDatafallback — inwithTransientRetry.defaultIsTransientErrorshort-circuits definitive logical signatures (constraint/validation/unique/…) to non-transient.fix(rest): import resolveRef flushes pending createsresolveRefflushes the pending-create buffer and retries once. A bare miss is no longer negatively cached.fix(objectql): reject short/non-array bulkCreate resultsengine.insert(array)throwsERR_BULK_RESULT_MISMATCHif the driver returns the wrong record count, rather than padding afterInsert contexts / caller results withundefined.fix(core,seed,rest): idempotent batch retry via attempt-aware recheckattemptcounter intowriteBatch/writeOne; onattempt > 1seed rechecks byexternalIdand import bymatchFields, 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 failuresrecomputeSummariesretries each parent's aggregate+update with backoff and collects failures; on exhaustion, insert/update/delete throwSummaryRecomputeError(ERR_SUMMARY_RECOMPUTE) carrying the written records. Seed/import recover it to success + warning (no re-write), identified bycodeto avoid an import cycle.fix(objectql): assign autonumbers after validationDesign notes (cross-issue interactions)
recomputeSummariesfails after the records are written; if that threw a plain error,bulkWritewould degrade and re-insert already-written rows (a bug(core/bulkWrite): 批量重试无幂等——驱动已提交但响应丢失时,重试导致整批重复插入 #3149-style duplicate). Instead the error is typed (ERR_SUMMARY_RECOMPUTE) and carries the written records; seed/import recognize thecode, recover to success, and never re-write.bulkWritededup —bulkWritecan't know the natural key. Theattemptcounter is the seam that lets each caller recheck between attempts.lastBatchUncertain), so it can't duplicate either.Tests
New/extended, all green (
vitest):bulk-write.test.ts: contract-guard (short/long/non-array → degrade),attemptpropagation, mixed constraint+network → non-transient.engine-bulk-contract.test.ts,engine-summary-retry.test.ts,engine-autonumber-batch.test.ts+ extendedsummary-rollup.seed-loader-retry.test.ts: self-ref retry, commit-then-lost-response no-dup, summary-failure recovery.import-runner-selfref.test.ts,import-runner-idempotency.test.ts+ extendedimport-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