Skip to content

Commit 744b8f5

Browse files
claude[bot]claude
andauthored
fix(metadata-protocol,spec): a stopped bulk write reports every record — NOT_ATTEMPTED tail and reconciling counters (#7539) (#7581)
* fix(metadata-protocol,spec): report NOT_ATTEMPTED for a stopped bulk 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 * docs(api): describe the stopped-batch NOT_ATTEMPTED tail on the non-atomic 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c168688 commit 744b8f5

11 files changed

Lines changed: 516 additions & 16 deletions
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/metadata-protocol": patch
4+
---
5+
6+
fix(metadata-protocol,spec): a bulk write that STOPS now reports every record — `NOT_ATTEMPTED` rows instead of a truncated `results` array, and counters that reconcile (#7539)
7+
8+
`POST /data/:object/batch` with no `options` (so `atomic` defaults `false`,
9+
ADR-0119 D4) and three records — valid, failing, valid — answered:
10+
11+
```
12+
200 { "total": 3, "succeeded": 1, "failed": 1,
13+
"results": [ { idx 0: ok }, { idx 1: VALIDATION_FAILED } ] }
14+
```
15+
16+
Two results for three records, no entry for idx 2, and `succeeded + failed` (2)
17+
`!= total` (3). The un-attempted record was invisible **twice over**: it
18+
produced no `results[]` entry and was counted in neither bucket, so the only
19+
trace of it was an arithmetic mismatch a client had to notice and interpret.
20+
21+
`buildBatchDataResponse` read `total` from the REQUEST (`records.length`) while
22+
`results` / `succeeded` / `failed` came from a loop that had stopped early. Its
23+
two siblings under-reported identically — the same defect on `updateManyData`
24+
and `deleteManyData`, whose per-object bulk counters lost the tail whenever a
25+
row failed without `continueOnError`. All three now go through one shared
26+
reconciler rather than a fourth copy of the same arithmetic.
27+
28+
**What changed is the REPORT, not the semantics.** Every record now gets a row
29+
saying what happened to it: records after the failure carry
30+
`errors[0].code === 'NOT_ATTEMPTED'` — the same registered ADR-0112 code the
31+
atomic arm has emitted since #4793, because "never ran" means the same thing to
32+
a client whether the batch stopped to roll back or stopped because it was told
33+
to. The message names the causal row index and `continueOnError`, since on this
34+
arm the caller's next action is a flag rather than a fixed row. `results` now
35+
always covers all `total` records, and `succeeded` / `failed` partition it, so
36+
`succeeded + failed === total === results.length` on both arms.
37+
38+
**The stop itself is unchanged, deliberately.** Without `continueOnError` the
39+
first failure still ends the run, records written before it stay written
40+
(nothing is rolled back on this arm), and the tail is still not attempted.
41+
That is the declared contract, not an accident:
42+
`BatchOptionsSchema.continueOnError` reads *"If true (and atomic=false),
43+
continue processing remaining records after errors"*, ADR-0119 D4 scopes the
44+
flag to exactly `atomic=false`, and D4's test plan holds non-atomic batches to
45+
"behave exactly as before". If `atomic: false` alone continued past a failure,
46+
`continueOnError` would be inert. Callers who want every valid row to land
47+
should send `continueOnError: true` — unchanged, and now the only difference
48+
between the two is whether the tail is attempted, not whether it is reported.
49+
50+
**Upgrade note.** A non-atomic batch that stops now returns more `results` rows
51+
and a larger `failed` count than before, for the same request and the same
52+
writes. `failed` counts every row that is not a success — matching the atomic
53+
rollback response, which has always counted never-reached rows this way. A
54+
client that summed `succeeded + failed` and compared it to `total` to detect
55+
truncation no longer needs to; one that treated `failed` as "rows the server
56+
tried and could not write" should branch on `errors[0].code` instead, where
57+
`NOT_ATTEMPTED` distinguishes "skipped" from "attempted and failed". No schema
58+
field was added or removed.

content/docs/api/data-api.mdx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ Execute a batch operation (create / update / upsert / delete) on multiple record
241241

242242
**Response**: `BatchUpdateResponse` with `succeeded`, `failed`, `total`, and a per-record `results` array. Each entry in `results` has `id`, `success`, `index` (the row's position in the request array), an optional `errors` array (`ApiError[]` — read `errors[0].message`, branch on `errors[0].code`), and optional `data` (the full record, present when `returnRecords` is `true`).
243243

244-
`options.atomic` defaults to `false` (sequential best-effort, stopping at the first failure). Set it to `true` and the whole batch runs inside one transaction: the first failure rolls back every prior write, and the response reports `succeeded: 0` — each row's `errors[0].code` says what happened: `ROLLED_BACK` (written, then undone), the causal row's own error code, or `NOT_ATTEMPTED` (never reached). A deployment whose driver cannot roll back rejects an atomic request with `501 NOT_IMPLEMENTED` instead of running it best-effort — probe `capabilities.transactionalBatch` on `/discovery` first. `atomic` takes precedence over `continueOnError`.
244+
`options.atomic` defaults to `false`: sequential best-effort that stops at the first failure. Records written before the failure stay written — nothing is rolled back on this arm — and every record after it is reported with `errors[0].code` `NOT_ATTEMPTED` rather than omitted, so `results` always covers all `total` records and `succeeded + failed === total` (#7539). Send `continueOnError: true` to process the remaining records instead of stopping. Set `atomic` to `true` and the whole batch runs inside one transaction: the first failure rolls back every prior write, and the response reports `succeeded: 0` — each row's `errors[0].code` says what happened: `ROLLED_BACK` (written, then undone), the causal row's own error code, or `NOT_ATTEMPTED` (never reached). A deployment whose driver cannot roll back rejects an atomic request with `501 NOT_IMPLEMENTED` instead of running it best-effort — probe `capabilities.transactionalBatch` on `/discovery` first. `atomic` takes precedence over `continueOnError`.
245245

246246
### `POST /data/:object/createMany`
247247

@@ -284,7 +284,9 @@ selects rows, so no body key can widen the delete into a filter.
284284
deleted one at a time by primary key, so each honours `deleteBehavior`
285285
(`cascade` / `set_null` / `restrict`) on relations pointing at it. The run stops
286286
at the first failure; `continueOnError: true` processes the remaining ids and
287-
reports the failures instead.
287+
reports the failures instead. Either way every id gets a `results` entry — the
288+
ids a stopped run never reached carry `errors[0].code` `NOT_ATTEMPTED`, so the
289+
counters reconcile against `total` (#7539).
288290

289291
`options.atomic: true` is honoured here the same way as on `/batch` (#4620): the
290292
whole id list runs inside one transaction, the first failure rolls back every

content/docs/references/api/batch.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const result = BatchConfigSchema.parse(data);
5555
| :--- | :--- | :--- | :--- |
5656
| **id** | `string` | optional | Record ID if operation succeeded |
5757
| **success** | `boolean` || Whether this record was processed successfully |
58-
| **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| … +259 more>; message: string; category?: string; httpStatus?: integer; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error (#4793). |
58+
| **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| … +259 more>; message: string; category?: string; httpStatus?: integer; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error (#4793). A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back (#7539). |
5959
| **data** | `Record<string, any>` | optional | Full record data (if returnRecords=true) |
6060
| **index** | `number` | optional | Index of the record in the request array |
6161
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability (#3407/#3431/#3455): caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` (#2948) / TRUE `readonlyWhen` (#3042) on update, or the #3043 create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. |
@@ -83,7 +83,7 @@ const result = BatchConfigSchema.parse(data);
8383
| :--- | :--- | :--- | :--- |
8484
| **atomic** | `boolean` || Opt-in all-or-nothing. When explicitly true the whole batch runs inside ONE engine transaction: the first failure rolls back every prior write, and the response reports zero successes — each row carries `errors[0].code` ROLLED_BACK (written, then undone), the causal row its own error, and rows never reached NOT_ATTEMPTED. A runtime that cannot roll back REFUSES the request (501 NOT_IMPLEMENTED) rather than silently degrading to best-effort — probe `capabilities.transactionalBatch` on /discovery first. Takes precedence over continueOnError. Default false: sequential best-effort. |
8585
| **returnRecords** | `boolean` || If true, return full record data in response |
86-
| **continueOnError** | `boolean` || If true (and atomic=false), continue processing remaining records after errors |
86+
| **continueOnError** | `boolean` || If true (and atomic=false), continue processing remaining records after errors. Default false: the first failure ENDS the run — records before it stay written (nothing is rolled back on this arm), and every record after it is reported `errors[0].code` NOT_ATTEMPTED rather than omitted, so `results` always covers all `total` records and `succeeded + failed === total` (#7539). |
8787
| **validateOnly** | `never` | optional | [REMOVED] `options.validateOnly` was removed from BatchOptions in @objectstack/spec (#4052). It was never implemented: the batch surfaces persisted regardless, so a "dry-run" would have silently executed. There is no dry-run today — drop the key. If you need to preview a batch without writing, open an issue so it can be designed (no-commit cascade / constraint semantics) and reintroduced as a flag that actually holds. |
8888

8989

packages/metadata-protocol/src/protocol.batch-atomic.test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,19 @@ describe('batchData non-atomic — unchanged (ADR-0119 D4 regression net)', () =
255255

256256
expect(t.engine.transaction).not.toHaveBeenCalled();
257257
expect(res.succeeded).toBe(1);
258-
expect(res.failed).toBe(1);
259258
expect(res.results[0].success).toBe(true); // committed, and honestly reported
260259
expect(res.results[1].success).toBe(false);
261-
expect(res.results).toHaveLength(2); // stops without continueOnError
260+
// Still stops without `continueOnError` — two inserts, never three.
261+
expect(t.insert).toHaveBeenCalledTimes(2);
262+
// [#7539] But the STOP is now reported rather than inferred from a
263+
// counter mismatch. This block used to assert `failed: 1` and
264+
// `results.length === 2` against `total: 3` — the truncated `results`
265+
// array and the `succeeded + failed != total` arithmetic that were the
266+
// card's entire symptom.
267+
expect(res.failed).toBe(2);
268+
expect(res.results).toHaveLength(3);
269+
expect(res.succeeded + res.failed).toBe(res.total);
270+
expect(res.results[2].errors[0].code).toBe('NOT_ATTEMPTED');
262271
});
263272

264273
it('atomic: false is best-effort, not a refusal, even on a non-transactional engine', async () => {

0 commit comments

Comments
 (0)