Skip to content

Commit bbcd20c

Browse files
fix(core): an import row reports the fields its write dropped, on the dry run and on the commit (#21203)
Fixes #20701 Clause-②: yes (widening) ## What changes An import row now says which of its fields the write dropped, on the dry run and on the commit, in the words the create door already uses. The card's own case, measured through `POST /api/v1/data/:object/import` on SQLite `:memory:` at `origin/main` `f3b16fc2f` (real ObjectQL, the real protocol, the real RestServer routes): | column | `POST /data/:object` | import dry run, before | import commit, before | |:--|:--|:--|:--| | formula `doubled` (the card's title case) | `201` + `droppedFields` `computed` | ok, created, no drop signal | ok, created, no drop signal | | static `readonly` `ro` | `201` + `droppedFields` `readonly` | ok, created, no drop signal | ok, created, no drop signal | | only a formula key | `201`, an empty record + `computed` | ok, created | ok, created (an empty record) | | writable `title` | `201` | ok, created | ok, created | The commit no longer fails the formula row: the engine strips the value before the driver sees it, and reports the strip per row. What was left was the report. The door said what it dropped, and the import row said nothing on either half, so a file whose formula column was ignored read exactly like a file that wrote it. After this PR, each `ok` row carries the engine's own per-row `droppedFields`, equal to the create door's. The dry run's report equals the commit's. The sync route, the async job's results and the async dry run all carry it. ## How - **One helper, `rowDrops`, in `packages/core/src/utils/import-runner.ts`.** It copies the engine's per-row report onto the row verbatim, from whichever single-row channel that row's write answered on: - the dry run: the `validateData` verdict for the row (`results[0].droppedFields`); - a create through `insertManyData`: the row's own outcome; - a create through `createData` (the inline path, and `bulkWrite`'s per-row degradation) and an update through `updateData`: the response, which is one row's. - `bulkWrite`'s per-row result carries only the record, so a buffered create's report is recorded by the write call that wrote the row, keyed by the row object itself, and read back when the row's result is built. `bulk-write.ts` is not touched. - ⛔ The import keeps no list of non-writable types and never reads a `reason`. The engine's report is the only source, so a reason the engine adds later reaches the row unchanged. One core case uses a reason outside today's enum to prove the copy is verbatim. - `ImportProtocolLike.insertManyData`'s declared outcome now names the optional `droppedFields` it already answered with. That is the step the engine PR's acceptance notes left to this one. - **REST: no edit.** The sync route answers `summary.results` whole. The job worker persists `capImportResults(summary.results)`, which keeps each row whole. The results route returns the stored items whole. The new REST pins measure all three. ## Where the key stays absent, and what the spec now says Two named places, both stated in the core header and in the spec: 1. **A create batched through `createManyData`** (a protocol without `insertManyData`). Its report is a batch-level union that names no row, and its own docblock forbids resolving it to rows, so those rows carry no key. That is not a guessed share. 2. **An upsert row matched to a record whose `readonlyWhen` is true.** The preview asks `validateData` in `update` mode, which runs no `readonlyWhen` or primary-key strip, because it has no prior record. So the dry run names nothing for that field, while the commit reports `readonly_when`, as `PATCH` does. This is the engine seat's ruling on that fork (keep the named limit; the commit still reports the drop). It is pinned as the limit, so that closing it flips the pin on purpose. The `ImportRowResultSchema.droppedFields` describe said "A dry-run row reports the strips the write would make". Cell 2 makes that false once the key is produced, so the describe now says a dry-run row reports the strips its preview runs, and names the update-mode limit. The docblock bullet that said the key was "DECLARED AHEAD of its producer … no server sets the key" now names the producer and both places. The neighbouring `warnings` bullet now names the runner's current path. `content/docs/references/api/export.mdx` is regenerated (`check:generated --fix`, only the stale `check:docs`). ## Two pending release notes corrected: please confirm This PR edits two changesets it did not add, because it makes one sentence in each false in the same release. That is the DELIBERATE CORRECTION class of `check-empty-changeset`, which exits 1 by design: - `.changeset/20921-import-row-dropped-fields.md`: it loses "The key is declared ahead of its producer." - `.changeset/20922-per-row-dropped-fields.md`: it loses "The REST import route does not copy the per-row report onto its row results yet." No other sentence in either note is changed. `skip-changeset` is not applied. `Check Changeset` stays red until someone confirms the correction here. ## Hypotheses, measured - **H1:** at `f3b16fc2f` the formula row succeeds on both halves; neither half reports the drop (table above). - **H2:** holds. `validateData` answers `results[].droppedFields`, and `insertManyData` answers `outcomes[i].droppedFields`. `createManyData` answers only a batch union. `createData` and `updateData` answer per call. Before this PR, `runImport` read none of them. - **H3:** a row whose only payload key is dropped: the dry run says created, the commit creates an empty record, and the door answers `201` with the same empty record. The two halves agree, and now both report the drop. The one remaining difference is cell 2 above. - **H4:** all three REST surfaces pass the row whole; no REST edit. - **H5:** objectui (`origin/main` `edfcf5a`). `ImportWizard.tsx` reads `row`, `ok`, `field`, `error` and `code` off import rows; `importRecords` returns the result untouched. The exhaustive `reason` consumer (`writeWarningToast`, `STRIPPED_LINE`) is fed only from create and update responses. So nothing in objectui reads `ImportRowResult.droppedFields` today. ## Tests All readings are on SQLite `:memory:`, with heavy steps under `os-verify-lock`. The final head is `345905c5f8`, a merge of `origin/main` whose incoming commits touch none of the import paths. - **New `packages/core/src/utils/import-runner-dropped-fields.test.ts`** (11 cases, `ImportProtocolLike` doubles, one channel per case): the dry run (create and update), the `insertManyData` outcomes, `createData`, a failed batch degraded to `createData`, a summary recompute failure after the write, `updateData`, the `createManyData` union (no key), verbatim copy of an unknown reason, and the controls: a clean row, a refused row, a failed outcome. - **New `packages/rest/src/import-row-dropped-fields-20701.test.ts`** (11 cases, real engine). The card's formula case, readonly, and both together: dry run and commit each equal the create door's `droppedFields`. Also: a row whose only key is dropped; per-row attribution in a mixed file; upsert updates equal to `PATCH`'s report; the `readonlyWhen` limit; and the async job's results route, commit and dry run. - **Flipped on purpose:** `import-row-report-field-20701.test.ts`, whose formula and readonly cells pinned only that both halves succeed. They now also assert the reported strip and its reason, on both halves. - `pnpm --filter @objectstack/core exec vitest run --project local --maxWorkers=2`, at `345905c5f8`: `Test Files 75 passed (75)`, `Tests 2114 passed (2114)`. - `pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2`, the whole package at `ca1af119d3`: `Test Files 256 passed (256)`, `Tests 4844 passed | 301 skipped (5145)`. - The last merge (`345905c5f8`) brought objectql, driver-memory, service-analytics and one spec contract file. On it, rest's closure was rebuilt and its 18 import suites re-ran: `Tests 413 passed | 21 skipped (434)`. - `pnpm --filter @objectstack/core run typecheck` and `pnpm --filter @objectstack/rest run typecheck`, at `345905c5f8`: exit 0. `tsc -p tsconfig.test.json --listFilesOnly` lists the new core pin. - Spec, at `0bfd86f84f`: `export.test.ts` and `validate-data.test.ts` (`53 passed`), and `pnpm --filter @objectstack/spec run typecheck` exit 0. - Import-side consumers, after building their closures: plugin-auth's four import suites (`140 passed`) and service-automation's `connector-pull` suites (`19 passed`). `tsc --noEmit` and `check:test-typecheck` pass in both packages. - **Reverse check of the cross-package type:** a throwaway rest file typed against core's rebuilt `.d.ts`. A valid `ImportRowResult.droppedFields` and a valid `insertManyData` outcome compiled. A bad `reason` gave exactly one `TS2322`. The file was then deleted. ### Ablation Each leg ran with the fix committed first, through `scripts/ablation-replace.mjs` (WRAP mode), and each was restored to `blob == HEAD (10ea4dd)` with an empty `git diff HEAD`. - **Core leg:** `rowDrops` returns `{}`. The anchor went from 1 hit to 0 and the blob changed. Result: `Tests 9 failed | 2 passed (11)`; the two greens are the no-drop controls. - **REST leg (dist-routed):** rest resolves `@objectstack/core` through `dist/`. The same mutation, with a `globalThis.__ABLATION_20701__` marker, was rebuilt into core. - `ablation-dist-preflight` found the marker in `dist/`. tsup's DTS step refused the mutated file (unused parameter), but the JS it reads was emitted. - Result: `Tests 11 failed | 8 passed (19)` over the new pin and the flipped file; the greens are the controls. - Restore leg: core rebuilt, and `ablation-dist-preflight … --absent` printed "marker absent from all 14 built files". The two files re-ran green (`19 passed`). ### Gates `node scripts/pm/dispatch-gates.mjs --commands` was re-derived at `345905c5f8` and named 110 commands. Each ran with its exit code recorded before any pipe. `--ran` reconciles: "110 derived famil(ies) accounted for — 110 run, 0 NOT-MEASURED". - 109 exited 0. - 1 exited 1 by design: `check-empty-changeset --base origin/main`, the DELIBERATE CORRECTION above. - `check:driver-conformance` read the same before and after: "OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt". ### Lint, narrowed with proof (at `345905c5f8`) 1. **The population comes from eslint's own config.** `isPathIgnored` is true for the three changesets and the generated `.mdx`. `calculateConfigForFile` lints the six `.ts` files. 2. **The counts come from eslint's JSON.** `eslint --no-inline-config --format json` over those six: `files=6 errors=0 warnings=0`. 3. **Invariance.** `parserOptions.project` and `projectService` are null on all six, so type-aware linting is off and this diff cannot move eslint's verdict on any file it does not touch. ## Acceptance notes - The claim's surface named the core runner, REST and `.changeset/20701-*.md`. This PR also edits: - `packages/spec/src/api/export.zod.ts`: a TSDoc docblock and one describe; - a spec test comment; - the generated `export.mdx`; - the two predecessor changesets. Each says something this PR makes false. The engine seat named the docblock bullet and the `insertManyData` outcome type as this PR's inputs. - Line 2 declares a widening, per the seat's answer 5935693776 on the card. `@objectstack/core` exports `ImportRowResult` and `ImportProtocolLike`, and both gain an optional `droppedFields`, an additive widening of its published TypeScript surface. So the changeset grades `@objectstack/core` `minor`. `@objectstack/spec` stays `patch`, for the describe text. No BREAKING marker is owed: nothing is removed, renamed or refused. - The `createManyData`-only path and the update-mode `readonlyWhen` preview are the two named absences above. Neither is a new defect. --- _Generated by [Claude Code](https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ce4e205 commit bbcd20c

10 files changed

Lines changed: 617 additions & 39 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/core': minor
3+
'@objectstack/spec': patch
4+
---
5+
6+
An import row now says which of its fields the write dropped, on the dry run and on the commit. A column mapped to a `formula` field, a static `readonly` field or a runtime-owned field is legally stripped by the engine: the row still succeeds, and the create door already reported the strip as `droppedFields`. The import row answered a bare `ok` / `created` on both halves, so a file whose formula column was ignored read exactly like a file that wrote it. `runImport` now copies the engine's own per-row report onto each `ok` row as `ImportRowResult.droppedFields`: from the `validateData` verdict on the dry run, from the row's `insertManyData` outcome, and from the `createData` / `updateData` response of a single-row write. The synchronous route, the async job's results and the job's dry run all carry it; no REST change was needed.
7+
8+
Clause-②: yes (widening)
9+
10+
- **Verbatim, in the engine's vocabulary.** The events are the engine's `DroppedFieldsEvent`s, one per reason (`computed`, `readonly`, `readonly_when`, `primary_key`). The import reads no reason and keeps no list of non-writable types, so a reason the engine adds later reaches the row unchanged. A reader that branches on `reason` must stay exhaustive.
11+
- **Where the key is absent although something may have been dropped.** A create batched through `createManyData` (a protocol without `insertManyData`) is reported only as a batch-level union that names no row, so those rows carry no key. And a row the import would UPDATE is previewed in `update` mode, which runs no `readonlyWhen` or primary-key strip, so its dry run can name fewer fields than its commit. The `ImportRowResultSchema.droppedFields` describe now says both.
12+
- **Unchanged:** `ok`, `action`, the counters, the failed rows and the async job's results cap. A clean row, a failed row and a skipped row carry no `droppedFields`.
13+
14+
`ImportProtocolLike.insertManyData`'s declared outcome now names the optional `droppedFields` it already answered with.

‎.changeset/20921-import-row-dropped-fields.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
'@objectstack/spec': minor
33
---
44

5-
The import row report and the validate-only answer can now say which fields a write drops. `ImportRowResultSchema` and each `ValidateDataResponseSchema.results[]` row gain an optional `droppedFields`: an array of `DroppedFieldsEventSchema`, the engine's own strip event. So the reason vocabulary is the engine's (`readonly`, `readonly_when`, `primary_key`, `computed`), and there is no second enum. The row still succeeds: `ok`, `action` and `valid` are unchanged. The key is declared ahead of its producer. A server that does not produce the report omits the key, so an absent key alone does not prove nothing was dropped.
5+
The import row report and the validate-only answer can now say which fields a write drops. `ImportRowResultSchema` and each `ValidateDataResponseSchema.results[]` row gain an optional `droppedFields`: an array of `DroppedFieldsEventSchema`, the engine's own strip event. So the reason vocabulary is the engine's (`readonly`, `readonly_when`, `primary_key`, `computed`), and there is no second enum. The row still succeeds: `ok`, `action` and `valid` are unchanged. A server that does not produce the report omits the key, so an absent key alone does not prove nothing was dropped.
66

77
`ImportRowResultSchema` also declares `warnings`, which the REST import dry run already serves: the findings the validate verdict admits, in the `ValidateDataIssue` shape, on an ok dry-run row. Until now `ImportRowResultSchema.parse` stripped the key, and readers typed by the spec could not see it.
88

‎.changeset/20922-per-row-dropped-fields.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ The dry run and the partial-success batch insert now say which row lost which fi
1010
- **The dry run and the commit agree.** On `insert` mode the preview runs the same strips the write runs, so a row's preview drops and its outcome drops are the same list. One gap is unchanged: the preview runs no hooks, so a key a `beforeInsert` hook assigns is reported by the preview and kept by the write. An `update`-mode preview does not run the `readonlyWhen` or primary-key strips, which judge a prior record the preview does not read.
1111
- **Unchanged:** the `onFieldsDropped` listener on `insert`, `insertMany` and `validate` still reports the batch-level union, one event per reason, naming no row. So does `insertManyData`'s top-level `droppedFields`. `insert(object, rows[])` still returns the records, with no per-row slot. `strictReadonlyWrites` still refuses the whole batch before any outcome is built.
1212

13-
Graded `minor` in both packages: each widens a published method's declared answer with a new optional key (`InsertManyRowOutcome` gains `droppedFields`, and so does each outcome of `insertManyData`'s return type), which is an additive widening of the public surface. Nothing is removed, renamed or refused. The keys on the wire, `ValidateDataResponseSchema.results[].droppedFields` and `ImportRowResultSchema.droppedFields`, were already declared in `@objectstack/spec`. The REST import route does not copy the per-row report onto its row results yet.
13+
Graded `minor` in both packages: each widens a published method's declared answer with a new optional key (`InsertManyRowOutcome` gains `droppedFields`, and so does each outcome of `insertManyData`'s return type), which is an additive widening of the public surface. Nothing is removed, renamed or refused. The keys on the wire, `ValidateDataResponseSchema.results[].droppedFields` and `ImportRowResultSchema.droppedFields`, were already declared in `@objectstack/spec`.

‎content/docs/references/api/export.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ const result = CreateImportJobRequestSchema.parse(data);
230230
| **code** | `string` | optional | Error code (failed rows) |
231231
| **error** | `string` | optional | Human-readable error message (failed rows) |
232232
| **warnings** | `{ field: string; code: string; message: string }[]` | optional | Findings this deployment ADMITS rather than rejects (ADR-0104 value shapes under a warn-first posture), in the same `{ field, code, message }` shape the validate verdict carries. Set only on a dry-run row the verdict accepted: the row is `ok` because the write would store it and log the same complaint. A committed row never carries it, since the write path has no channel to report admitted findings on. |
233-
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A dry-run row reports the strips the write would make; a committed row reports the strips it made. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
233+
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A committed row reports the strips its write made. A dry-run row reports the strips its preview runs, which for a row the import would update excludes `readonlyWhen` and primary-key strips, so that row can name fewer fields than its commit. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server or write path that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
234234

235235

236236
---
@@ -358,7 +358,7 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo
358358
| **code** | `string` | optional | Error code (failed rows) |
359359
| **error** | `string` | optional | Human-readable error message (failed rows) |
360360
| **warnings** | `{ field: string; code: string; message: string }[]` | optional | Findings this deployment ADMITS rather than rejects (ADR-0104 value shapes under a warn-first posture), in the same `{ field, code, message }` shape the validate verdict carries. Set only on a dry-run row the verdict accepted: the row is `ok` because the write would store it and log the same complaint. A committed row never carries it, since the write path has no channel to report admitted findings on. |
361-
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A dry-run row reports the strips the write would make; a committed row reports the strips it made. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
361+
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A committed row reports the strips its write made. A dry-run row reports the strips its preview runs, which for a row the import would update excludes `readonlyWhen` and primary-key strips, so that row can name fewer fields than its commit. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server or write path that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
362362

363363

364364
---
@@ -377,7 +377,7 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo
377377
| **code** | `string` | optional | Error code (failed rows) |
378378
| **error** | `string` | optional | Human-readable error message (failed rows) |
379379
| **warnings** | `{ field: string; code: string; message: string }[]` | optional | Findings this deployment ADMITS rather than rejects (ADR-0104 value shapes under a warn-first posture), in the same `{ field, code, message }` shape the validate verdict carries. Set only on a dry-run row the verdict accepted: the row is `ok` because the write would store it and log the same complaint. A committed row never carries it, since the write path has no channel to report admitted findings on. |
380-
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A dry-run row reports the strips the write would make; a committed row reports the strips it made. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
380+
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key' \| 'computed'> }[]` | optional | Write-observability: caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own `droppedFields` shape and reason vocabulary. A committed row reports the strips its write made. A dry-run row reports the strips its preview runs, which for a row the import would update excludes `readonlyWhen` and primary-key strips, so that row can name fewer fields than its commit. The row still succeeds: `ok` and `action` are unchanged. Present only when at least one field was dropped. A server or write path that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped. In an async job's `results`, an ok row's drops reach the reader only if the row falls inside that capped, failures-first sample. |
381381

382382
### Nested Shape: `ImportRowResult.warnings[number]`
383383

0 commit comments

Comments
 (0)