feat(pr-review): align developer delivery with accepted goals - #4578
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
…d outcomes Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Author validation and delivery evidenceExact head: Changed surfaces: review policy/packet instructions, contributor task and PR forms, AGENTS.md, task routing and self-repair. The policy reuses the existing checker and
Coverage is sufficient for the changed consistency contract and public workflow: positive/negative delivery verdicts, policy compatibility and actual CLI packet/render/check paths are exercised. It does not certify a reviewer's semantic judgment or complete the parent roadmap. Future-facing simplification was applied by reusing evidence and removing duplicate maturity/word-count guidance. GitHub CI and independent review remain separate from this local validation. |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
评审 head:bdc09a546cd786ccd73501d67f1047aac66352cb;base:main(merge-base 81f435d6b)。本次发布使用的 packet 是 policy 5(当前安装的运行时),而 head 自报 policy 6;本 review 的结论只对当前 head 成立,合并后旧结果需按 policy 6 重新产生(PR 描述与 skill 都已写明)。
动机
PR 要解决的是仓库自身的开发者工作流缺口:agent 可以产出正确工件、通过 review,却没有证明它推进了被接受的这件事。旧契约(policy 5)用五段式 + 固定字数区间(200-350/300-500/450-800/250-500/150-300)来规范评审,却没有任何"这次变更是否推进被接受目标"的判定,于是"绿着通过但偏题"的 PR 可以合法地被 APPROVE。
代价是隐性且复利的:review 精力被字数而非证据牵引,而 off-goal 的变更一旦合并,浪费的是后续所有人的时间。作者没有选择再加一段说明文字(prose 无法阻止任何事),而是把判定放进既有 checker 的阻断项里——这是这个 PR 最关键的取舍,也是我认可它的原因。
改动思路
契约层(review_contract.py):REVIEW_POLICY_REVISION 5→6;decision_procedure 首位新增 establish_goal(先确认"当前被要求的产出",检查方向变更与既有相关工作,禁止把本仓库 roadmap 强加给其他仓库);problem_context 新增类型化 verdict(goal_achieved / justified_increment / off_goal / fragmented / not_yet_proven)与按 verdict 要求的字段;completion_gate.blocking_evidence_verdicts 增加 problem_context: [off_goal, fragmented, not_yet_proven];五个 section 的固定字数提示替换为"按证据需要;无最低字数"。
指令层:AGENTS.md 新增 Goal-Oriented Development(从任务到 PR 携带一份 compact delivery brief,选择完整可评审可回滚的切片),并明确写着"作者声明与这段 prose 都不能认证评审判定、也不结算 Goal";CONTRIBUTING、issue/PR 模板、capability README、loopx-pr-review skill(revision 门禁改 6)与 self-repair 参考同步。
边界守得住:没有新 packet schema、没有新 ledger/TS 权威、没有 LOC/PR 数/roadmap id 门槛,scheduler/quota/settings/runtime 权限不变。
具体改动
14 个文件、+373/-82:契约约 66 行、测试约 147 行、AGENTS 40 行、模板约 68 行,其余是 skill、capability README、贡献者文档与两个 smoke。
关键代码讲解
review_contract.py:7 REVIEW_POLICY_REVISION 5→6:这是"不改 packet 形状、只改评审要求"的版本位;skill 要求与实际 revision 相等,不相等时不得发 APPROVE(失败闭合,而不是静默降级)。
review_contract.py:189 problem_context.verdict_values / fields_by_verdict:把"是否推进目标"变成类型化判定,并规定 justified_increment 必须给出 remaining_gap/next_step/boundary_reason,三个阻断值必须给出 reason/minimum_repair。规则文本明确:按被验证的产出判定,不按文件/PR/测试数量;小 diff 不等于 fragmented;justified_increment 不必完成父目标。
review_contract.py:939 completion_gate.blocking_evidence_verdicts + verdict_policy.open_pr_unjustified_delivery:把上一步的判定接到真正的门禁上——"绿检查不能替代有证据的目标增量"。我用两个探针验证它真的有牙:把我上一轮真实的 #4573 结果改成 off_goal 后仍写 APPROVE → check-result 返回 ok=false, errors=[approval_contradicts_evidence](exit 1);删掉 verdict 字段同样被拒。
AGENTS.md:3 Goal-Oriented Development:把同一份 brief 带到所有 lane,并显式声明自身不认证判定、不结算 Goal、真实授权/成本/运维停止门保持不变;同时允许"characterization、前置项、研究、文档、维护"在消除已知缺口时算作有效产出(避免新规则变成规模配额)。
对主干的风险
最强回归是过度阻断:新判定挤压了"小而完整的切片"、前置项或 characterization 的合并空间,或者被当成规模/批次配额使用。契约里对这三件事都有反向约束(justified_increment 的存在、明确的"小 diff 不是 fragmentation"、以及禁止最低 LOC/PR 数/roadmap id),并且测试与"valid increments 仍可批准"的 parity 用例覆盖。
第二个风险是判定不可验证:checker 只能拒绝自相矛盾的结果,无法判断 reviewer 写的 goal_basis 是否诚实。这一边界在契约、AGENTS.md、PR 描述里都写明,我把它留在残余风险里,而不是假装门禁能证明它。
第三个是政策升级的连带影响:policy 5 的结果不再具备批准资格(PR 与 skill 已披露)。实测:capability 套件 97 passed / 12 skipped(12 条为 opt-in live-model,默认关闭,因此这次改动不声称模型可靠性提升);pr-review-command-smoke ok;docs-governance-smoke ok;.github/ISSUE_TEMPLATE/contributor-task.yml 仍可解析且原有 6 个必填字段(direction/intent/summary/scope/target_base/validation)全部保留。
P3(非阻塞):policy 升级要求 CLI 与已安装 skill 同步。如果某个 lane 只升级运行时、或只更新 skill,会遇到 revision 不匹配而必须停止发布批准。失败方向是安全的(不会静默批准),但建议在升级/发布说明里点明"release + skill 成对更新"。
我的整体评价
baseline(policy 5:无 delivery verdict、有固定字数)与 head(policy 6:类型化判定 + 阻断、字数按证据)对比:新增的是一个可执行的判定,而不是又一段说明;同时删掉了字数配额与重复的成熟度叙述,属于"压缩而非追加"。指令面、模板、skill、capability README 与契约在同一提交里对齐,失败方向全部是闭合(禁止批准)而非放行。
体量与收益匹配:契约增长集中在 packet 已渲染的文本,测试覆盖了新的负例;没有新增状态 owner、CLI 参数或权限面。repository_reuse: reused、typed_state_rule 为类型化枚举 + 门禁列表、default_off_isolation: isolated(唯一 opt-in 面是 12 条默认跳过的 live-model 用例)、authority_semantics: aligned(明确不结算 Goal、不授予运行时权限)、semantic_alignment: aligned(复用既有 evidence item 与阻断词表)。
结论 APPROVE,一条 P3 非阻塞。复评只需在 head 变化时重跑上述三组命令与两个探针。
English verdict: APPROVE - exact head bdc09a5; the batch turns "did this advance the accepted outcome" into a typed, machine-blocked delivery verdict inside the existing pr-review capability, aligns AGENTS/CONTRIBUTING/templates/skill with one delivery brief, and removes fixed review word counts without adding a state owner, size policy or authority. Verified independently: 97 passed / 12 skipped (opt-in live-model, default off), pr-review-command-smoke ok, docs-governance-smoke ok, contributor YAML keeps its six required fields, and two probes show check-result refusing an APPROVE whose problem_context is off_goal or omits the verdict (approval_contradicts_evidence). One non-blocking P3: the policy bump requires the release CLI and the installed skill to move together.
Goal And Delivered Outcome
Agents could produce correct artifacts and pass review without demonstrating progress on the accepted task. Connect task selection, implementation, handoff and PR review to one verifiable delivery outcome. Related to #4574; base:
main.problem_contextwith a goal basis and typed delivery verdict. The existing checker now blocks APPROVE foroff_goal,fragmentedandnot_yet_proven, even with otherwise green evidence.Scope And Continuation
Complete for the developer workflow and review-consistency contract. Semantic judgment remains the reviewer's responsibility; this does not automatically certify Agent behavior or settle a Goal. Online model qualification remains separate.
Review policy advances from 5 to 6, using the same packet schema. Old results require a fresh policy-6 review; queue selection, settings, scheduler and runtime authority are unchanged. The existing Python review capability owns this contract; no parallel TS state owner or delivery ledger is added.
Validation
bdc09a546cd786ccd73501d67f1047aac66352cbexamples/pr-review-command-smoke.py: real source CLI packet, checker, Markdown rendering and thin skill adapter.Coverage and gaps: no live provider invocation or claim of improved model reliability. Full-file formatting also reports inherited unrelated drift; changed Python lint passes and broad formatting churn is excluded. Exact-scope quality receipt and final premerge results are recorded in the validation comment.
User Entry Points And Boundaries
CLI review packet/rendering and GitHub contributor forms change. No dashboard/Lark settings or transport shape changes: they do not render these review sections, so no packaged frontend companion is required. No shared-authority provider fields, scoring, permissions or public/private evidence rules change.
Future-facing pass: reuse the existing evidence checker and canonical roadmap, remove duplicated guidance, and keep the review skill within its existing adapter-size gate.