Skip to content

feat(spec): declare the per-row write-drop signal on the import and validate answers, and the served import warnings - #20930

Merged
os-justin merged 3 commits into
mainfrom
claude/issue-20921-import-row-dropped-fields
Sep 30, 2026
Merged

os-justin merged 3 commits into
mainfrom
claude/issue-20921-import-row-dropped-fields

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #20921

Clause-②: yes (widening)

  • ImportRowResultSchema and each ValidateDataResponseSchema.results[] row gain an optional droppedFields: z.array(DroppedFieldsEventSchema), the reference BatchOperationResultSchema.droppedFields already uses. No second reason enum: the set is the engine's, computed included.
  • ImportRowResult.warnings is declared as served: z.array(ValidateDataIssueSchema), set on ok dry-run rows only. Producer packages/rest/src/import-runner.ts:852; it reaches the sync route and the async job results; zero readers in objectui at 741864f7b9. Undeclared, parse stripped it.
  • ImportJobResultsSchema.results describe: a capped sample, failures first; an ok row's report reaches the async reader only inside it. The cap is unchanged.
  • Pins in export.test.ts and validate-data.test.ts. Ablation at 3e9b7ece06 (each key renamed out of its schema, restore proven by blob hash): 4 / 3 / 1 tests red; control 53/53 green.
  • Local at 3e9b7ece06: spec test 583 files, 17177 passed; spec typecheck exit 0; check:generated 15 of 15 current; 104 of 108 derived gates exit 0, 4 NOT MEASURED (they need whole-workspace builds; CI runs them).
  • Changeset: @objectstack/spec minor. It names objectui's STRIPPED_LINE toast table as the exhaustive reason consumer.

Acceptance notes


Generated by Claude Code

…alidate answers

ImportRowResultSchema and each ValidateDataResponseSchema row gain an
optional droppedFields: an array of DroppedFieldsEventSchema, the engine's
own strip event, so the reason vocabulary (computed included) is the
engine's and no second enum exists. ImportRowResultSchema also declares the
dry-run warnings the REST import runner already serves, in the
ValidateDataIssue shape; undeclared, parse stripped them and typed readers
could not see them. ImportJobResultsSchema.results now says an ok row's
report reaches the async reader only inside the capped, failures-first
sample. The cap is unchanged.

Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1
Co-authored-by: Claude <noreply@anthropic.com>
…clared import warnings

Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1
Co-authored-by: Claude <noreply@anthropic.com>
… projection

Generated only: gen:docs for the two describe-bearing schemas, and the
authorable-surface/api.json rows the build's schema step writes for the two
new ImportRowResult keys.

Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

6 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/spec/authorable-surface/api.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/authorable-surface/api.json) — pages documenting those are invisible to this run
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 137 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json aaad682dbcb36bd00635a40530aca826062b9903 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4a58b0b9efe64580bb1aa3bff279a485f952b06b — the merge of head 3e9b7ece066ca4b8ad04ad5e73ae7ed186c6ea93 into base aaad682dbcb36bd00635a40530aca826062b9903, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4a58b0b9efe64580bb1aa3bff279a485f952b06b && git checkout 4a58b0b9efe64580bb1aa3bff279a485f952b06b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin aaad682dbcb36bd00635a40530aca826062b9903 3e9b7ece066ca4b8ad04ad5e73ae7ed186c6ea93 && git checkout -B drift-repro aaad682dbcb36bd00635a40530aca826062b9903 && git merge --no-ff 3e9b7ece066ca4b8ad04ad5e73ae7ed186c6ea93

node scripts/docs-audit/affected-docs.mjs --json aaad682dbcb36bd00635a40530aca826062b9903

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 3e9b7ece066ca4b8ad04ad5e73ae7ed186c6ea93
Local-runs: none

Reviewed: PR #20930 (8 files, +247 / -4, three commits dbf343ddea / 15c9bfedf5 / 3e9b7ece06) for card #20921, read as the net diff against main (87847a27ce; merge base 4d0b9cd542; the diff is byte-identical to the two-dot diff from the merge base, and main has not moved any of the eight files since). Inputs: the card's body and its three comments (claim 5917907584, dev report 5918850206, ruling 5918893030), #20701 (scope 5906762084, the maintainer's 「20701 同意」 in 5909519064, the split 5917724406), #20922, the PR body, file list and diff, and the check-runs on the head. PR #20805 answers 404 on the unauthenticated pulls endpoint; its computed arm is read off main instead (packages/spec/src/data/data-engine.zod.ts:310, packages/objectql/src/engine.ts:12350). Nothing was checked out, built, run or re-run.

① Derived judgments

Accept-set and public-surface changes the diff implies

  1. ImportRowResultSchema (packages/spec/src/api/export.zod.ts:342-366) gains droppedFields: z.array(DroppedFieldsEventSchema).optional(). Right. The element is the engine's own schema imported from ../data/data-engine.zod (the same form api/batch.zod.ts:217 uses for BatchOperationResultSchema.droppedFields), so the reason set is the engine's ['readonly', 'readonly_when', 'primary_key', 'computed'] (data-engine.zod.ts:310); readonly_when is spelled nowhere else under packages/spec/src, so no second vocabulary exists. The pins assert element identity with toBe(DroppedFieldsEventSchema) and walk DroppedFieldsEventSchema.shape.reason.options, so they follow the enum rather than restating it. Because ImportResponseSchema.results (export.zod.ts:383) and ImportJobResultsSchema.results (:469) both embed the row schema, the key lands on the sync answer and the async results answer in one declaration.
  2. Each ValidateDataResponseSchema.results[] row (packages/spec/src/api/protocol.zod.ts:2142-2148) gains the same optional key. Right. protocol.zod.ts:20 already imported DroppedFieldsEventSchema, so the diff adds no import there; the row pin asserts the same element identity.
  3. ImportRowResultSchema declares warnings: z.array(ValidateDataIssueSchema).optional(). Right, and it is what the REST runner serves today. Producer: packages/rest/src/import-runner.ts:852 (the one warnings writer in packages/rest/src outside tests), inside the dryRun branch, on the created / updated rows the verdict accepted, copying verdict.warnings, which is ValidateDataIssue[] (import-runner.ts:60; the engine builds them as ValidateDataIssue at engine.ts:12413-12449). Both routes carry it: the sync route answers results: summary.results verbatim (rest-server.ts:9139); the async worker persists capImportResults(summary.results) (:9330), which filters and slices row objects without reshaping them (:842-848), and the results route returns the stored items verbatim (:9487). So the served shape is exactly { field, code, message }[], which is what the schema now declares.
  4. Derived accept-set narrowing from declaring keys on a non-strict z.object: a malformed warnings or droppedFields value, which parse used to strip silently, is now refused (pinned: a partial issue fails at warnings.0.message; an outside reason fails at droppedFields.0.reason / results.0.droppedFields.0.reason with invalid_value). Right, and not a narrowing of anything served: the only producer emits ValidateDataIssue[], and no server sets droppedFields on either payload today (git grep droppedFields on main in import-runner.ts and the import routes: none; validateData at packages/metadata-protocol/src/protocol.ts:11800-11806 passes mode and context only, no listener).
  5. ImportJobResultsSchema.results describe rewritten to a capped, failures-first sample. Right: IMPORT_JOB_RESULTS_CAP = 500 (rest-server.ts:831), failures first by [...failures, ...successes].slice(0, CAP) (:844-846), resultsTruncated from the stored truncated flag (:9487), and the job counters come from the runner's per-row tallies. No REST file is in the diff, so the cap is unchanged, as the card's scope 4 requires.
  6. Exported types: ImportRowResult (z.input) gains two optional members and the validate row type gains one. expectTypeOf pins the element types to DroppedFieldsEvent and ValidateDataIssue. Additive; no removal or rename anywhere in the diff.
  7. packages/spec/authorable-surface/api.json gains api/ImportRowResult:droppedFields and api/ImportRowResult:warnings. Right: the projection lists top-level keys of named schemas (api/ImportRowResult:action … already present; api/ValidateDataResponse:results is the row for the validate answer, whose nested row key is not a projection row), so two rows and no third is the correct regeneration. authorable-surface.base.json untouched is consistent with an additive change.
  8. content/docs/references/api/export.mdx and protocol.mdx are regenerated from the describes; every sentence there is judged with its source describe below.

Author-shown and AI-facing text, sentence by sentence

  • export.zod.ts TSDoc, "warnings was SERVED before it was declared … both the synchronous route and the async job's results carry it (pinned at the wire by packages/rest/src/import-dryrun-parity.test.ts)". The served fact is true on main today (item 3). The pin parenthetical is over-broad: import-dryrun-parity.test.ts:200-207 pins the synchronous route only; no test under packages/rest/src reads warnings off the async results route, so the async half rests on the code reading in item 3, not on a wire pin. Not verdict-changing: the statement of fact holds; only the attribution of the pin is wider than the test.
  • export.zod.ts TSDoc, "Undeclared, it was invisible to every reader typed by this schema, and ImportRowResultSchema.parse stripped it." True: the schema is a plain non-strict z.object from zod, and the exported type is z.input of it.
  • export.zod.ts TSDoc, "droppedFields is DECLARED AHEAD of its producer … The engine has to report drops per row out of validateData and insertMany, and the REST import route has to copy them onto the row; until both do, no server sets the key". True on main (item 4) and true when this PR lands; it becomes a served key only after engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922 and 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 1. It matches engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922's scope in words and names no tracker number.
  • droppedFields describe (import row), "caller-supplied fields the engine LEGALLY strips from THIS row, one event per reason, in the engine's own droppedFields shape and reason vocabulary". True by the element identity. "A dry-run row reports the strips the write would make; a committed row reports the strips it made." True only after both producers land; on main when this PR lands no row reports anything. The same describe carries its own guard, "A server that does not produce this report omits the key too, so an absent key alone does not prove nothing was dropped", which is the reading an absent key may bear today, and it forbids the reading it may not bear (absent = nothing dropped). "Present only when at least one field was dropped" is a producer obligation in the form batch.zod.ts:217 already states (omit when empty); the schema itself parses an empty array, so it binds the producer, not the parser. "The row still succeeds: ok and action are unchanged" and the async-sample sentence are true (items 5, 6).
  • warnings describe, "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 … A committed row never carries it, since the write path has no channel to report admitted findings on." True on main: the single writer is in the dry-run branch on accepted rows; the committed-row results at import-runner.ts:855-870 and the bulk flush carry no warnings member; the runner's own interface docblock (import-runner.ts:53-59) says a real write has no equivalent channel, and no write response schema in packages/spec/src carries warnings (only ValidateDataResponse does).
  • ImportJobResultsSchema TSDoc and describe: true (item 5). "resultsTruncated says whether the sample is partial, and the job counters count every row" matches :9487 and the runner's tallies.
  • protocol.zod.ts TSDoc, "the engine's validate already runs the write's strips and reports them through the write's own listener, but validateData does not yet forward that listener or attribute its events to rows, so no server sets the key until it does." The forwarding and attribution halves are true on main (protocol.ts:11800-11806 forwards no listener; engine.ts:12303-12351 accumulates readonlyDropped over every row and computedStrip.dropped set-wide, then emits ONE event per reason for the whole set). "the write's strips" is over-broad: the preview runs the undeclared-field door, the computed strip and the caller-write strips (runtime-owned and static readonly on insert, stripReadonlyFields on update), and by its own comment (engine.ts:12297-12302) does not run readonlyWhen or the primary-key strip, so the listener it feeds reports computed and readonly only. The sentence's conclusion (no server sets the key) is unaffected. "exactly as errors / warnings share ValidateDataIssueSchema with a rejected write" is true (protocol.zod.ts:2081-2085).
  • droppedFields describe (validate row), "caller-supplied fields the write would LEGALLY strip from THIS row … A strip is not a finding: valid is unaffected, and the fields appear in neither errors nor warnings." True by construction on main: the strips remove the keys before validateRecord runs, and drops leave the engine only through the listener, never as issues. The "would" tense is the right one for a preview; the per-row claim is true only once engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922 attributes per row (see ③), and the absent-key guard sentence is present here too.
  • Changeset, "The import row report and the validate-only answer can now say which fields a write drops." True of the contract (the schema can carry it), not of any server until both producers land; the body's own "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" states that moment. "So the reason vocabulary is the engine's (readonly, readonly_when, primary_key, computed), and there is no second enum." True on main. The warnings paragraph and the ImportJobResultsSchema.results paragraph are true (items 3, 5). "A consumer that branches on reason must stay exhaustive over DroppedFieldsEvent['reason']" restates the engine schema's own docblock (data-engine.zod.ts:264-267). "The known exhaustive consumer is objectui's write-warning toast table, STRIPPED_LINE in packages/app-shell/src/providers/writeWarningToast.ts, which covers all four reasons today" — unsourced within this review's inputs: the objectui tree is outside them; the dev measured it at objectui 741864f7b9 and the card's pin asks only that the consumer be named, which it is. The type-check sentence about a table keyed by the union is a TypeScript fact for a Record over the union, the form the engine docblock prescribes. The body carries no tracker number and no model identifier.
  • PR body: bullets 1-3 true (items 1-5; import-runner.ts:852 is the producer's line on main); "zero readers in objectui at 741864f7b9" is outside the inputs, as above. The ablation and local-run figures (4 / 3 / 1 red, 53/53 control, 17177 passed, 104 of 108 gates) are the dev's local measurements, not re-run here; the head's Test Core (1-6), Lint & Repo Gates and the four Type Check runs corroborate the suite and the gates, not the ablation. The four NOT MEASURED gates the report names all sit in Lint & Repo Gates or Type Check, both green on the head. Acceptance note, "Until both land no server emits droppedFields, and both describes say an absent key does not prove nothing was dropped": true on main (item 4) and the describes say it. Clause-②: yes (widening): right (②).
  • Regenerated docs: the export.mdx rows and the two nested shapes, and the protocol.mdx row and results type, are the describes above verbatim; the same judgments apply. The docs-drift run on the merge commit found no hand-written page naming the changed anchors; api.json yielding no anchor is a projection, not a page.

② Semver level

.changeset/20921-import-row-dropped-fields.md grades @objectstack/spec minor. The diff publishes three new optional keys on published payloads (two on ImportRowResult, reaching ImportResponse.results and ImportJobResults.results; one on ValidateDataResponse.results[]) and a describe rewrite, and removes or renames nothing. A purely additive widening of a published package's public surface takes at least minor (the WHICH LEVEL rule, pr-automation.yml:753; AGENTS.md's "yes takes at least minor"), and only packages/spec source moves, so one changeset on one package is the whole grade. Not breaking, so no ADR-0087 marker is owed and none is written; the changeset body cites no tracker number.

Clause-②: yes (widening) — the PR body's line, corrected from the claim's no by ruling 5918893030; it is the literal answer under check-changeset-no-major.mjs's definition ("a new key on a published payload"), the widening arm is the only arm that applies (the sole refusals the declaration adds are of malformed values nobody serves), and the head's Check Changeset run is success. Level and line match what the diff publishes.

③ Boundary flags

  • open_questions[0] — the Clause-② line (no vs yes (widening)). Answered by the ruling: B. The PR body reads Clause-②: yes (widening), judged right in ②; the level axis is satisfied by minor and the Check Changeset run is green.
  • Deviation — the changeset filename carries the card number. Accepted, not a breach: the .changeset/ISSUE-SLUG.md form is what pr-automation.yml:747 and check-empty-changeset.mjs:599 prescribe, and the no-tracker-number rule covers prose, runtime strings and rules; the changeset body, the describes and the TSDoc carry none.
  • Deviation — the PR body copied the claim's Clause-② line verbatim. Superseded by the ruling's edit; the line now reads yes (widening).
  • out_of_scope_findings[0] — engine.validate reports one union event per reason across ALL rows. Confirmed on main (engine.ts:12303-12351: readonlyDropped accumulates over every row, computedStrip.dropped is set-wide, one push per reason). Consequence for this PR: the per-row key it declares on the validate answer cannot be filled by forwarding the listener alone; engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922 must attribute drops per row, which its scope item 1 ("forwards a drop listener … Its answer carries the drops") does not yet spell out. The protocol.zod.ts TSDoc already says "or attribute its events to rows", so this contract is not wrong; the ruling carries the pointer to engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922 at landing, and engine: validateData forwards engine.validate's onFieldsDropped and answers the drops, and insertMany reports drops per outcome through insertManyData: #20701 item 1, engine half #20922 has no comment yet at this read — escalated: that pointer is owed by the seat when this lands.
  • Over-broad sentences (①): the TSDoc's "pinned at the wire" covers the sync route only, and "runs the write's strips" omits that the preview skips readonlyWhen and the primary-key strip. Neither changes the contract or the verdict; both can be tightened when 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 1 pins the async route, or in a docs-only edit.
  • objectui claims (the STRIPPED_LINE table and "zero readers"): outside this review's inputs; carried on the dev's measurement at 741864f7b9. The card requires the consumer to be named in the changeset, and it is.
  • File surface: eight files, all inside the claim's surface (the two schema files and their tests, generated projections regenerated only, the changeset); no engine or REST code. Liveness: the dev reports the ledger does not govern response schemas; the Spec property liveness run is success.
  • Check-runs on 3e9b7ece06, read last, deduped by name keeping the newest started_at: 35 names, all completed; 31 success (Build Core, Build Docs, Check Changeset, Check Documentation Links, Dogfood Regression Gate and its 3 shards, Dogfood Verify CLI, Flag docs affected by code changes, Governed Surface Queue Guard, Lint & Repo Gates, the three claim guards, Part-of PR must not also close its card, Spec property liveness, Temporal Conformance, Test Core and its 6 shards, Type Check · consumer gates / debt ledger / source gates / workspace, TypeScript Type Check, filter); 4 skipped (Auto Label, Check PR Size, Console Pin Gate — the paths leave the console filter false — and Packed-tarball smoke (opt-in) — its label is absent). None failed, none still running.

Implemented-by: claude/issue-20921-import-row-dropped-fields
Reviewed-by: session_01Sfe5YjBLwB9J3y8fvm2xq1

VERDICT: PASS

Adopted and posted by domain:spec seat 5 (session_01Sfe5YjBLwB9J3y8fvm2xq1) · 2026-09-30T20:39Z · rendered by the seat's at-tier review subagent on this head. The seat read its served tier family from the subagent transcript before posting.


Generated by Claude Code

@os-justin
os-justin marked this pull request as ready for review September 30, 2026 20:41
@os-justin
os-justin added this pull request to the merge queue Sep 30, 2026
Merged via the queue into main with commit 95555e7 Sep 30, 2026
51 checks passed
@os-justin
os-justin deleted the claude/issue-20921-import-row-dropped-fields branch September 30, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants