/review: incremental-review anchor is lost permanently after a round fails to close cleanly, forcing full-diff re-reads

[wenshao]

What happened?

/review does incremental re-review across rounds using an anchor recorded in the previous round's posted marker, so a later round only re-reads what changed. Observed over a multi-round review: once a round failed to close cleanly, the anchor was withheld — and it never came back. The reviewer stated this itself:

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again.

So the degradation is sticky: a single non-clean round drops the incremental state, and every subsequent round then re-reads the entire diff from scratch. On a large PR each round's review-pr job already runs multiple hours; losing incrementality makes every later round pay full-diff cost, with no path back on its own.

A related visible symptom: the reviewer kept referencing a stale test path (packages/cli/src/utils/findings.test.ts — no such file or directory) after that file had moved to packages/cli/src/commands/review/, consistent with the incremental cache pointing at a pre-move snapshot.

What did you expect to happen?

• A round that does not close cleanly should still be able to re-establish the incremental anchor on the next successful round, rather than permanently falling back to full-diff re-reads.
• Failing that, the reviewer should treat "anchor lost" as a recoverable state (recompute the anchor from the current posted comments / head SHA) rather than a one-way latch.

Anything else we need to know?

• This compounds the cost of long review loops: a PR that takes several rounds to converge pays full-diff review time on every round after the first non-clean one.
• It is self-reported by the tool in the round body ("Mechanism health"), so the condition is already detected — the gap is the lack of recovery.

中文

问题

/review 跨轮做增量复审，靠上一轮已发布 marker 里记录的锚点，让后续轮只重读变更部分。在一次多轮评审里观察到：某轮未能干净收尾后，锚点被扣留——而且再也没回来。reviewer 自己也说明了：

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again.

即这个退化是粘性的：一次不干净收尾就丢掉增量状态，之后每一轮都从头全量重读整个 diff。在大 PR 上每轮 review-pr job 本就要跑好几小时，失去增量后每个后续轮都付全量成本，且自身无法恢复。

一个相关的可见症状：文件从 packages/cli/src/utils/findings.test.ts 移到 packages/cli/src/commands/review/ 之后，reviewer 仍反复引用旧路径（no such file or directory），与增量缓存指向移动前的快照一致。

期望

• 未干净收尾的一轮，应仍能在下一次成功轮重建增量锚点，而不是永久退回全量重读。
• 退一步，reviewer 应把"锚点丢失"当作可恢复状态（从当前已发布评论 / head SHA 重算锚点），而非单向锁死。

补充

• 这会放大长评审循环的成本：需要多轮才收敛的 PR，第一次不干净收尾之后的每一轮都付全量评审时间。
• tool 已在轮次正文（"Mechanism health"）里自述该状态，即条件已被检测到——缺的是恢复机制。

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Triage

• Type: bug
• Labels: type/bug, category/development, priority/P2
• Next: maintainer decision on the recovery direction (see analysis)

Analysis

• Reproduced: not executed — unattended CI, static analysis of the anchor withhold/recovery path only. A live repro needs a multi-round /review run containing a non-clean round.
• Confidence: mechanism confirmed in code; which withholding leg kept the observed run fail-closed is plausible, not confirmed (no run artifacts linked in the issue).

Root cause — the withhold is by design; the gap is the missing bounded recovery:

• Withholding is the fail-closed rule anchorFailsClosed() in packages/cli/src/commands/review/compose-review.ts: the anchor is withheld when the round cannot prove the whole scope was read (scopeUnproven), when a dimension gap is not depth-only, or when any verdict cap other than unreviewed-dimension fires. The anchor certifies "reviewed clean up to sha"; the code states that writing one past unreviewed scope would let no later round ever re-cover that gap — so this leg is a correctness invariant, not an oversight.
• Recovery reads only the last posted round's marker (pr-context → qwen-review-pr-<n>-prev-ledger.json). SKILL.md Step 1 is explicit: a side file with no sha (fail-closed round, or a ledger truncated by the marker's size caps) means "there is no anchor to recover, and the review is full-range". There is no fallback to an earlier anchored round.
• So the loss is not literally one-way — any later round that closes cleanly posts a marker carrying the anchor and re-establishes incrementality (the disclosure itself says "until a round's marker carries an anchor again"). But it is effectively sticky: if successive rounds fail to close, no anchor ever rides again and every round pays full-diff cost, exactly as observed.
• Chronic withholding legs that survive the August fixes: model-identity drift on the write side (runtime model ≠ the round-start stamp, or a blank runtime on a stamped round), a missing/unreadable fetchedSha, scopeUnproven reading-doubt, and the recovery-side same-model gate (cross-model rounds, or review.attribution: false environments — documented in docs/users/features/code-review.md). The mechanism-health check under-reports these: anchorChainBroken only sees the cap-related legs; the plan/model legs are decided where the marker is built (stated limit in its own docstring).
• Timeline: the observation postdates all prior anchor fixes — feat(review): carry the incremental anchor sha in the posted ledger marker #9067 (anchor rides the marker), feat(review): validate and scope the incremental anchor inside fetch-pr #9100 (validate/scope in fetch-pr, v0.21.13), fix(review): repair seven pipeline defects found by live runs #9175 (depth-only exception killing the build-and-test closed loop, v0.21.14), fix(review): gate the recovered incremental anchor on the model that certified it #9184 (same-model gate, v0.21.14). The issue quotes the current disclosure wording and was filed after v0.22.0, so the chain break is still reachable.
• The stale-path symptom is a related but separate gap: carried ledger entries keep their original file paths and nothing re-resolves them against the current tree before re-posting (packages/cli/src/utils/findings.test.ts no longer exists — the review code moved to packages/cli/src/commands/review/).

Fix direction (needs maintainer decision):

• The naive recompute proposed in the issue (anchor from head SHA / posted comments) is ruled out by design — SKILL.md rejects commitId as an anchor because "scoping the diff to it would skip exactly the range a fail-closed round could not certify".
• Candidate directions that preserve the certification invariant: (a) fall back to the most recent marker that does carry an anchor instead of full range — bounded cost (re-reads only the failed round's range), but needs work-list/anchor pairing analysis, since the sha travels with its own round's work list; (b) let transient fail-close causes (one-off model switch, transient context-fetch failure) self-heal instead of latching the chain; (c) re-resolve carried findings' paths against the current tree before re-posting.

中文说明

• 类型: bug
• 标签: type/bug, category/development, priority/P2
• 下一步: 由维护者决定恢复机制的方向（见下分析）

分析

• 复现: 未执行——无人值守 CI，仅对锚点扣留/恢复路径做静态分析。实际复现需要一次包含"未干净收尾"轮次的多轮 /review 运行。
• 置信度: 机制在代码中已确认； observed run 具体卡在哪条扣留分支上属于合理推测、未确认（issue 未附运行产物）。

根因——扣留是有意设计，缺口在于缺少有界的恢复：

• 扣留是 packages/cli/src/commands/review/compose-review.ts 中 anchorFailsClosed() 的 fail-closed 规则：当本轮无法证明读完了全部范围（scopeUnproven）、维度缺口不是纯深度型、或出现 unreviewed-dimension 之外的任何裁决上限时，锚点被扣留。锚点的含义是"干净地审到了 sha"，代码注释明确：在未审范围之上写入锚点会导致后续任何轮都不再补审该缺口——这是正确性不变量，不是疏忽。
• 恢复只读取最近一轮已发布的标记（pr-context → qwen-review-pr-<n>-prev-ledger.json）。SKILL.md Step 1 明确：侧文件没有 sha（本轮 fail-closed，或账本被标记尺寸上限截断）即"无锚点可恢复，评审为全量范围"。没有回退到更早带锚点轮次的路径。
• 因此这并非字面意义上的单向锁死——之后任何一轮干净收尾，其标记就会带上锚点、恢复增量（披露文案自己也写了"直到某一轮的标记重新带上锚点"）。但它是事实上的粘性：若连续多轮都无法干净收尾，锚点永远不会回来，每轮都付全量成本，与观察一致。
• 在 8 月系列修复之后仍然存在的慢性扣留分支：写侧的模型身份漂移（运行时模型 ≠ 轮次开始时的印记，或带印记轮次的运行时模型为空）、缺失/不可读的 fetchedSha、scopeUnproven 阅读存疑，以及恢复侧的同模型门（跨模型轮次，或 review.attribution: false 环境——见 docs/users/features/code-review.md）。mechanism-health 检查对这些欠报：anchorChainBroken 只能看到裁决上限相关分支；plan/模型分支在标记构建处裁决（其 docstring 自述了该局限）。
• 时间线：该观察晚于所有既有锚点修复——feat(review): carry the incremental anchor sha in the posted ledger marker #9067（锚点随标记发布）、feat(review): validate and scope the incremental anchor inside fetch-pr #9100（fetch-pr 内校验/定界，v0.21.13）、fix(review): repair seven pipeline defects found by live runs #9175（深度型例外，终结 build-and-test 闭环，v0.21.14）、fix(review): gate the recovered incremental anchor on the model that certified it #9184（同模型门，v0.21.14）。issue 引用的是当前披露文案，且提交于 v0.22.0 之后，说明链条断裂现在仍可复现。
• 旧路径症状是相关但独立的缺口：被携带的账本条目保留原始文件路径，重发前不会对照当前树重新解析（packages/cli/src/utils/findings.test.ts 已不存在——review 代码已移至 packages/cli/src/commands/review/）。

修复方向（需维护者决策）：

• issue 提出的朴素重算（从 head SHA / 已发布评论重算锚点）被设计明确排除——SKILL.md 拒绝把 commitId 当锚点，因为"以其定界会恰好跳过 fail-closed 轮无法认证的范围"。
• 保持认证不变量的候选方向：(a) 回退到最近一个带锚点的标记而非全量——成本有界（只重读失败轮的范围），但需分析工作清单/锚点的配对关系，因为 sha 与其所在轮的工作清单一同生效；(b) 让瞬时性 fail-close 原因（一次性模型切换、瞬时上下文拉取失败）自愈，而不是锁死整条链；(c) 重发前把携带发现的路径对照当前树重新解析。

--- Qwen Code