fix(delivery): unify typed claim diagnostics before writeback - #4136
Conversation
|
Additional regression qualification completed on d19df6b: 160 Python tests passed across delivery claim/history/semantics, real refresh-state/history CLI, refresh/replan gates and quota settlement CLI (471 s). Separate focused post-edit run: 85 passed. Counts overlap and are not additive. The real CLI negative case covers both normal and dry-run rejection without filesystem mutations. No live Goal, provider routing or settled receipt was modified. No self-merge requested. |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Exact head: d19df6bf7aec06fee820bab34f2580699288df30
动机
这个 PR 要解决的问题成立:新写入不能同时声称“主要成果”和“阻塞/仍需 follow-through”,而历史上已经存在的矛盾记录也不能被静默解释成进展。基线把 outcome、turn kind、typed observation 分散在写入和历史读取路径里,长期会制造错误的进展统计与后续义务。把组合规则放到 TypeScript 的 delivery outcome owner,再由 Python 只做 transport/effect adapter,比补一段提示或复制 Python 判定更可靠。
改动思路
主方向是对的:diagnoseDeliveryClaim 产出三个 typed conflict;新写入经 work_item.delivery_claim.validate 在 effect 前拒绝,旧记录由 projectDeliveryHistory 降为 unknown 并暴露 delivery_claim_conflicts,不改写历史。正向路径中,primary_goal_outcome + typed blocked observation 会在 registry/artifact 写入前失败;合法 partial progress、blocker writeback 与 state-only refresh 仍可执行。
但 refresh 集成位置破坏了既有输入校验顺序。新 decorator 先调用 normalize_progress_observation,而基线是在 refresh_state_run 内先校验 delivery_outcome。当 outcome 和 observation 同时非法时,基线先报告 unsupported outcome,本 head 却先报告 observation schema。这不是本 PR 披露的“矛盾组合修正”,会让自动化得到不同 remediation。
具体改动
关键代码讲解
delivery_outcome.ts:29的diagnoseDeliveryClaim是正确的单一规则 owner:只看显式 enum/typed observation,不解析 prose,也不验证证据真伪。delivery_history.py:75的require_consistent_delivery_claim把 Python run facts 压缩后交给 TS,shape mismatch 与 invalid claim 都 fail closed;history.py的 reserved-run writer 和 refresh 路径都已接入。delivery_history.ts对历史矛盾返回unknown、空 follow-through 和 conflict codes,status.py将诊断保留到紧凑 readback,历史数据不被改写。state_refresh.py:127是当前阻塞点:raw kwargs 中的 observation 被提前 normalization,形成第二个 preprocessing order;同一新增代码还使state_refresh.py越过 repository maintainability ratchet 的 module metric budget。
最小修复是把 claim consistency check 放到既有 delivery outcome / progress observation 归一化之后、任何 registry/effect 之前,复用已归一化值;再加一个“两项同时非法”的 precedence 回归。不要通过放宽 ratchet 来隐藏这次增长,优先把 admission seam 放回最近的 bounded owner。
对主干的风险
我在 base b8837d84790c568772c26a956316f5307851cb0e 与本 head 上跑了同一个真实 refresh_state_run 反例:unsupported_outcome + bad progress_observation。base 返回 delivery outcome 允许值错误,本 head 返回 progress observation schema 错误,证明存在可观测语义漂移。另一个独立阻塞是 required maintainability smoke 明确报告 module_metric_budget:loopx/state_refresh.py,与远端 test-shard (2) / pytest / merge-gate 红灯一致。
其余验证为绿:75 个 focused Python tests、8 个 Node delivery-history tests、TypeScript typecheck 均通过。它们证明主规则和 no-write 路径,但没有覆盖重叠非法输入的 rejection precedence,因此不能抵消上述反例。PR 还落后当前 main,需要修复后 rebase 并在新 exact head 重跑。
我的整体评价
整体架构与问题规模相称,typed conflict 作为派生诊断、TS 单一决策 owner、历史只读兼容都值得保留;不需要扩大成新 capability 或持久 ledger。当前不能批准的原因不是风格,而是两个可复现的交付门禁:公开错误优先级发生未披露变化,以及 required maintainability ratchet 失败。修复这两点并补回归后,我会按完整 PR 而不是只看最后一个 commit 复审。
English verdict: REQUEST_CHANGES at exact head d19df6b — preserve refresh validation precedence and resolve the required state_refresh.py maintainability-ratchet failure; 75 Python tests, 8 Node tests, and TS typecheck pass, but the base/head counterexample and required CI gate fail.
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
d19df6b to
f23f242
Compare
Signed-off-by: huangruiteng <huangrt01@163.com>
Review resolution and design judgmentThe two findings at The refresh decorator introduced a second preprocessing order. It has been removed, not expanded into another admission framework. The original field normalizers run once; their results feed the existing TypeScript claim diagnosis. An explicit runtime lock surrounds the unchanged state-dependent admission/writeback body. This also removes a second registry read and makes the lock use the exact resolved runtime path used by the writer. Observable behavior disclosure: malformed input now precedes registry access/errors and lock creation, including dry runs. This is intentional: invalid requests should not touch storage. Valid requests retain state-dependent admission and persistence serialization. The new contradiction checks remain enforced write admission, not optional guidance; historical contradictions remain visible diagnostics without rewriting receipts. This does not waive Todo/replan/settlement obligations or alter the small-delivery floor. Scope and review lenses
ValidationExact head:
Public/private scan is clean. No live Goal, registry, lease, provider route or settled receipt was changed. No manual product hold remains for this bounded correction. The runtime entrypoint, historical compatibility, input priority, no-write paths, concurrency and mutation coverage form the risk-based premerge qualification; this is not a claim that all hosted CI has completed. Decision: approved for owner-authorized admin self-merge of this exact head. Both original findings are resolved without a budget exemption or a second authority boundary. |
Summary
refresh-stateand the reserved-run writer.Issue Or Task
Owner-requested delivery semantics refinement, stage 1 of 2. Stage 2 (#4137) reconciles historical supervision with current canonical waiting/work state. This PR does not remove the outcome floor.
Observable Behavior
New writes reject progress/preparation-only, primary-outcome/blocker and primary-outcome/explicit-follow-through contradictions. Historical conflicts become
unknownplus diagnostics without rewriting records or receipts. State-only refresh, valid partial progress, valid blocker writebacks and existing settlement fences remain supported.Malformed input now precedes registry errors and lock creation, including dry runs. This is intentional; independent field-error precedence is preserved. No new agent-authored declaration, evidence-truth validator or capability is introduced.
Validation
f66310806321b4c762565c144ba446ebe9a9350c(runtime unchanged fromf23f24294)Counts overlap and are not additive. No live Goal or registry was modified. AST comparison verifies that every state-dependent admission/writeback statement remains unchanged inside the explicit lock. The initial hosted mutation job stopped at an indentation-dependent CAS locator; the final test-only commit removes that incidental dependency. The CAS control passes and its deliberate regression is killed by an assertion. Final hosted CI is tracked separately from these completed local checks; pending jobs are not reported as passing.
Technical Direction
main; refactor(delivery): unify history-to-obligation rules in TypeScript #4134 has merged.Boundary Checklist