Review sustained progress and user experience before PR approval

[huangruiteng]

Goal And Delivered Outcome

A PR can implement its local feature correctly while preventing later work from progressing or making the user's ordinary journey harder. Reviews need to judge those product outcomes before approving implementation details. The acceptance-scope incident fixed by #4989 is one example, not the full review objective.

This PR makes sustained useful progress and user experience explicit judgments in the existing problem_context.outcome_impact. A local goal_achieved declaration or green CI cannot override a material regression or missing decisive evidence in either dimension. Reviewers reuse bounded real walkthroughs and validation references, including later invocations and the actual affected user entrypoint.

Related: #4989; overall roadmap, especially sustained coordination, S5 user journeys and S10 recovery. Base: main.

Scope And Continuation

• Primary judgment: long_horizon and user_experience each declare preserved, improved, accepted tradeoff, regression, not yet proven, or scoped inapplicability. Applicable conclusions need before/after evidence; deliberate friction or waiting needs an independent acceptance basis and bounded cost/recovery. The checker validates declarations, not their truth.
• Real counterexamples: repeated successful calls that starve ordinary work; recovery that only records a blocker; misleading or incomplete user journeys; unnecessary repeated setup/confirmation. Legitimate safety, budget and external-dependency waits remain valid when their owner and resume/cancel route are clear. No mandatory soak or universal UI test is imposed on unrelated changes.
• Gate-specific checks: covered work, unrelated work, future work, scope escape and recovery to actual execution remain within observable_semantics.scope_coverage.
• Publication quality: policy revision 10 requires the exact final Markdown in review_body. Local checking, GitHub readback and merge readiness share one parser. Five section floors are 40/80/180/120/60 explanatory characters for behavior-bearing changes and 20/30/50/30/20 otherwise. Fenced code, HTML comments, headings, URL targets, hashes and duplicate lines within a section cannot supply the prose; conflicting verdicts are rejected. The shared parser now rejects an otherwise complete review hidden in an HTML comment or in a longer code fence with a shorter/invalid closing line. Existing short open-head conclusions require a substantive replacement.
• Reuses the existing capability/evidence owners, removes duplicate body checks, updates the thin managed skill and replaces the narrower self-repair diagnosis. Queue priority and post-merge selection are unchanged. No new Goal authority, scheduler, provider or frontend setting is introduced.
• Historical evidence: the default body is the substantial fix: bind checkpoint-only recovery to a fresh decision basis #4882 post-fix review. Two request-changes reviews (feat(progress-review): raise the existing replan obligation from typed drift receipts of an optional Jev observer #4854 and fix: bind checkpoint-only recovery to a fresh decision basis #4882) and 13 exact-source code excerpts form a three-case corpus with immutable URLs, commits, file paths and line ranges. Public review bodies are preserved with documented test normalization. Model inputs exclude the historical review, expected verdict and decisive-location oracle.
• Validation hold: the current-head historical model probe again attributes the retry fault to the wrong boundary (2 passed, 1 failed), despite a passing intermediate rerun. Configured mypy still fails in unchanged baseline code. No merge-ready, fully qualified, or autonomous-investigation claim is made.

Validation

• Tested revision: e363b96bbea1f19c412c51bc8e013fa37acef7a8. The HTML-comment and code-fence fixes and regression tests were committed with identical tested source content.
• Run state: finished
• Input classes: synthetic, public_fixture

Check kind	Result	Public-safe evidence / limitation
unit	passed	Focused body, result checker, contract, decision corpus and GitHub scan suite: 196 passed, 27 opt-in live probes skipped in that ordinary run. Added hidden-comment and invalid-fence negatives for local body checking, structured result checking, and GitHub conclusion readback. Tests reject a local-success approval with negative continuity/experience evidence and accept a justified bounded wait.
real_entrypoint	passed	uv run --extra test python examples/pr-review-command-smoke.py; actual source CLI packet and merge-readiness routes with the historical checkpoint review body, including rejection of a heading-only approval.
integration	passed	Earlier synthetic real-provider corpus: 24 verdict probes plus two corpus checks passed on the unchanged production contract. Both rejection and approval oracles are retained. New scope, starvation, repeated intervention and valid confirmation rationales were inspected. These are supplied-evidence probes, not autonomous repository investigation or a measured uplift claim.
integration	failed	Prior historical real-provider run: 2 passed, 1 failed. Both checkpoint decisions and locations passed. Retry-claim rejection was correct, but the explanation blamed the consumer instead of producer deduplication; the independent decisive-location assertion rejects it. The oracle was not relaxed.
integration	passed	Intermediate head a73b6164 process-only historical probe rerun: 3 passed, 26 deselected. It does not override the current-head failure.
integration	failed	Current head e363b96b historical probe: 2 passed, 1 failed, 26 deselected. The retry verdict is correct, but its cited ranges miss the producer's Turn deduplication at external_progress_review.py:226-230. The independent location oracle remains unchanged.
manual	passed	Three bodies compared with public GitHub API responses; 13 code excerpts compared with exact historical Git objects. Historical test reports are not represented as rerun production validation.
static	passed	Ruff on changed Python files, isolated strict mypy on the current body parser, git diff --check, and source CLI public-boundary scanning of four changed docs/fixture surfaces.
static	failed	Final configured uv run --extra test python -m mypy: one no-any-return at local_authority_shadow_projection.py:100. Its base/head file content is identical. The strict quality receipt retains this blocker.
manual	passed	Earlier real GitHub/source-CLI readback recognized the substantial public #4989 review under the new body rules; its failure was not attributed to insufficient length.

For the earlier synthetic corpus, the initial live run produced correct verdicts but mismatched broad concern labels in two cases. Corpus families now organize cases rather than acting as a product requirement; fixed positive/negative verdict oracles remain, and compact explanations are saved for inspection. A prior exploratory strict import-closure run outside the repository's configured mypy suite also failed (4099 diagnostics before the parser annotation fix); it is not a clean qualification oracle or a passing result. The current required mypy failure is reported above.

The first historical probe run matched all three verdicts but a manual rationale audit found false causal attribution. The final corpus includes the actual producer, novelty codec and consumer and checks a concrete source location. One intermediate rerun passed 3/3, while the current-head rerun again missed the producer deduplication in the retry case. This is an exposed review limitation, not a successful qualification claim; the verdict and location oracle were not relaxed.

Public scanning passed using an isolated temporary Effect runtime after the shared locator refused connection; two unrelated existing registry warnings remain outside this diff. No active Goal was rewritten for validation.

Frontend / Visual Evidence

UI impact: none. The changed user entrypoints are the PR-review CLI packet/checker and managed review workflow. This PR reviews affected frontend/messaging journeys when applicable; it does not change their runtime behavior or configuration.

Type of Change

• Bug fix
• Breaking change (review policy revision and required evidence/body)
• Documentation update
• Test update

LoopX Area

• Capability or extension (providers, adapters, skills)

Technical Direction

Core control-plane hardening: product outcome, continuity and user-journey review. No TypeScript/provider migration claim.

Shared-authority RFC fixture impact

N/A: no promotion, routing or persisted authority contract changes.

Boundary Checklist

• No private state, credentials, raw traces, internal links or local machine paths are included.
• No maintainer-owned benchmark work is duplicated.
• The change is scoped to review evidence and publication quality.
• UI impact is none.
• Every commit includes a DCO sign-off.

[huangruiteng]

Request changes conclusion (author-owned PR; GitHub blocks formal self-review)

Reviewed exact head: 95c6fe530d9734913eebbc1026fa01e3ca3b654d (base da4acd4cfb8466268e6ec4a929e9f7929de0e433). This is a review of the code and its observable behavior, not an acceptance of the PR description.

动机

The intended outcome is sound: a reviewer should judge whether a change sustains useful work and keeps the user's journey usable, rather than approving a locally correct implementation that strands later work. Previously the review contract lacked those two explicit outcome dimensions, and the published-body check only looked for headings, a head OID, and a verdict. The PR makes a useful, independently reviewable policy increment, but its promised stronger publication/readback gate is not yet established at this head. A hidden review body can still satisfy that gate, so the user may see an apparently reviewed PR without any visible reasoning.

改动思路

The pr-review packet remains the reviewer entry point. review_contract.py adds long-horizon and user-experience judgments to problem_context, a typed scope-coverage assessment to observable_semantics, and explicit prose floors. result_check.py checks a submitted structured result; pr_review.py uses the same review_body.check_review_body on remote review readback. That shared owner is preferable to two divergent parsers. The positive path is a visible five-section review bound to the exact head, followed by publication and remote readback. The negative path must reject a body whose headings and verdict exist only in non-rendered Markdown; at present it does not. These checks are on the normal review/merge-readiness path, not an opt-in provider.

具体改动

The exact diff adds the shared review_body.py, extends the result checker and remote conclusion reader, revises the review-plan instructions and public skill, and adds historical cases, unit tests, and a CLI smoke. The fixture set has source references for three prior review situations; it is not itself a live model outcome. The public README discloses the policy revision and that older short reviews may no longer qualify. This is a default review-contract change, not a new actor lifecycle or permission grant.

关键代码讲解

• review_contract.build_review_execution_contract defines the new typed dimensions and required evidence. It asks the reviewer to compare continued progress and user experience against the accepted goal, including bounded counterfactuals for a gate's authorized subjects.
• result_check.check_review_result validates the structured result and delegates the rendered review-body shape to check_review_body; a REQUEST_CHANGES result can carry blockers without falsely approving.
• review_body._visible_lines skips fenced code, but yields HTML-comment contents as though rendered. check_review_body then counts their headings, prose, exact head, and English verdict.
• pr_review._review_conclusion applies that same check to the remote GitHub review. Therefore the parser error reaches the published-review/merge-readiness decision, not just a local helper.

对主干的风险

P1 — A wholly invisible review passes the new publication and merge-readiness format gate. The trigger is an otherwise full five-section body wrapped in <!-- ... -->. At this head, check_review_body returns valid: true and verdict: APPROVE; the production _review_conclusion path likewise returns status: valid for a synthetic exact-head APPROVED review containing only that hidden body. GitHub renders none of its reasoning. This contradicts the PR's explicit goal of rejecting empty review shells and can make an exact-head approval appear substantive when it is not. Please exclude HTML comments and other non-rendered text from the visible-content check, then add a negative regression through both check_review_body and _review_conclusion/merge-readiness. Keep a normal visible five-section review passing. Re-run the focused PR-review tests and command smoke after the fix.

语义与 CI 对齐

The new policy extends the existing PR-review contract; it does not create a second authority. The typed assessment fields and generic wording are appropriate, and the docs disclose the tighter default. Local exact-head validation: 156 focused tests passed and 27 opt-in live-model probes skipped; the PR-review command smoke, changed-file Ruff, and git diff --check passed. The configured mypy run fails at local_authority_shadow_projection.py:100, but the exact base fails identically, so I do not attribute that error to this PR. The PR description also reports a historical live-model probe failure; I have not independently run that credentialed probe, and it remains an author-disclosed validation hold. Remote CI was not consulted under the review packet's wait_for_ci=false policy.

我的整体评价

REQUEST_CHANGES on this exact head. Long-horizon value is not yet proven by the current validation set: the historical model probe is reported failing, and a false review-readback can certify an unreviewed path. The user's visible review experience does not yet meet this PR's intended stronger gate; the same hidden-comment hole existed in the weaker baseline, so this is an unmet new guarantee rather than a newly introduced baseline regression. The shared parser is the right bounded owner; a related visibility fix and negative test belong in this PR, not another framework or broad migration. The 17-file diff is mostly fixture/test material and is proportionate to a review-policy change only if the publication/readback guarantee actually holds. Please repair the hidden-content counterexample, rerun the named local checks, resolve or explicitly hold the live-model probe, and request a fresh review after the head changes. No self-merge of this control-plane change is appropriate.

English verdict: REQUEST_CHANGES

[huangruiteng]

Request changes conclusion (author-owned PR; GitHub blocks formal self-review)

Reviewed exact head: e363b96bbea1f19c412c51bc8e013fa37acef7a8 (base da4acd4cfb8466268e6ec4a929e9f7929de0e433). 本次从原始目标复核整个 base-to-head diff，并单独核查了上次 review 后的两笔解析器修复；未把 PR 描述或先前结论当作通过证据。

动机

原有 PR review 可在局部功能和 CI 都看似成功时忽略长期工作是否继续、用户是否还能顺畅抵达目标；旧的发布格式检查也只识别标题、head 和 verdict，无法保证这些文字真的可见。#4998 把持续推进与用户旅程放回已有 problem_context 判断，并让本地结果校验和 GitHub 读回共用正文解析器。这个交付方向有价值：一次 review 应阻止“写出一个成功回执却饿死后续工作”或“新设置让普通恢复每次都要额外干预”。但当前历史模型探针仍在关键因果位置上不稳定，不能把这项能力说成已经完成资格验证。

改动思路

入口仍是 loopx pr-review 生成的 exact-head packet；review_contract 规定 long_horizon、user_experience 以及覆盖范围的类型化证据，reviewer 负责真实判断。result_check 只核验声明之间不矛盾，不能证明证据为真。发布后，pr_review._review_conclusion 用相同的 review_body.check_review_body 读取远端正文，随后 merge-readiness 才能消费这个结论。正向路径应是可见的五段论证、与本 head 绑定、验证后发布并读回；负向路径必须挡住标题和 verdict 全藏在注释或代码围栏里的正文。后两笔修复留在这个共享解析器，没有另立第二个状态或权限所有者；无新的 opt-in 开关，普通 review 默认受到更严格式约束。

具体改动

全单涉及契约、结果检查、远端结论、公开说明与技能、历史评审语料、单测和 CLI smoke。契约修订为 policy revision 10，新增持续推进和用户体验两维度、对显式覆盖主体及独立工作的反例要求；文档声明较短的既有 open-head review 需重做。历史语料保留三份公开评审和对应不可变源码片段，作为格式及可选模型探针，不冒充当下代码通过历史测试。上次 head 95c6fe5 的 P1 是 HTML 注释中的整篇 review 会被误认有效；本次 a73b6164 排除了注释，e363b96b 又修复四反引号围栏被三反引号或带尾随文本的行提前关闭。覆盖本地正文、结构化结果和远端结论的负例均已加入，正常可见正文仍通过。

关键代码讲解

• review_contract.build_review_execution_contract 定义判断维度、验证责任和阻塞条件；这改变默认 review 义务，但没有给 reviewer 或 Goal 新的写入权限。
• result_check._check_outcome_impact 与 _check_scope_coverage 检查类型化声明及反例覆盖，check_review_result 将正文格式问题列成错误；声明被检查不代表真实场景已验证。
• review_body._visible_lines 现在跨行剔除 HTML 注释，按围栏字符、开启长度和合法关闭行过滤隐藏内容；check_review_body 只对剩余可见行计算五节、head 与英文结论。
• pr_review._review_conclusion 对 GitHub review 读回复用该解析器，避免本地结果“通过”而远端隐藏正文也“通过”的双重标准。

对主干的风险

之前的两个可见性绕过在当前 head 已被反例测试封住。当前主要 hold 是能力资格而非新发现的确定性解析器缺陷：历史 retry-claim 样本中，真实模型能给出 REQUEST_CHANGES，但在当前 head 的进程内探针仍把关键位置引用到后续窗口/消费者附近，漏掉 external_progress_review.py:226-230 先按 Turn 去重、丢失较早 typed observation 的分支；本次为 2 通过、1 失败。中间一次 3/3 通过不足以抹掉前后复现的错误归因。最小修复是提高对“数据在生产端哪个分支被过滤”的因果追踪，保留独立位置断言，再用相同历史输入和正向样本验证；不能扩大允许的源码范围来求绿。配置的 mypy 仍在未改动的 local_authority_shadow_projection.py:100 失败，base/head 相同，故不把它定性为本 PR 引入，但必需检查未全绿仍是合并前需要处理的事实。格式下限只是防空壳，不会自动验证论证质量；相关风险应在发布判断中持续可见。

语义与 CI 对齐

本 PR 复用既有 pull-request-review 能力和 problem_context 证据词汇，没有新 provider、持久 receipt、scheduler 或 actor 权限。影响的入口是 CLI packet/checker、托管 review 指令和 GitHub 读回；没有 frontend/Lark 行为改动。当前 head 的五个相关测试模块为 196 通过、27 个未启用的可选 live 探针跳过；可选历史 live 子集另跑为 2 通过、1 失败。PR 命令 smoke、改动文件 Ruff、解析器隔离 mypy 与 diff check 通过。配置 mypy 的既有 1 个错误如上；远端 CI 按 packet 的 wait_for_ci=false 未读取或等待。较新 main 的相邻变化未修改本 PR 的解析器/结果检查入口，PR 当前仍可合并但落后于 main；这不是放宽验证的理由。

我的整体评价

REQUEST_CHANGES on this exact head. 修复了上次 P1，且没有发现新的确定性源码阻塞；共享解析器及聚焦回归是合适的小范围自修复。持续推进判断作为规则已接入，普通用户可见 review 的注释/围栏绕过已消除，但历史真实模型的因果定位反复失败，使“更扎实的 review 能稳定识别真正所有者”仍未证实；长期效果和用户信任不能仅凭格式、单次绿灯或正确 verdict 判定为达标。17 文件、约 1.5k 行的整单主要是公开语料及测试，当前阶段可理解，但后续应优先修资格缺口而非追加框架。仓库规则还要求 loopx/** 控制面行为变更由 maintainer 合并，不能因作者自审、admin bypass 或这两次解析器修复而自合并。本机默认版也不应在未合并时升级到这条 PR；可继续用本 worktree 作隔离验证，待 maintainer 完成合并后再从 main 升级并读回版本。

English verdict: REQUEST_CHANGES - exact head e363b96; hidden-comment and fence bypasses fixed, but the current-head historical causal-location probe still fails (2 passed, 1 failed), and configured mypy retains a known baseline error.