Skip to content

Import dry run green-lights a row the write then rejects: structured value shapes (address / location) are not pre-checked #4633

Description

@os-zhuang

Found while authoring HotCRM's first mapping artifacts (objectstack-ai/hotcrm#603). Not blocking that work — filing so the divergence is recorded rather than rediscovered.

What happens

A CSV cell aimed at a structured address (json-backed) field passes the dry run and fails the real write.

Measured on 17.0.0-rc.1, POST /api/v1/data/crm_account/import, mapping Billing Address to billing_address (a Field.address):

dryRun: true, runAutomations: false

{ "total": 1, "ok": 1, "errors": 0, "created": 1,
  "results": [ { "row": 1, "ok": true, "action": "created" } ] }

the same payload, dryRun omitted

{ "total": 1, "ok": 0, "errors": 1, "created": 0,
  "results": [ { "row": 1, "ok": false, "action": "failed",
    "code": "VALIDATION_FAILED",
    "error": "Billing Address has an invalid address value: Invalid input: expected object, received string" } ] }

Nothing is corrupted — the engine rejects the string per row, loudly and readably. The problem is only that the dry run promised otherwise.

Why it matters

The dry run's stated contract is that it predicts the verdict the real write produces. import-coerce.ts says so twice, and both firstMissingRequiredField and firstConstraintViolation exist specifically to close that gap:

> Mirrors the numeric-range and string-length rules of the engine's validateRecord — same type applicability, same comparison, same code and message text — so the import's dry run predicts the verdict the real write produces (framework#3956). Before this, a dry run only reported coercion failures … so -500 in a min: 0 column was reported valid and then rejected by the write.

The engine's value-shape validation (valueShapeStrictFor / mediaValueShapeStrictFor) is a third such rule, and it has no pre-check counterpart. coerceCell routes address / location through its final catch-all — "Everything else (text, email, phone, json, html, single file, …): pass through" — so no verdict is formed at all.

This is precisely the "false all-clear" #3956 set out to eliminate, and it lands on the user who did the right thing: dry-run a 5,000-row file, get a clean report, run it, watch a column's worth of rows fail.

Suggested shape

Add a value-shape pre-check beside firstConstraintViolation, rendering the same code / message the engine's own rejection renders, so the two read identically. Scope it to the same "engine's OWN list" discipline the bounds check already follows — only the types validateRecord actually shape-checks — so it does not start false-alarming on types the write accepts.

Not the same thing

Composing an address object from separate Street / City / Postcode columns is a feature the mapping spec does not have (no object-building transform), and that is a defensible gap, not this bug. This issue is only about the dry run and the write disagreeing on the payload a caller did send.


裁决后状态(2026-08-06,cli 车道 PM 按裁决 D 拆分): 本单收缩为 cli 消费半边(rest/import 走新 validate-only 操作 + import-coerce.ts 手抄预检镜像退役);契约半边(DataProtocol 新增 validate-only 操作,spec 先行)已拆出 #6037。

Blocked-by: #6037

Activity

  1. claude commented on Aug 3, 2026

    @claude
    Contributor

    分诊纠偏:修复落点实测为 packages/rest/src/import-coerce.ts(dry run 预检镜像),按 Domain lanes 锚定规则(域=修复落地包,非标题词汇)应为 domain:cli(rest 暂归类所在),原 domain:engine 标签有误,已更正。归 cli 车道 PM 视野。


    Generated by Claude Code

  2. baozhoutao commented on Aug 5, 2026

    @baozhoutao
    Contributor

    🔒 认领:PM 循环第 4 轮(cli 车道)
    会话:session_01VkPSGsX9o17MsGv3Lbxu2w
    分支:claude/issue-4633-import-dryrun-value-shape
    Worktree:objectstack-issue-4633
    域:domain:cli
    文件面:packages/rest/src/import-coerce.ts(分诊纠偏实测落点)+ 测试 + changeset。与同批 #5387(doctor.ts)、#5244(examples 注释)不相交;#4886 刚在同包 rest-server.ts 落地,文件不同、无 barrel 交集,以 origin/main 为准。

    执行口径:按 issue 建议在 firstConstraintViolation 旁加 value-shape 预检,镜像引擎 validateRecord 自己的类型清单(同类型适用性、同 code、同 message 文案 —— #3956 的既有纪律),不对引擎实际接受的类型误报。issue 明示的边界照守:对象拼装(Street/City 拼 address)是 mapping spec 缺 feature,不在本单。


    Generated by Claude Code

  3. baozhoutao commented on Aug 5, 2026

    @baozhoutao
    Contributor

    [决策] dry run 该如何预测姿态相关的 value-shape 判决 —— 转维护者,挂 needs-user-decision

    第 4 轮派发的 dev 复核后按 scope gate 停下(零推测代码,worktree 已清)。关键新事实,推翻 PM 派发裁定的一半:写入侧的拒绝不是无条件的 —— record-validator.ts:515-548 仅在 ADR-0104 姿态为 strict(新库自证 / os migrate value-shapes --apply / env 强制)时 fail;未迁移部署走 warn-first,警告后照收该行。同一行两种姿态实测双向证实(strict:write errors:1;lenient:write created:1 + accepted for now 警告)。因此「无条件把 shape 预检折进 firstConstraintViolation」会在每个未迁移部署上让 dry run 报 failed 而 write 报 created —— 镜像假缺陷,恰是该函数 doc 注释点名要避免的形态。

    四选项(dev 给出,PM 转录;完整论证见其报告,以下为压缩):

    • A — 镜像姿态:给 DataProtocol 加可选姿态探针(如 getValueShapePosture(objectName)),预检按真实姿态判。两种姿态下 dry run 与 write 一致。代价:动 spec 公共协议 + metadata-protocol + objectql(私有 helper 要导出)+ import-runner,四包。
    • B — 无条件拒:字面执行原裁定。代价:未迁移部署全员镜像假警报,并把 dry-run 契约从「预测写入判决」静默改成「预测 strict 写入的判决」。建议直接否决。
    • C — import 面永远 strict(dry run 与 write 同 strict,理由:warn-first 的宽限是给存量数据的,不是给 import 即将新增的行;每行被收下的坏值都是阻止部署自证的新反例 bug(objectql): ADR-0104「空库即已迁移」自证写在首启 seed 之前 —— 部署证明了一个它同一次启动就违反的契约,第二次 pnpm dev 起永久 10 条 ERROR #4769)。代价:import 比其它写入面更严,新的不一致;write 侧也要加检查。
    • D — 经引擎验证:新增 validate-only 协议操作(validateData / createData 的 dryRun 旗标),dry run 直接跑 validateRecord 不落盘。一次关掉整个「dry run 预测不了」家族(value shapes、format 检查、对象级 validations、状态机),import-coerce.ts 自己的注释已点名这是正确终态('Closing those means validating through the engine itself rather than growing this copy')。代价:最大改动,新公共协议操作;5000 行 dry run 逐行过引擎(验证在进程内、无 driver I/O,可缓解)。

    dev 推荐 D(A 为垫脚石),cli 车道 PM 附议:两轴上 D 都是唯一去掉「第三份手抄镜像」的选项 —— 一个谓词、一个位置、在生产者处;AI 写错 mapping 在 dry run 时拿到引擎自己的具名拒绝,不存在第二份「何为 malformed」可以被后来者只更新一半。B 在防 AI 轴上最坏:假警报教会作者不信任 dry run,而那是 AI 生成的 5000 行导入前唯一的门。D/A 均动 DataProtocol 公共契约(spec 单一所有者面),故不由车道 PM 拍板。时序注:加性协议操作,不占 v17 breaking 窗口,可 17.x 排。

    裁决后去向:D → 立 program 化的 spec 子单链(契约先行);A → 同样 spec 先行四包序列;C → import 面双侧 strict 单独立单。本单退出派发队列待裁。


    Generated by Claude Code

  4. 5 remaining items

  5. os-project-manager commented on Aug 8, 2026

    @os-project-manager
    Collaborator

    Label repair: pm:queue → pm:blocked (paired comment, domain:cli seat, session session_017uFVNMmTxLpmfQYiuKM1Yx, 2026-08-08).

    The body has carried Blocked-by: #6037 since the 2026-08-06 contract-first split, but the labels still read pm:queue — so this card showed up as dispatchable in every sweep, including this shift's round-1 batch selection, while it cannot legally be dispatched. The machine half and the human half had drifted apart; this restores them.

    No other labels touched (bug and domain:cli preserved — the write is a whole-set PUT, so they are re-sent verbatim rather than dropped).


    Generated by Claude Code

  6. os-project-manager commented on Aug 8, 2026

    @os-project-manager
    Collaborator

    Unblocked — pm:blocked → pm:queue (unlock sweep, domain:cli seat, session session_017uFVNMmTxLpmfQYiuKM1Yx, 2026-08-08 02:0xZ).

    The blocker cleared: #6037 closed as completed at 02:00:23Z, and its contract half is on origin/main as feat(spec,objectql,metadata-protocol): validate-only data operation — DataProtocol.validateData (PR #6474, commit 18189983d). Verified against origin/main, not from the issue's own claim.

    This card is a candidate again. Two notes for whoever dispatches it — the second one matters more than it looks:

    1. Re-price before dispatching; do not reuse the filing-time estimate. This card's scope was written when the fix had to be a hand-copied pre-check mirror. The ruling (D) replaced that with a protocol operation, and that operation now exists. So the consuming work is no longer "mirror the engine's value-shape rules in import-coerce.ts" — it is "call validateData and retire the mirror". Whether that makes this cheaper, or moves work into it, depends on what #6474 actually shipped versus what the dry-run path needs (ADR-0104 posture semantics in particular: the ruling requires the validate-only verdict to follow the deployment's real posture, strict or warn-first, matching the real write — it explicitly rejected unconditional strict). Read #6474's landed surface first, then size this.

    2. The acceptance hard clause from the ruling still binds this half. #6037 carried it — declaration and enforcement land in the same PR, the #4052 lesson (batch.zod.ts's validateOnly was retired precisely as a "promised, never implemented" key). On this side that reads: the dry run must actually route through the new operation, not merely gain the ability to. A dry run that still predicts with its own logic while a validateData operation sits unused would be the same defect wearing the fix's clothes.

    Labels: pm:blocked removed, pm:queue restored; bug and domain:cli re-sent verbatim (the write is a whole-set PUT). No assignee — unclaimed and available.


    Generated by Claude Code

  7. os-project-manager commented on Aug 8, 2026

    @os-project-manager
    Collaborator

    Claim: PM loop round 2 (domain:cli seat, 2026-08-08 shift) — dispatched immediately on unblock, at the maintainer's "加速" instruction
    Session: session_017uFVNMmTxLpmfQYiuKM1Yx
    Branch: claude/issue-4633-import-dryrun-validate-data
    Worktree: objectstack-issue-4633
    Domain: domain:cli
    File surface: packages/rest/src/import-coerce.ts + packages/rest/src/import-runner.ts (the dry-run path) + their tests + one .changeset/*.md (stop on breach; explain in the report)
    Serial constraints cleared — this one needed an active intervention, not just a check:

    Re-priced on unblock, per the note above — not dispatched at the filing-time estimate. The upstream (#6037 → PR #6474, DataProtocol.validateData) landed at 02:00Z, which changes what this half is: not "mirror the engine's value-shape rules in import-coerce.ts" but "route the dry run through the engine's own validation and retire the mirror". The dispatch carries the full ruling chain — D adopted, B (unconditional strict) explicitly vetoed because warn-first deployments would get false alarms, A available only as an internal stepping stone — plus the spec seat's 2026-08-07 cross-seat note that valueSchemaFor(def, form) is already public on main, so nothing further is being waited on.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions