fix(metadata-protocol,spec): a stopped bulk write reports every record — NOT_ATTEMPTED tail and reconciling counters (#7539) - #7581
Merged
Conversation
…write's tail, and make the counters reconcile (#7539) A non-atomic `/batch` that stopped at the first failure answered with a truncated `results` array and counters that did not add up: two results for three records, no entry for the un-attempted record, and `succeeded + failed != total`. The skipped record was invisible twice over — no `results[]` entry and counted in neither bucket — so the arithmetic mismatch was its only trace. `buildBatchDataResponse` read `total` from the request while `results`, `succeeded` and `failed` came from a loop that had stopped early. `buildUpdateManyResponse` and `buildDeleteManyResponse` under-reported the same way. All three now share one reconciler that pads the outcome out to the request length with `NOT_ATTEMPTED` rows — the registered ADR-0112 code the atomic arm has emitted since #4793 — and returns `failed` as the count of every non-success row, so `succeeded + failed === total === results.length` on both arms. The stop itself is unchanged, per `BatchOptionsSchema.continueOnError` ("If true (and atomic=false), continue processing remaining records after errors") and ADR-0119 D4, whose test plan holds non-atomic batches to "behave exactly as before". This is a reporting fix; `continueOnError` remains the flag that buys continuation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gd2bypaK4KYs78q8RP38G
…tomic arm (#7539) `data-api.mdx` described `atomic: false` as "sequential best-effort, stopping at the first failure" without saying what the response contains for the records it never reached — the shape the fix makes explicit. `batch.mdx` is regenerated from the two `.describe()` strings this change touches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gd2bypaK4KYs78q8RP38G
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
os-zhuang
marked this pull request as ready for review
August 11, 2026 06:36
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
2 participants
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.
Fixes #7539
(B) is RESOLVED by ADR-0119 D4 — and it resolves AGAINST the card's "Expected"
The dispatch split this card in two and told me not to guess on (B). D4 settles
it, so this PR implements the settled answer rather than reporting an open
question. Quoting the record:
And the description D4 defers to,
BatchOptionsSchema.continueOnError(
packages/spec/src/api/batch.zod.ts):So
atomic: falsealone promises best-effort that stops at the firstfailure.
continueOnErroris the knob that buys continuation, and it isscoped to exactly
atomic=false. D4 never touched non-atomic continuation — itexplicitly pinned it as unchanged. The card's "Expected per ADR-0119 D4 … valid
rows land, and neither valid row is blocked by the other" is a misreading: D4
says that about
atomic: true's rollback reporting, not about the non-atomicarm's continuation.
The dispatch's own suspicion was right — the
continueOnError: truecontrolproducing exactly the card's "expected" result is the tell. If
atomic:falsealone continued,
continueOnErrorwould be inert, which is thedeclared-but-unenforced shape ADR-0049 and this ADR exist to eliminate.
So the break stays, and the third record still does not land. Three of the
card's four symptoms are real defects and are fixed; the fourth ("the third,
valid record never lands") is correct behaviour under the declared contract, and
the caller's remedy is
continueOnError: true— which the response now names inthe skipped row's message.
(A) The reporting defect — fixed outright
buildBatchDataResponsereadtotalfrom the REQUEST (records.length) whileresults/succeeded/failedcame from a loop that had stopped early, so anun-attempted record was invisible twice over: no
results[]entry, andcounted in neither bucket. The only trace was
succeeded + failed != total— anarithmetic mismatch no client should have to notice, let alone interpret.
Every record now gets a row saying what happened to it. Records after the failure
carry
errors[0].code === 'NOT_ATTEMPTED'— the same registered ADR-0112 codethe atomic arm has emitted since #4793, because "never ran" means the same thing
to a client whether the batch stopped to roll back or stopped because it was told
to. The message names the causal row index and
continueOnError, since on thisarm the caller's next action is a flag rather than a fixed row.
The card's same-family observation — per-object bulk counters under-report
whenever a row fails without
continueOnError— is the identical defect onupdateManyDataanddeleteManyData, fixed in the same pass through oneshared reconciler, not a third and fourth copy of the arithmetic (#4620's
lesson: copies are how these three drifted apart before).
Counter shape: why no new
notAttemptedfieldBatchUpdateResponseSchemadeclares{ total, succeeded, failed, results }.The already-shipped atomic precedent (
buildRolledBackBatchResponse) countsnever-reached rows into
failedand holdssucceeded + failed === total. Thismatches it, so the envelope reads the same on both arms:
succeeded= rows withsuccess === truefailed= rows withsuccess === falsesucceeded + failed === total === results.lengthA
notAttemptedenvelope field would have bought the same information at theprice of two meanings for
failedon two arms of one endpoint. The three-termidentity is still asserted in the tests, with
notAttemptedderived from therow codes. No schema field added or removed.
Base SHA re-verification of the two controls
Re-measured at my branch point (
9051802), before any source edit — both behaveexactly as the card records:
9051802continueOnError: truesucceeded 2 / failed 1, both valid rows persistedatomic: trueROLLED_BACK/VALIDATION_FAILED/NOT_ATTEMPTED,succeeded 0 / failed 3, nothing persistedBoth are kept as
GUARDcases in the new suite and are green in bothdirections.
File surface
packages/metadata-protocol/src/protocol.tsreconcileStoppedBatch(pads a stopped outcome to the request length withNOT_ATTEMPTEDrows;failedbecomes the count of non-success rows). Wired intobuildBatchDataResponse,buildUpdateManyResponse,buildDeleteManyResponse. No change to any loop or torunAtomicBatch.packages/spec/src/api/batch.zod.tscontinueOnErrorandBatchOperationResultSchema.errorsdescriptions state the non-atomic stop's reporting contract.packages/spec/src/api/error-code-ledger.zod.tsNOT_ATTEMPTED's ledger comment widened from "atomic data-batch row" to either stop reason.packages/metadata-protocol/src/protocol.batch-not-attempted.test.tsGUARD.packages/metadata-protocol/src/protocol.batch-atomic.test.tsexpect(res.results).toHaveLength(2)againsttotal: 3) rewritten; the STOP re-pinned viainsertcall count.packages/metadata-protocol/src/protocol.many-data-atomic.test.tsupdateManyData; added a store assertion that the tail is still not written.packages/metadata-protocol/src/protocol.delete-many.test.ts,protocol.record-not-found.test.tsdeleteManyData.content/docs/api/data-api.mdx/batch+/deleteManysections describe theNOT_ATTEMPTEDtail.content/docs/references/api/batch.mdx.describe()edits (check:docsgate caught it)..changeset/batch-not-attempted-tail-reporting.md@objectstack/spec+@objectstack/metadata-protocol.buildBatchDataResponseand both siblings areprivate; the only callers arethe four sites in
protocol.ts(grepped). REST is a pure pass-through(
res.json(result)— no status or counter derivation), so the wire shape movesexactly with the protocol.
Reverse verification
Predictions were written before the first run
(
succeeded + failed + notAttempted === totalandresults.length === total,plus per-index outcomes by identity — not "one new entry is present"). Revert
was a one-line early
return outcomein the reconciler, which restores thedefect exactly.
batchData: 3 results for 3 records, idx 2NOT_ATTEMPTED, counters reconcilebatchData:returnRecords:falsekeeps theNOT_ATTEMPTEDrowupdateManyDatastopped run: length + countersdeleteManyDatastopped run: length + countersprotocol.batch-atomic.test.tsnon-atomic regression netprotocol.many-data-atomic.test.tsnon-atomic regression netprotocol.delete-many.test.tsstop-at-first-failureprotocol.record-not-found.test.tsmissing-id stopcontinueOnError:true→ 3 results, both valid rows landatomic:true→ROLLED_BACK/causal/NOT_ATTEMPTEDRevert turned exactly the 8 predicted pins red and left all 4 GUARDs green.
Missed prediction (1, disclosed)
I predicted the semantics GUARD ("idx 2 is never attempted and never lands")
would be GREEN at base. It was RED —
TypeError: Cannot read properties of undefined (reading 'success'). Cause: I had put two assertions aboutresults[2]'s shape inside a control whose job is the semantics, and at baseresults[2]does not exist — the very defect under repair. A both-directionsGUARD must not depend on the thing being fixed. Fixed by moving those two
assertions into the reporting pin, leaving the GUARD on engine-call counts and
store contents only; it is green in all three columns above. No other prediction
missed. (One further self-inflicted red during implementation: an assertion I
added guessed a fixture's initial value as
'c'when it was'c-old'— a testauthoring slip, corrected, no product implication.)
Gates
@objectstack/metadata-protocolvitest@objectstack/specvitest@objectstack/restvitest@objectstack/objectqlvitestturbo typecheck(spec, metadata-protocol, rest, objectql)eslint --no-inline-config(all changed files)pnpm --filter @objectstack/spec check:docscheck-regen-pending/check:doc-authoringOut of scope
#4620 (
deleteManyData/updateManyDataatomicity) is not folded in — itis closed, and it covered the atomic arm, not this loop's break-and-miscount.
The two siblings are touched here only for the identical counter
under-report, which the card names explicitly as same-family.
Note for the record
No semantic-migration registry entry: the response schema is unchanged, the code
is already registered in the ADR-0112 ledger, and no consumer must move a read.
This restores a declared invariant rather than changing a declared shape. The
behaviour change is nonetheless real for clients — a stopped batch now returns
more
resultsrows and a largerfailedcount for the same request and the samewrites — and the changeset carries that upgrade note.
🤖 Generated with Claude Code
https://claude.ai/code/session_016gd2bypaK4KYs78q8RP38G
Generated by Claude Code