feat(objectql,metadata-protocol): validate and insertMany answer which row lost which field - #21041
Conversation
…and insertMany outcomes engine.validate answers each accepted row's droppedFields, and an ok insertMany outcome carries what the strips took from that row, both recorded at the strips themselves. The batch-level onFieldsDropped union is unchanged. validateData relays the verdict; insertManyData passes the outcomes through. Two spec TSDoc sentences now say what the producer does. Claude-Session: https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG Co-authored-by: Claude <noreply@anthropic.com>
…un and commit Claude-Session: https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG Co-authored-by: Claude <noreply@anthropic.com>
…r-row-dropped-fields
📓 Docs Drift CheckThis PR changes 3 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c139b7f3c17d3452fff2cbfd8eefdd36ccb08109 && git checkout c139b7f3c17d3452fff2cbfd8eefdd36ccb08109
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2f2fa11d756f665a4c06160480c1dce15b9d67a4 886ad2c43ee3a561d9f81bc41f65de09f8a3a8b6 && git checkout -B drift-repro 2f2fa11d756f665a4c06160480c1dce15b9d67a4 && git merge --no-ff 886ad2c43ee3a561d9f81bc41f65de09f8a3a8b6
node scripts/docs-audit/affected-docs.mjs --json 2f2fa11d756f665a4c06160480c1dce15b9d67a4
|
Contract reviewServed-tier: Inputs read: card #20922 (its body and all five comments, 5919693532 through 5924083608), PR #21041 (its body, its eight-file list, and the net diff of the branch head against merge-base ① Derived judgmentsPublic surface, as the two packages'
Accept set: unchanged everywhere. Nothing is newly refused or admitted. Batch-level union: Per-row attribution, judged at the code: recorded at the strips ( Scope item 3 (no second strip, no second reason vocabulary): one builder, one Pins the card names: a formula column ( Docs named by the drift comment (5924017625). ② Semver level
③ Boundary flagsOpen questions (report 5924041038):
Deviations (five):
Dev-declared NOT MEASURED, answered by the head's check-runs: Out-of-scope findings (two, carried to the PR that lands #20701's REST item 1): the half-stale Check-runs on the head (40, read at the time stamped above): 26 success; 5 skipped ( Implemented-by: VERDICT: PASS |
Fixes #20922
Clause-②: yes (widening)
What changes
The dry run and the partial-success batch insert now say which row lost which field.
ObjectQL.validateanswersdroppedFieldson each accepted row ofresults: the fields the write would strip from that row, oneDroppedFieldsEventper reason.validateDatarelays the verdict, so its answer now fillsValidateDataResponseSchema.results[].droppedFields.ObjectQL.insertManyanswersdroppedFieldson eachokoutcome: the fields stripped from that row.insertManyDatapasses the outcomes through, and its declared return type now names the key.ok: falseoutcome, carry none: a drop means the write completed without the field (theDroppedFieldsEventSchemacontract), and the write does not complete that row.How
stripComputedWriteFieldsnow also returnsdroppedPerRow(the keys taken from each row, index-aligned). The caller-write strips (stripRuntimeOwnedFields, the staticreadonlystrip, andstripReadonlyFieldson anupdate-mode preview) record each taken key into the existing union and into the taking row's own list, in the same step. Nothing is reconstructed from the union afterwards. That matters because abeforeInserthook can exempt a key on one row and not another (hookWrittenKeys), so "which rows supplied N" is not "which rows dropped N".droppedFieldEvents(object, computed, readonly)turns a strip result into events: one per reason,computedbeforereadonly, the order the strips run. It builds the batch-level union events (validate's listener,insert'sinsertDrops) and the per-row lists, so the two cannot disagree on a reason or its order. There is no second strip and no second reason vocabulary.onFieldsDroppedevents ofinsert,insertManyandvalidateare still one per reason, naming no row, built from the same union arrays in the same order.insertManyData's top-leveldroppedFieldsand both union docblocks (engine.tsinsertMany,protocol.tsinsertManyData) are byte-identical. The per-row channel is documented onInsertManyRowOutcomeand at the code points instead.insert(object, rows[])still returns the records with no per-row slot.strictReadonlyWritesstill refuses the whole batch before any outcome is built. The engine's aggregate door and the packaged-base door inprotocol.tsare not touched.Spec docblocks (two TSDoc sentences, no schema change)
packages/spec/src/api/protocol.zod.ts, theValidateDataResponseSchemadocblock: "the engine'svalidatealready runs the write's strips ... no server sets the key until it does" now says what the producer does.validateruns the computed-field strip and the staticreadonly/ runtime-owned strips, records what each takes per row, and reports it on an accepted row.validateDatarelays it. Anupdate-mode preview does not run thereadonlyWhenor primary-key strips, so it can report fewer drops than the update it predicts.packages/spec/src/api/export.zod.ts, theImportRowResultSchemadocblock: "pinned at the wire" now names what is pinned. The synchronous route is pinned byimport-dryrun-parity.test.ts, and no test readswarningsoff the async job's results..describe().check:generatedreports all 15 artifacts up to date after a spec build. The diff touchespackages/spec/src/**, so the landing owes an at-tier contract review.Premise checks (measured at
9b0de7de7before the first edit)engine.validatebuiltcomputedStrip.droppedandreadonlyDroppedacross all rows and emitted one event per reason. It already built per-rowresults, and the per-row channel now rides them.insertManyDatacallsengine.insertMany, which isinsert(…, { __partialRowErrors: true }). The outcomes are assembled ininsert's partial-mode branch, and that is where the per-outcome report is attached, from the strips' own record.insertManyDataadds nothing.insert-mode preview runs the same three strips the write runs (computed, runtime-owned, staticreadonlywith its re-default), in the same order and under the sameisSystemgate. A row's preview drops therefore equal its outcome drops (pinned row for row). The one known difference is the documented "no hooks run" gap: a key abeforeInserthook assigns is reported by the preview and kept by the write.update-mode preview runs noreadonlyWhenor primary-key strip, while the update it predicts does. The import route previews an upsert's matched rows inupdatemode and commits them throughupdateData. On a row whosereadonlyWhenis true (for example the showcase invoice'stax_rateoncestatus == 'paid'), the dry run names nothing for that field and the update drops it underreadonly_when. This PR keeps the existing named limit, and the spec docblock now states it. The options and their costs are in the report on the card.Tests
The tests ran at
24ca898f8/99a7547af. HEAD886ad2c43adds only a merge oforigin/main, whose three commits touch none ofobjectql,metadata-protocol,specorrest. The gate union ran at886ad2c43.packages/objectql/src/engine-per-row-dropped-fields.test.ts(14 cases, recording driver). It pins a formula column (computed) and areadonlycolumn (readonly) through the dry run, the commit, and both relayed through the real protocol (validateData,insertManyData). A clean row carries none (the control), and the batch-level union is unchanged. A refused row and a failed outcome carry none. A hook-exempted row is not named. UnderisSystem, only the computed strip reports.update-mode previews report per row too.engine-autonumber-runtime-owned.test.ts: the case that pinned "names no row" against the real engine now pins the row that lostaccount_numberon its own outcome, beside the unchanged union.protocol.dropped-fields.bulk.test.ts: a new case checks that an engine-attributed outcome passes through as answered. The existing cases still pin that the seam derives nothing from the union.pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2: 349 files, 6835 tests passed.pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2: 195 passed / 3 skipped files, 2897 passed / 19 skipped tests.pnpm --filter @objectstack/objectql --filter @objectstack/metadata-protocol run typecheck: exit 0 (check:test-typecheckOK, ledger unchanged).dist/: all 25packages/rest/src/import-*.test.tsfiles (622 passed, 21 skipped), and fourplugin-securitypreview/write tests (132 passed).scripts/ablation-replace.mjs, which landed and restored the mutation on disk; the test readsengine.tsfromsrc): theinsertManyoutcome was given the batch union instead of its row's list. Result: 4 failed / 10 passed. The tree was restored to the HEAD blob, andgit diff HEADwas empty.validate's rows. Result: 5 failed / 9 passed, then restored.insertManyData's outcome (read through@objectstack/metadata-protocol's rebuilt.d.ts) turnedcheck:test-typecheckred (1 type error in the new file). It was then restored.Gates
node scripts/pm/dispatch-gates.mjs --commandswas derived at886ad2c43and named 89 families. All were run: 87 exited 0, and 2 exited 3. Reconciled with--ran: "89 derived famil(ies) accounted for, 87 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)".check:dual-build-cjs-loads,check:type-check-debt. Reason: PREREQUISITE NOT MET, because both read a full workspace build this worktree does not have. CI builds it.packages/runtimeintegration tier (undeclared-field-write-driver-split.integration.test.tscallsvalidate). Reason: 16 packages in runtime's closure are unbuilt here. It makes no deep-equality assertion onresults. Declared to CI..tsfiles with--no-inline-config --format json: 7 files, 0 errors, 0 warnings. Each file is inside the config's population (--print-configresolves it, and no "file ignored" warning). The narrowing excludes nothing:eslint.config.mjsenables no type-aware linting for any file (its own note near line 327), so this diff cannot move a verdict on an untouched file.Changeset
.changeset/20922-per-row-dropped-fields.mdgrades@objectstack/objectql: minorand@objectstack/metadata-protocol: minorby the WHICH LEVEL rule. Each widens a published method's declared answer with a new optional key:InsertManyRowOutcomegainsdroppedFields, and so does each outcome ofinsertManyData's return type. That is an additive widening of the public surface. Nothing is removed, renamed or refused. The spec edits are comments, so the spec carries no entry.Acceptance notes
ImportRowResultSchemadocblock'sdroppedFieldsbullet still reads "The engine has to report drops per row out ofvalidateDataandinsertMany, and the REST import route has to copy them onto the row; until both do, no server sets the key". This stays true until the REST half copies the report, and the engine half is now done. The PR that lands rest(import): a column for a formula field passes the dry run, then fails the row at commit with the driver's SQL error, where the create door answers 400 INVALID_FIELD #20701's REST item rewrites it. It is out of this card's two-sentence spec scope.packages/rest/src/import-runner.ts'sImportProtocolLike.insertManyDatatypes its outcomes withoutdroppedFields. Widening it is the REST half's first step, and it is not touched here.Generated by Claude Code