Skip to content

feat(pr-review): declare the reviewer and bind the review to its specification - #5419

Merged
huangruiteng merged 14 commits into
loopx-project:mainfrom
songoow:codex/pr-review-self-evidence
Oct 2, 2026
Merged

huangruiteng merged 14 commits into
loopx-project:mainfrom
songoow:codex/pr-review-self-evidence

Conversation

@songoow

@songoow songoow commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

  • Outcome basis / optional anchor: review provenance and specification binding. A published review in this project is routinely written by an agent, and increasingly read by another operator's agent with no shared context, memory or host.
  • Goal/source and gap: today a review can be published without saying who wrote it, and without naming the specification the change was judged against. The identity of the reviewing model, and the mapping from an accepted RFC's criteria onto the diff, exist only in prose — or not at all. Neither is machine-readable, so a reader on another host cannot weigh the review or open the same specification text.
  • Observable before → after, with the validation row that proves it: build_review_plan previously had no reviewer field and no spec mapping. loopx pr-review --check-result now rejects a result whose reviewer declaration is missing, malformed, or absent from the published body, and rejects a mapped spec basis that omits a criterion, cites no source, or carries a not_met criterion alongside APPROVE. Proven by the new cases in tests/capabilities/test_pr_review_result_check.py.
  • Issue/task and intended base: Related to the /loopx-pr-review review-contract discussion. Base main at 3c50e59c0.

Author Declaration

  • Written by: model_agent — Claude Opus 5.5, Anthropic

Implemented against

  • Specification and revision: the accepted review-execution contract owned by loopx/capabilities/pr_review_queue/review_contract.py (pull_request_review_execution_contract_v2, policy revision 13 at base, bumped to 14 here), plus AGENTS.md "PR Review Comments" and "Public And Private Boundary", and the maintainer review rounds on this PR (pullrequestreview-5388397352, pullrequestreview-5389107437); judged at base 3c50e59c0.
  • Criteria:
Criterion (spec clause) Disposition Symbol / path Test or command
Contract must state who reviewed and for whom the declaration is evidence implemented review_contract.py::REVIEWER_DECLARATION, build_review_execution_contract tests/capabilities/test_pr_review_contract.py
A review must bind criteria to the spec it claims, not to its own narrative implemented review_contract.py::SPEC_BASIS_ASSESSMENT tests/capabilities/test_pr_review_contract.py
Declaration must reach the published body, since the structured result stays local implemented result_check.py::_reviewer_errors, review_body.py::reviewer_declaration_lines test_pr_review_result_check.py::test_review_must_say_which_model_wrote_it_in_the_published_body
not_met criterion cannot coexist with APPROVE implemented result_check.py::_check_spec_basis test_spec_basis_cannot_approve_with_incomplete_or_contradictory_mapping
Already-published reviews must not be retroactively invalidated implemented review_body.py — check_review_body deliberately unchanged tests/capabilities/test_pr_review_body.py, examples/fixtures/pr-review-history/
Mapped spec_ref, spec_revision and every criterion_id must be published text a remote reader can find implemented result_check.py::_check_spec_basis, _unpublished_spec_references, _published_as_token test_a_mapped_specification_reference_must_be_published_text, test_a_criterion_identity_must_be_published_text, test_a_specification_reference_must_appear_as_a_whole_token
A repository spec (accepted_rfc, accepted_contract_doc) is pinned by a full commit id, not a branch or tag implemented review_contract.py::SPEC_BASIS_ASSESSMENT.commit_pinned_spec_sources test_a_repository_specification_is_pinned_by_a_full_commit_id
No private runtime detail (endpoint, account, credential) may be published implemented result_check.py::_DECLARED_NAME_FORBIDDEN, _PUBLISHED_LINE_FORBIDDEN test_reviewer_declaration_rejects_unknown_or_infrastructure_values
  • Self-check before submission: read review_contract.py, result_check.py, review_body.py, pr_review.py and skills/loopx-pr-review/SKILL.md before editing, plus docs/architecture/rfcs/ for the roadmap id convention. Confirmed by reading call sites that pr_review.py::_review_conclusion uses check_review_body to recognize reviews already published on GitHub, which is why the new body requirement is confined to the pre-publication path. Left out deliberately: the coverage ledger and the review-scoped time/token receipt discussed as the next slice; they touch packet serialization and the usage collector for a separate reason.

Scope And Continuation

  • Completed scope and remaining work: the reviewer declaration and the specification binding are complete — contract, validation, publication path, tests and both teaching surfaces.
  • Slice boundary / successor: a staged increment. Remaining, as an unscoped follow-up: a per-review coverage ledger (which changed files were read, which surrounding owners were named) and a review-scoped duration/token receipt. Both need a runtime producer rather than a contract field, which is why they are not here.

Validation

  • Tested revision: 9f0018b95
  • Run state: finished
  • Input classes: public_fixture; synthetic
Check kind Result Public-safe evidence / limitation
unit passed tests/capabilities/test_pr_review_contract.py, test_pr_review_result_check.py — 221 passed in the pr-review suites; 1917 passed, 42 skipped (opt-in live-model) across tests/capabilities.
regression_parity passed tests/capabilities/test_pr_review_body.py and examples/fixtures/pr-review-history/ pass unchanged, proving historical published bodies are still recognized after the policy bump.
static passed ruff check loopx/ tests/ clean.
real_entrypoint passed examples/pr-review-command-smoke.py ok; enforces the skill's 180-line contract and its locked phrases.
integration passed examples/docs-governance-smoke.py ok, examples/repository-hygiene-smoke.py ok, examples/slash-command-catalog-smoke.py ok.
manual passed Same synthetic inputs through the real loopx pr-review --check-result at base 3c50e59c0 and this head: legal no_spec/mapped results identical; legacy shape, missing reviewer, not_met+APPROVE, unpublished revision, non-text spec_ref, EX-1 only inside EX-10, and spec_revision=main pass at base and are rejected at head.
  • Coverage and gaps: the changed paths are review_contract.py (contract data), result_check.py (validation), review_body.py (publication parsing), plus docs. check_review_body is intentionally unmodified and covered by its existing suite. Untested path: none identified. The checker cannot verify that a declared identity is true, that criteria are complete, or that a commit id exists; those stay reviewer judgments. CI on this head is read in the published review.

Frontend / Visual Evidence

  • UI impact: none — no dashboard, website, desktop or documentation chrome change.
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: N/A — review-contract capability change, no roadmap card.

Shared-authority RFC fixture impact

  • Production-scale fixture schema: N/A
  • Semantic dimensions changed, or reviewed no-impact rationale: N/A
  • Provider conformance arms run: N/A
  • Read-only legacy/file/PostgreSQL three-arm rehearsal (required for promotion, runtime-routing, or compatibility-projection changes): N/A

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

Reviewer Notes

This is a control-plane contract change (loopx/**), so it is proposed for review and not self-merged, per AGENTS.md.

Two design points worth a reviewer's attention:

  1. The declaration is not evidence. An agent cannot verify its own identity, so reviewer sits beside review_body rather than inside evidence, and a gap makes the result unpublishable rather than producing a finding against the author's code. Folding it into evidence would have been less code and worse semantics.
  2. check_review_body is deliberately untouched. It is shared with the path that recognizes reviews already published on GitHub; adding a body requirement there would have retroactively invalidated every historical review. tests/capabilities/test_pr_review_body.py and the historical fixture cases pass unchanged, which is the evidence for that boundary.

The infrastructure-shape check (_DECLARED_NAME_FORBIDDEN) is a documented heuristic with a concrete known false positive: router- or version-qualified ids such as vendor/model or model@version are rejected. The typed follow-up is a runtime-reported identity, which a host would supply once one exists.

🤖 Generated with Claude Code

song added 3 commits October 1, 2026 09:42
…ification

A published review can be written by another operator's agent, working from a
different host with no shared context or memory. Two things every such review
needs were missing: who wrote it, and which specification it judged the change
against.

reviewer (result field plus one visible Reviewer: line)
  Declares actor_kind, declaration_source, and for a model_agent the model and
  provider. It is provenance, never review evidence: no reviewer can verify its
  own identity, so it lives beside review_body rather than inside evidence, and
  a missing or malformed declaration makes the result unpublishable instead of
  manufacturing a code finding against the author. Shape checks reject URL,
  path, host:port, account and vendor-qualified identifiers, with the known
  false positive documented.

problem_context.spec_basis (sub-assessment)
  Maps each material acceptance criterion from the target repository's own
  accepted RFC or contract onto an exact-head symbol or path, with its
  disposition. Implemented rows name a symbol and a validation; deferred rows
  name the successor; not_met blocks approval. spec_ref and spec_revision pin
  the text so another operator opens the same revision, and a change that edits
  the specification it cites is judged against the pre-change text. A criterion
  must also appear in the published body, because the structured result stays
  local and the body is all another operator reads.

check_review_body is deliberately untouched: it also recognizes reviews already
published on GitHub, so adding a body requirement there would retroactively
invalidate every historical review. The publication checks live in the
pre-publication path instead. REVIEW_POLICY_REVISION 12 -> 13, which is what
rejects a stale saved result.

Signed-off-by: song <song@example.com>
Fixtures gain a declared reviewer and a no_spec basis, and the shared body
fixture is prefixed rather than edited so the historical review text stays
byte-identical for the independent fixture suites.

New cases pin the rules that carry the design: a declaration hidden in a
comment or a fenced block is not published, a body line that disagrees with the
structured declaration is rejected, unknown or infrastructure-shaped values are
rejected, a human operator needs no model, an unmet criterion blocks approval,
a mapped basis cannot cite a source it did not read, and every criterion plus
spec_ref must reach the body.

Signed-off-by: song <song@example.com>
…view surfaces

- pr-review SKILL: fill result.reviewer and open the body with
  reviewer_declaration.body_marker; read problem_context.spec_basis's
  specification before the implementation and satisfy it criterion by criterion.
  Compressed to stay inside the skill's 180-line contract rather than raising
  it; every smoke-locked phrase is preserved.
- PR template: an Author Declaration section naming who wrote the change and the
  specification plus revision it was implemented against, one row per criterion.
  It is published attribution, so it stays short and carries no runtime detail.

Signed-off-by: song <song@example.com>
@mergify

mergify Bot commented Oct 1, 2026

Copy link
Copy Markdown

This pull request has merge conflicts with main and cannot be merged
until they are resolved. Please rebase or merge the base branch, @songoow.

Choose the remote for the base repository, not an out-of-date fork.
For a fork clone, first inspect git remote -v; upstream must point
to https://github.com/loopx-project/loopx.git. If it is absent, add it
with git remote add upstream https://github.com/loopx-project/loopx.git.
Then run:

git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEAD

For a same-repository clone whose origin points to
https://github.com/loopx-project/loopx.git, use origin instead of
upstream for fetch/rebase. If you prefer merging the base, use
git merge <base-remote>/main and push normally.

Keep the DCO Signed-off-by trailer on every commit when you rebase.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added needs-rebase Mergify: the pull request has merge conflicts with its base branch and removed needs-rebase Mergify: the pull request has merge conflicts with its base branch labels Oct 1, 2026

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent · GPT-5 family · OpenAI (self-reported family; not identity attestation)

动机

评审 head:d050ea0de0b3608a9a0a9a10c9b328667a20f6d0;对照当前不可变基线 850268bffc6f7123e88f578d3743f8f6da5c5c73。要交付的是公开评审能说明谁写的、另一位操作者能找到同一版规范及其条款判断。既有 loopx/capabilities/pr_review_queue/README.md 的 review-depth / published-body 约定和基线 review_contract.py 是当前接受边界:本地一致性检查不是事实验证,正文是公开协作产物。新增 reviewer/spec 字段属于提议的能力扩展,不能用修改后的规范自证成功。本次重新检查了完整 base-to-head 差异及最后一笔 skill 压缩,未继承旧 head 的结论;approval-closeout 和 180 行预算已保留,但两个公开出口反例仍存在。

改动思路

将 reviewer 放在 result 顶层而不是证据分数内,以及不改变识别历史 GitHub 正文的 check_review_body,都是合理的边界选择。声明来源不会变成身份认证或额外批准权限;旧公开评审也不应仅因缺少新字段而被追溯作废。新规范要求应作用于新结果的发布前路径,声明字段、正文和规范映射必须一起闭合。本 head 对常规 missing reviewer、缺少本地 spec_revision、not_met 却 APPROVE 等输入正确拒绝,但不能据这些正控宣称跨操作者的规格绑定已经完成。

具体改动

完整七文件差异 +479/-39:production 三文件负责新 contract data、validator 和可见正文解析;两个测试文件补充正负例;PR template 和可选 review skill 教授声明/条款 mapping。没有 dashboard、Lark、存储后端或自动发布 authority 改动。相较 7e6a009,最后一笔只压缩 skill 两行净增,未改变 production、测试、base 或依赖;完整新 head 的入口、历史 reader、关键反例和伴随测试仍重新核验。

关键代码讲解

  1. _reviewer_errors(result_check.py:226)检查 actor/source 闭集、模型和 provider 的非空文本,再要求一条可见 Reviewer 行。它拒绝注释或代码块中伪装的声明,但并不认证模型身份。vendor/model 等名称被形状规则拒绝的误报已明说;规则不能被包装成完整 credential scanner。
  2. _check_spec_basis(result_check.py:257)按 mapped/no_spec/not_yet_proven 检查源类型及 criteria,implemented/deferred/not_met 各有不同字段,not_met 是发布前批准阻塞。问题是 required-fields helper 只检查非空,随后直接用 criterion_id 做 set 操作,用 disposition 查 dict,尚未把条款 identity 规范化为可发布的非空文本。
  3. _unpublished_spec_references(result_check.py:300)将本地 mapping 对照可见正文,这是当前公开出口。它只收集 spec_ref 和字符串 criterion_id,既遗漏 spec_revision,又跳过非字符串 identity。另一个 host 没有本地 JSON,因此不能靠这个检查恢复同一版文本。
  4. build_review_plan / build_review_execution_contract 和 skill 路由新增声明、spec 映射与 policy 版本;模板把这些信息带入作者的公开叙述。历史 body reader 保持原样,避免把发布前的新门槛回灌成历史无效结论。

阻塞发现

  • [P2] 将不可变规范版本一并带入公开正文。 result_check.py:305–308:本地 spec_ref=docs/reference/example-contract.md、spec_revision=b…b、criterion_id=C1,正文只出现路径和 C1 时,真实 loopx pr-review --check-result 返回 exit 0、ok/approval_consistent 都为 true。正文读者只能找到随主干变化的路径,找不到被审查的 b…b 版本,违反本次声明要解决的“另一台机器打开同一份规范”结果。最小修复:要求公开的 spec_ref + spec_revision,或检查包含该 revision 的不可变 public permalink;同步 contract/skill 和缺失版本的负例,保留 no_spec 的明确分支。
  • [P2] 在 membership/hash/publication 前验证 criterion identity 的类型。 result_check.py:284–291:将唯一 criterion_id 改为 JSON true,正文只出现 spec 路径、不出现条款 identity,真实 CLI 仍给出 APPROVE 一致。因为 bool 被当作非空且可 hash,而 _unpublished_spec_references 仅检查字符串,这绕过“每个 criterion 必须公开”的新门槛。对象/数组 identity 和对象 disposition 则退到 command-level unhashable type,而不是带字段位置的一致性拒绝。最小修复:在这一窄边界明确要求非空字符串 identity 和已知字符串 disposition,失败返回现有结构化 blocker;加入 bool/object/array、重复条款和合规文本的配对回归。不要用广泛 exception catch 替代输入校验。

独立源 CLI 使用显式隔离 registry,在当前不可变 base 与本 head 比较相同九类常规/畸形输入。基线允许旧格式,这是原行为;head 正确新增部分拒绝,却仍接受公开版本缺失和布尔 identity。194 个 contract/result/body/approval-closeout 测试在隔离路由下全部通过;docs governance、slash catalog、Ruff、semantic vocabulary smoke、diff advisory 和 diff hygiene 通过。首次测试被本机两个默认 runtime 的路由冲突挡住,不计为 PR 回归,隔离重跑没有该失败。

预算复测:7e6a009 曾在 skill 182 行处使 command smoke 失败;当前提交压缩到 180 行,没有提高限制,且保留了 approval-closeout。相同隔离路由下,examples/pr-review-command-smoke.py 在不可变 base 和本 exact head 均完整通过,这项问题已关闭。上述 194 项测试、九类真实 CLI 输入、docs governance 与 slash catalog 也在本 head 重跑;不能把旧失败留作当前 blocker,亦不能因预算修好就忽略仍复现的两个公开出口缺口。验证没有真实 GitHub 发布副作用或 CI 查询。

对主干的风险

这是默认行为变化:新 reviewer/spec 声明成为每份新 review result 的强制发布前输入,不是“guidance”,未满足就不能发布相应 verdict。该增量在 contract、skill 和评审中须清楚披露;不应据此撤销旧 GitHub review。现有 body fixture/历史 corpus 在 head 通过,真实 CLI 普通输入也保持历史正文的解释,不引入新的 GitHub 写入、dismiss 或 merge authority。

语义与 CI 对齐

两处缺口会积累成看似通过、但外部读者无法还原审查依据的公开批准;不能通过提高正文长度或 relabel evidence 来修复。相邻的有界演进应统一条款字段规范化与公开版本锚点,复用此能力当前解析 owner,不扩成通用身份平台或全量语言迁移。当前 Python owner 没有对应的 TS review domain owner;闭集应留在该 capability-local 契约并先验证文本类型。主干当前已使用 policy revision 13 承载 approval-closeout;新 head 保留了 closeout,却仍用同一个 revision 13 新增必需字段,需按现有 policy bump 规则分配新的 revision。它们是本地语义/验证事实;CI 没有查询,不据远端红绿做结论。

我的整体评价

身份来源、发布前验证与历史读取分离的设计可以保留,生产增量也没有引入未使用框架。但“规范版本能跨操作者重现”和“每条 criterion 都公开”正是此 PR 的核心出口,两个反例说明出口尚未完成。建议在现有七文件范围内补类型和版本绑定、扩相应负例,再对新的 exact head 重跑能力评审。这里不是要求后续 coverage ledger 或 token receipt,亦没有否定所有已有测试;请求修复当前新增契约已声称保障的两条结果。无合并、无旧异议撤销。

English verdict: REQUEST_CHANGES — d050ea0de0b3608a9a0a9a10c9b328667a20f6d0. Real check-result still accepts a mapped review without its public immutable revision and a boolean criterion with no published identity. 194 focused tests and complete command smoke pass under an isolated registry; skill compaction closes the prior line-budget failure and retains approval-closeout. No CI or merge claim.

@songoow
songoow requested a review from huangruiteng October 2, 2026 05:55
@songoow

songoow commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

两个公开出口反例已修复,当前 head 5a4847c79。

[P2] 公开正文缺失不可变规范版本

_unpublished_spec_references 现在同时收集 spec_ref 与 spec_revision,并对照可见正文;只出现路径不再通过。契约文本同步说明"路径随分支移动、版本钉住被审文本"。

[P2] membership/hash/publication 前校验条款 identity

criterion_id 先在唯一进入点要求非空字符串,再做重复检测、disposition 查表与正文比对。布尔、对象、数组、整数与空串现在都返回结构化 problem_context:spec_basis:invalid_criterion_id,不再依赖后续 set/dict 操作给出 command-level unhashable type;disposition 同样要求字符串后再查表。

policy revision

REVIEW_POLICY_REVISION 13 → 14:新增的是发布前必需字段,按现有 bump 规则需要新 revision,approval-closeout 保留在新 revision 上。

回归证据

  • test_a_published_mapping_must_carry_its_immutable_revision:正文只带路径时在 errors 里得到 review_body:spec_reference_not_published:<revision>,ok 与 approval_consistent 均为 false;同映射补上版本后通过。
  • test_a_criterion_identity_must_be_published_text:True/False/["EX-1"]/{"id":"EX-1"}/1/"" 六种输入均被拒。
  • 既有正例夹具的公开正文改为携带 40 位版本,no_spec 分支保留。

tests/capabilities/ 全部通过(result_check 132、contract/body 合计 165,全量能力套件 184+);examples/pr-review-command-smoke.py 在 180 行预算内通过。本 head 已合并最新 main。未查询远端 CI,未合并。

@mergify

mergify Bot commented Oct 2, 2026

Copy link
Copy Markdown

Hi @songoow, the DCO Sign-off check did not pass. Please inspect
its details first: checkout, fetch, timeout or infrastructure errors
need their own recovery, not a rewrite of otherwise signed commits.

If the log confirms a missing Signed-off-by trailer, amend the
affected commit with git commit --amend -s; for multiple commits,
use an interactive rebase against the current base from the correct
base-repository remote and sign off each affected commit. Push the
rewritten PR branch with git push --force-with-lease origin HEAD.

@songoow

songoow commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

新 head 4b5a13ff5,已把最新 main(e38b057b3)并入并推送。

本次变动

  • 只并入 main,未改任何评审契约内容;上一 head 的两条 [P2] 修复保持原样。
  • PR base 已从 3a2a29198 前进到 e38b057b3,mergeable 恢复;此前 mergify 报的冲突已消解。
  • 验证:tests/capabilities 1900 passed / 42 skipped(本 head 全量跑);build 通过。未查询远端其余 check 结论之外的 CI。

需要一次对比本 head 的复审:reviewDecision 目前为空,上一轮的 REQUEST_CHANGES 挂在 d050ea0de 上,本 head 尚无结论。

English: head 4b5a13ff5 fast-forwards the base to current main (e38b057b3) with no contract-content change; the two [P2] fixes from the prior review round are unchanged. tests/capabilities 1900 passed / 42 skipped on this head; build green. Requesting a fresh review against this exact head.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent · GPT-5 · OpenAI(self-reported;不是身份认证或独立模型溯源)

动机

复审完整 head 4b5a13ff5902949c3d047ef9f2653982600f9ddc,比较不可变 base e38b057b3b7ca322419a214d9bfcb43ddb960e3c。依据是 AGENTS.md 的 PR Review Comments / Public And Private Boundary、loopx/capabilities/pr_review_queue/review_contract.py 的既有评审契约,以及本 PR 声明的“跨 host 的读者能打开同一份规范”结果,不把作者的完成声明当作验收。

C1:公开、文本型的规范引用与版本必须随评审到达读者;当前仍未满足。旧问题中“只公开路径、不公开文本版本”和布尔 criterion identity 已修复,不能因这两个旧反例通过就直接批准整份新契约。

改动思路

build_review_execution_contract/build_review_plan 增加 reviewer 与 spec_basis;check_review_result 在发布前验证声明、条款 disposition 和可见正文;visible_review_text/reviewer_declaration_lines 过滤隐藏注释和代码块。历史 GitHub 评审识别继续使用不变的 check_review_body,避免追溯性失效。policy 13→14 正确表明新增发布义务;这次评审本身按当前已安装 capability policy 13 执行,不冒充新 policy 已上线。

具体改动

完整差异为 7 文件、+538/-40:三个 production 文件 +230/-3;两个测试文件 +236/-1;PR 模板和 review skill +72/-36。模板教作者声明来源与规范条款;skill 压缩保留 180 行及 approval closeout;没有新增 provider、后台任务或合并权限。

[P2,阻塞] result_check.py:276–280,307–324:mapped 的 spec_ref/spec_revision 仍只经过通用非空校验,随后正文检查用 isinstance(reference, str) 静默跳过非字符串。用真实公开 CLI pr-review --check-result --packet …、全新隔离 runtime/registry,固定有效的 C1 映射,仅改一个字段:

  • 合法文本路径+40 位版本+C1:exit 0,ok=true, approval_consistent=true。
  • 合法文本版本未公开、或 criterion_id=true:现在正确拒绝。
  • spec_ref=true、非空对象,或 spec_revision=true、非空数组:仍 exit 0、两个标志都为 true;正文完全不带被替换的锚点。

这不是格式偏好:另一个 operator 无法取得被审规范或固定版本,而校验已允许 APPROVE。最小修复是在同一 spec 边界统一验证这些身份字段为非空文本,再做发布一致性比较;补 bool/object/array/whitespace 的反例及文本正例,保留历史评审读回和 no_spec 分支。无需为了此修复新增并行 Python/TS 决策 owner。

对主干的风险

真实 base/head CLI 对照证明旧缺口修复有效,同时上述四个绕过在 head 仍存在。当前 head 的 contract/result/body/approval-closeout/behavior 套件 204 passed、33 skipped;同一 base 套件 176 passed、33 skipped。33 项是需显式开启的 live-model qualification,本轮未调用模型,也不以跳过代替资格证明。command smoke、改动文件 Ruff、diff whitespace 检查通过;语义 advisory 没有支持的候选,不代表动态字段已被证明安全。最初一次测试因本机隐式双 runtime 路由冲突失败,显式隔离后通过,该环境错误不算作者回归。未读取或等待 GitHub CI。

Reviewer 声明是 self-report,不是信任/权限凭证;基础设施形状 heuristic 的 vendor/model、model@version 误拒风险已被契约说明,不能称为完整敏感信息扫描。UI 无生产改动;模板首屏不变,review skill 是可选安装教学路径。历史识别边界与 closeout 未新增撤销任何未解决评审的授权。

我的整体评价

REQUEST_CHANGES。功能归属与范围合理,旧修复应保留;但 C1 的类型缺口使本次独立结果尚未完成。future-facing pass:建议在现有 spec 边界收敛“可公开文本身份”校验,而不是继续逐字段追加不一致规则;更广 TS 迁移不作为本 PR 前置。修复后应重跑上述公开 CLI 的两端反例,再检查整份当前 head。未批准、未撤销仍有效的阻塞评审、未合并。

English verdict: REQUEST_CHANGES — head 4b5a13f; non-string mapped specification references/revisions still bypass publication consistency and permit APPROVE. Prior textual-revision and criterion-identity fixes passed; 204 focused tests and command smoke passed, 33 live-model cases intentionally not run.

songoow and others added 8 commits October 2, 2026 18:52
Signed-off-by: song <liusongstep@gmail.com>
…dex/pr-review-self-evidence

Keep main's approval_closeout step in the publication paragraph and this
branch's 'public blocker belongs on the PR' clause.

Signed-off-by: song <liusongstep@gmail.com>
Merging main into this branch added two lines to the skill, so its length
contract failed (182 > 180). Rewrap the merged publication paragraph and the
reviewer-declaration instructions without dropping any requirement from either
side.

Signed-off-by: song <liusongstep@gmail.com>
…identity

Two accepted-result gaps let a review pass locally while a reader on another
host cannot reconstruct what was judged:

- The published-body check accepted a mapped basis whose spec_ref appeared
  without its immutable spec_revision, so a path that moves with the branch
  stood in for the reviewed text.
- A criterion_id only had to be non-empty, so JSON `true` hashed, joined the
  duplicate set, and fell out of the string-only body check.

Criterion identity is now a non-empty string validated where membership,
duplicate detection and publication all assume it, and the published mapping
must carry both spec_ref and spec_revision. Required policy revision moves to
14 for the new publication requirement.

Signed-off-by: song <liusongstep@gmail.com>
…dex/pr-review-self-evidence

Signed-off-by: song <liusongstep@gmail.com>
…evidence

Signed-off-by: song <liusongstep@gmail.com>
…evidence

Signed-off-by: song <liusongstep@gmail.com>
…nd revision

spec_ref and spec_revision passed the non-empty field check as bool, object
or array values and then silently fell out of the published-body comparison,
so an approval could cite a specification no reader on another host could
open. Validate both as non-empty text at the spec-basis boundary, alongside
criterion_id, and cover bool/object/array/int/whitespace for each field.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>
@songoow
songoow force-pushed the codex/pr-review-self-evidence branch from 4b5a13f to 4bbdd36 Compare October 2, 2026 11:03
@songoow

songoow commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

针对 4b5a13ff5 评审中的 [P2] 已修复,新 head 为 4bbdd368c。

[P2] mapped 的 spec_ref / spec_revision 必须是可公开的文本

  • result_check.py::_check_spec_basis:通过 mapped 必填检查后,新增对 SPEC_BASIS_ASSESSMENT["published_text_fields"](spec_ref、spec_revision)的校验,要求非空字符串。bool、object、array、int 或纯空白会返回结构化 blocker problem_context:spec_basis:invalid_<field>,不会再被正文一致性检查静默跳过。处理方式与 criterion_id 在同一边界保持一致,没有新增并行 owner。
  • review_contract.py:规则文本由只要求 criterion_id 改为要求三者都是非空字符串,契约数据新增 published_text_fields。policy revision 仍为 14:这是对同一个未发布 revision 内新增字段的收紧,main 仍是 13。
  • _unpublished_spec_references 的行为不变,补充了注释,说明非文本值已在上游被拒绝。

回归证据

  • 新增 test_a_mapped_specification_reference_must_be_published_text,对两个字段 × True / {"path": …} / [rev] / 1 / " " 共 10 种情况断言 invalid_<field>,且 ok 与 approval_consistent 均为 false;即使正文中带着合法的路径和版本也一样会被拒绝。
  • Mutation 检查:恢复修复前的 result_check.py 后,这 10 个用例全部失败;合法文本的正例(test_mapped_spec_basis_binds_criteria_to_the_head_and_the_published_body)和此前的修复用例仍然通过。
  • tests/capabilities 全量:1910 passed / 42 skipped(42 项为需显式开启的 live-model 用例,未运行)。examples/pr-review-command-smoke.py、docs-governance-smoke.py、Ruff、git diff --check 均通过;generate_semantic_inventory.py --changed-from origin/main 未报告新增的 vocabulary carrier。

DCO 与冲突

  • Sign-off 失败的原因是两笔 merge commit(6395b89e7、4b5a13ff5)缺少 trailer。我按原 tree、原 parents 重建了这两笔及其后续提交并补上 Signed-off-by,内容没有变化:重建后的 tree 与 4b5a13ff5 逐字节一致。之后合并了最新 main(46a8c9b65),没有冲突。范围内每个 commit 都已签署。
  • 上一轮 CI 中 shard 的失败多数是 Both default LoopX runtime roots contain state。在隔离 HOME 下用新 head 重跑相关测试文件,这类失败没有复现。本地剩下的失败只有 test_dashboard_command.py 的 5 项,原因是本地 worktree 没有构建 chat bundle(bundle-manifest.json 缺失),属于环境限制,与本 PR 无关。新 head 的远端 CI 结果仍需查看。

请对 4bbdd368c 这个 exact head 重新评审。本次未合并,也未撤销任何评审。

English: head 4bbdd368c rejects non-string or blank mapped spec_ref/spec_revision with problem_context:spec_basis:invalid_<field> before the publication check; 10 new negative cases fail on the prior code and pass now. Unsigned merge commits were recreated with identical trees plus DCO trailers, and latest main merged cleanly. tests/capabilities 1910 passed / 42 skipped. CI on the new head not yet observed; no merge.

songoow and others added 3 commits October 2, 2026 19:38
…tokens

The body check matched references by substring, so a criterion EX-1 counted
as published inside EX-10, a short criterion id inside the 40-hex revision or
head, and a declared provider "AI" inside "OpenAI". A repository spec could
also be "pinned" by a branch or tag name, which moves. Match each reference
and declared reviewer value as a whole token, require a full commit id as the
spec_revision of accepted_rfc and accepted_contract_doc sources, and document
the revision 14 publication requirements in the capability README.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>
…evidence

Signed-off-by: song <liusongstep@gmail.com>
…fore implementing it

The reviewer reads the specification; it does not implement it. Keep the step unambiguous within the skill's 180-line contract.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: song <liusongstep@gmail.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent · GPT-5 · OpenAI

动机

未发现剩余阻塞问题。评审 exact head:9f0018b95fefcd3d944292afd3271c155d4cbd95,不是继承上一轮批准。完整 PR 为 8 个路径、+673/-40;GitHub 当前目标 base 和 PR merge-base 均为 3c50e59c0c3da96ac00b1715e7978440030c13a9。发布前发现 head 从 4bbdd368c1f8fdd428ffe282f112097b225e42bf 更新,旧稿从未发布;新正文、checker 增量、测试及安装读回均重新资格化。随 main 合入的 release/reading-guide 等内容不是本 PR 的八文件增量。

目标是让另一个操作者在公开评审中读到审查者来源,并打开同一版规范、逐项理解验收结论;不把模型名称当真实性证明,不让“机器可读”代替独立审阅。现有一致性 checker 是最近的归属,不需要新身份服务、另一份决策数据库或新增 capability。最强反对理由是增加必填声明会增加流程成本;这里保留 human_operator、no_spec 和历史正文识别,只有新结果的声明一致性变严格,成本与跨操作者可追溯的缺口相称。

本次依据既有 capability、AGENTS.md 的 PR Review Comments / Public And Private Boundary,及上一份明确的评审框架。该不可变 review 的 C1 要求 mapped 的 ref/revision 真正是可公开文本;其他历史要求是正文携带 immutable revision、criterion identity 不是 Boolean、skill 不超既有预算。它们均在当前 head 独立核验,不以作者的陈旧 policy12/base 声明当通过证据。

改动思路

继续扩展 pr_review_queue 当前 owner:契约声明字段和闭集、正文模块仅提取可见文本、结果 checker 执行一致性。公开 PR 模板和 managed skill 教用户如何填写;它们不执行模型鉴权、远端撤销或 merge。这个既有 Python capability 的有界扩展没有复制 TS authority/closeout 决策,整体 TS 迁移不是该修复前置。

相比上一评审 head 4b5a13ff5902949c3d047ef9f2653982600f9ddc,关键修复是两个 published_text_fields 在正文比较之前必须为非空 string。不再让 generic nonempty 接受 true/object/array 后,被 publication filter 静默跳过。当前 head 还把“可见”收敛成完整 token:EX-10 不能冒充 EX-1,commit 内的 bbb 不能冒充独立条款,OpenAI 不能冒充声明的 AI。仓库 RFC/contract 的版本必须是完整 40/64 位 commit,分支、tag 或短 SHA 不再充当固定文本版本;linked task/review thread 仍允许它们自己的文本版本。它是机器执行的 publication obligation,不是建议;新的 policy14 是明确的默认变化,不是 opt-in 改动。运行中的 policy13 packet 仍按其现有要求校验这份评审,不偷换已安装 policy。

具体改动

关键代码讲解

  • review_contract.py:145/169 的 REVIEWER_DECLARATION / SPEC_BASIS_ASSESSMENT 在同一个 contract owner 中定义 actor/source、mapped/no_spec/not_yet_proven 及 criterion disposition。模板、plan 和 checker 共享它们;没有第二份状态机或新 peer authority。
  • result_check.py:230/237 的 _published_as_token / _reviewer_errors 共用一个可见身份匹配规则,验证声明形状、单一可见 Reviewer 行和完整 token 一致性。human_operator 不需要模型/provider;注释或代码围栏中的声明不算公开归属。模型/provider 名称只能是普通产品族词汇,自报不会得到额外信任。
  • result_check.py:269 的 _check_spec_basis 先检查决策/来源,再验证两个公开引用的 string 类型与仓库文档的完整 commit;criterion 在去重与 set 使用前验证 string,not_met 不得搭配 APPROVE。ref/revision 的 10 个类型反例覆盖 Boolean、object、array、integer 和 whitespace。
  • result_check.py:330 与 review_body.py:70/74 比较真正可见正文中的 ref、immutable revision 和 criterion ID,复用完整 token matcher。历史 check_review_body 的识别责任不改成重审旧结果,未把旧 receipt 或旧讨论迁移/删除。
  • .github/PULL_REQUEST_TEMPLATE.md 增加 Author Declaration / Implemented against;capability README 明确 policy14 强制字段、固定 commit、完整 token 和旧 policy13 结果须重新生成。managed skill 仍为 180 行,要求先读引用的规范再读 diff,保留 exact-head、closeout、隐私与完整五段正文规则。

正路径:接受文本规范 → 映射 criterion → 输出同一公开 ref/revision/ID → 真实 python -m loopx.cli pr-review --check-result 允许一致的 APPROVE。负路径:删除正文 revision 或替换为非文本 ref/revision/criterion → 同一 CLI 非零并保留诊断;补回合法文本 → 再次通过。checker 始终只检查声明一致性,不证明规范已经真正满足。

本轮独立验证:

  • 同一 7 例公开 CLI harness 在当前 immutable target-base、上一评审 head 和当前 head 执行。base policy13 尚无新 gate,7 例均接受,这是披露的有意 delta;上一评审/current 的输入 SHA256 逐例相同,旧 head 4 个 malformed anchor 误批准,当前 6 个 negative 全拒绝、1 个 positive 通过。base 的输入仅随 policy13 contract 生成,不能冒充相同 policy14 bytes。
  • 新增独立 11 例真实 CLI oracle,在旧 4bbdd368c 与当前 9f0018b95 使用逐例相同 SHA256 输入。旧 checker 误接受的 6 例(较长条款、commit 内条款、provider 子串、main/tag/短 SHA)当前全部 exit1;合法完整 SHA1、SHA256、uppercase SHA1、修正 provider 和 linked-task 文本版本 5 例 exit0。不能只从新 unit test 或作者自报推导这些结果。
  • 同一 8 文件 native pytest 矩阵:当前 298 passed、33 skipped;target-base 253 passed、33 skipped。33 项是需显式开启的 live-model qualification,本轮没有开启或消耗模型。
  • examples/pr-review-command-smoke.py 通过;改动路径 Ruff、diff whitespace 通过。语义 advisory 检查 3 个 source snapshots,支持的 vocabulary carrier 为 0;它不识别动态/嵌套语义,所以闭集仍由本次人工 owner 分析负责。
  • 实际 workflow-skills --install --skills-dir <isolated-fixture> 读回 ready_for_host_load、当前 source revision 和 digest 一致;安装的 review skill 与当前源文件逐字相同。25 项 workflow install/recovery/readback 测试通过。未改变操作者真实 skill home,也不声称模型已经采用新说明。

语义与 CI 对齐

CLI command-size smoke 仍报 turn.py has 1135 lines, above budget 1114。在 target-base 与当前 head 用完全相同命令重跑,错误身份/详情一致,相关 command/smoke 路径没有 PR 变化;归为 pre-existing unrelated,预算未调高、失败未隐藏。该抽取由独立 #5380 边界处理,当前 PR 的新约束由上述实跑覆盖。未读取、轮询或等待 GitHub CI;APPROVE 不等于整树全绿或 merge readiness。

对主干的风险

主要兼容风险是新 publication policy:旧 JSON 不能继续用新 checker 认证,必须重新执行当前 plan 并生成当前结果,而不是只补字段;历史正文仍可识别。它不能自动认证审查者、读取任意规范或证明全文每条 criterion 被作者列出。尤其 product-family 的 slash/@/colon 禁止 heuristic 可能误拒 vendor/model 或 model@version,这个具体限制已被 contract 说明;不能把它宣传成完整隐私 scanner。完整 token 仍是词法一致性检查,并非 semantic identity 验证、commit 存在性或外部访问证明;独立 reviewer 必须真的打开并判断规范。

未新增默认执行、排程、quota、Goal 写入、UI/Lark 权限或远端自动 dismissal。当前 user journey 在已有 CLI/公开正文/managed skill 内完成,UI/Lark 没有 changed shared entrypoint,不需要虚构 companion 页面。纠错后同一结果重新检查即可,不需新增审批或猜测来源。回滚应同时回滚 policy/checker/模板/skill,不修改历史记录。

有界 future-facing pass:当前统一公开文本类型检查、完整 token matcher 与既有 visible-lines owner 是恰当收敛,避免 reviewer/spec 两套子串规则;更广的 typed runtime identity 可以在真实身份 owner 可观察时替换 heuristic,现阶段不应添加鉴权框架或平行 TS/Python owner。长期追溯得到改善,但这不关闭 LoopX 的其他交付/安装或整个 roadmap 验收。

我的整体评价

APPROVE。上一轮 C1 与更早 immutable revision / Boolean criterion / skill budget 问题都已核验修复,完整当前差异、声明兼容范围和真实 CLI 反例支持这个独立可回退结果。没有把测试数量、作者声明或陈旧结果当批准理由;未测 paid model / host adoption 不属于本次声明一致性边界。

发布后按 capability 检查有效阻塞评审。只在每个 finding 已证实解决且权限/owner 授权成立时做 native dismissal,保留讨论;若同一账号的最新 APPROVE 已取代旧阻塞,则不做多余撤销。这个批准不授权合并,且不替上述独立预算失败作整树放行。

English verdict: APPROVE — head 9f0018b. Same-input real CLI counterexamples reject malformed/unpublished anchors, embedded identities and moving repository revisions while retaining the legal paths. 298 tests passed, 33 explicit live-model cases skipped; actual isolated skill digest/head readback passed. The unchanged turn.py budget failure is separately disclosed; no merge.

@huangruiteng
huangruiteng merged commit 9047dcc into loopx-project:main Oct 2, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants