From b8c7313cd2747bbb8d4914d8efd02874e74541d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 03:20:58 +0000 Subject: [PATCH] docs(metadata-protocol): the create-strip aggregation states what the tree does, not a premise it falsifies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docblock on `mergeDroppedFieldEvents` justified `createManyData`'s aggregated `droppedFields` with a second, falsified reason: that the create-side static-`readonly` strip is "schema-uniform — every row drops the same set". Maintainer ruling C (#14147) falsifies it. The strip runs inside `engine.insert`, after `beforeInsert`, and exempts keys a hook itself assigned — indexed per row at the call site (`hookWrittenKeys: rowHookWrittenKeys[i]`, packages/objectql/src/engine.ts), so a hook that stamps a protected key on some rows and not others makes those rows drop different sets. The first reason the docblock already gave is the correct one and is the one `packages/spec` kept when the sibling carrier was corrected: the `{ object, records, count }` response has no per-row slot, so a union is the only view it can represent. The shape is unchanged; only the justification and the reader guidance move. Reader guidance matches the landed spec wording — read a name as "at least one row dropped this field", not "every row did". Three carriers in this package, judged by claim rather than by spelling: the docblock, the bulk test's header comment, and a test TITLE that named the pinned behaviour by the falsified premise. The test's assertions are untouched — the aggregate is still right for the first reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --- .../src/protocol.dropped-fields.bulk.test.ts | 7 +++-- packages/metadata-protocol/src/protocol.ts | 27 ++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts b/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts index 6e736116d7..d52162a4ad 100644 --- a/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts +++ b/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts @@ -10,7 +10,10 @@ // - updateManyData / batchData → per-row `droppedFields` on each result row; // - insertManyData → per-row `droppedFields` on each outcome; // - createManyData → aggregated top-level `droppedFields` (its -// response has no per-row slot; the insert strip is schema-uniform). +// response has no per-row slot, so a union is the only view it can +// represent; read a name there as "at least one row dropped this field", +// never "every row dropped the same set" — ruling C (#14147) exempts keys a +// `beforeInsert` hook assigned, recorded per row, so rows CAN differ). import { describe, it, expect, vi } from 'vitest'; import { assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; @@ -87,7 +90,7 @@ describe('createManyData — aggregated top-level droppedFields (#3455)', () => return { p: new ObjectStackProtocolImplementation(engine as any), engine }; } - it('aggregates the schema-uniform create strip across rows into one event', async () => { + it('two rows forging the same readonly key surface ONE aggregated top-level event', async () => { const { p } = makeProtocol(); const res: any = await p.createManyData({ object: 'approval_case', diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 3d7ca6399e..510807d99c 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1873,14 +1873,27 @@ const CLONE_STRIP_FIELDS: readonly string[] = [ * `(object, reason)` with the UNION of dropped field names. * * Used by the bulk-create surface (`createManyData`), whose `{ object, records, - * count }` response has no per-row slot to hang a `droppedFields` on. The - * create-side static-`readonly` strip is schema-uniform — every row drops the - * same set — which makes an aggregated view faithful rather than lossy. (Since - * #14147 that strip is the ENGINE's, which reports one event per CALL for it, - * so the aggregation is over the runtime-owned per-row events.) Returns `[]` when nothing was dropped so callers can spread + * count }` response has no per-row slot to hang a `droppedFields` on — a union + * is the only view that response can represent, which is the whole reason this + * collapse exists. (Since #14147 that strip is the ENGINE's, which reports one + * event per CALL for it, so the aggregation is over the runtime-owned per-row + * events.) + * + * ⚠️ So read a name in a merged event as "AT LEAST ONE row dropped this field", + * never "every row dropped the same set". Maintainer ruling C (#14147) put the + * static-`readonly` strip INSIDE `engine.insert`, AFTER the `beforeInsert` + * hooks, where it exempts keys a hook itself assigned — recorded PER ROW and + * indexed per row at the call: `packages/objectql/src/engine.ts` hands + * `stripReadonlyFields` the option `hookWrittenKeys: rowHookWrittenKeys[i]`, + * and that option's only power is to turn a STRIP into a KEEP. A hook that + * stamps a protected key on some rows and not others therefore makes those rows + * drop DIFFERENT sets, so the union is faithful to the BATCH without being + * faithful to any one row. + * + * Returns `[]` when nothing was dropped so callers can spread * `...(x.length ? { droppedFields: x } : {})` and keep the omit-when-empty shape. - * The per-row `insertMany`/`batch` paths keep row precision instead (they have a - * per-row result to carry it). + * The per-row `insertMany`/`batch` paths carry their own per-row `droppedFields` + * instead — they have a per-row result to hang one on. */ function mergeDroppedFieldEvents(events: DroppedFieldsEvent[]): DroppedFieldsEvent[] { if (events.length === 0) return [];