fix(review): ban unexecuted mutation claims in coverage briefs - #9923
Conversation
Reader agents (Agent 5 and the test-matrix) described their reading-based mutation analysis in execution-grade language, so test-coverage findings could claim "mutant verified N/N green" while their own gap notice admitted the suite could not run in the review environment. Add witness discipline to both briefs: an unrung mutation must be phrased as a reasoned hypothesis, never an executed result, and a claim whose weight depends on an unperformed run carries `witness: not run — <why>`.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. #9901 quotes actual dogfood-review output where test-coverage findings assert "the mutation … ships 35/35 green" / "mutant verified 27/27 green" while the same review's gap notice discloses the worktree has no Direction: aligned. This fixes the review skill's own witness discipline — "verified" is exactly the word a maintainer reads as observed, and coverage findings (Suggestions by design, not gated by the mechanical witness rule that only covers confirmed Criticals) are precisely where that rule can be quietly hollowed out. No direct CHANGELOG reference, but the area is internal review tooling and clearly in scope. Size: not applicable — no core paths touched. 6 production-logic lines (prompt text) + 15 test lines. Approach: scope feels right. The issue offered two options: (1) actually run mutations in the review environment, (2) enforce honest witness wording. This PR takes option 2 and leaves option 1 as an explicit follow-up — the right minimal cut, since option 1 needs install/cache/harness infrastructure. Anchoring the rule on ownership (executed mutation verdicts belong to Agent 7's efficacy probe, which really runs commands) rather than on "you have no runner" is the right call — the review-agent tool table is role-neutral and includes the shell, so a capability claim would be false. The one-word "tried" → "considered" tweak sits inside the same paragraph and reads as consistent with the intent, not drive-by churn. Risk: no elevated risk signals (no high-risk paths matched). Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到,非理论性问题。#9901 引用了真实 dogfood review 的输出——测试覆盖类 finding 断言 "the mutation … ships 35/35 green"、"mutant verified 27/27 green",而同一份 review 的缺口披露却写明 worktree 没有 方向:对齐。这是修复 review skill 自身的 witness 纪律——"verified" 恰恰是维护者读作已观测的那个词,而覆盖类 finding(按设计是 Suggestion,不受只管已确认 Critical 的机械 witness 闸门约束)正是该规则可能被悄悄掏空的位置。CHANGELOG 无直接引用,但该领域属于内部 review 工具,显然在范围内。 规模:不适用——未触及核心路径。6 行生产逻辑(prompt 文本)+ 15 行测试。 方案:范围合理。issue 给了两个选项:(1) 在审查环境真跑变异,(2) 强制诚实的 witness 措辞。本 PR 选择方案 2,并把方案 1 明确留作后续跟进——这是正确的最小切分,因为方案 1 需要安装/缓存/独立 harness 等基础设施。规则锚定在分工上(已执行的变异判定属于 Agent 7 的 efficacy probe,它真的执行命令),而不是 "you have no runner",这是对的——阅读型 agent 的工具表是角色中立的且包含 shell,能力断言会是假的。同段里 "tried" → "considered" 的一字微调与意图一致,不算顺手夹带。 风险:无升级风险信号(未命中高风险路径)。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThe approach is the one I'd have picked: the discipline lands exactly where the mutation-analysis instructions live — appended to Agent 5's equivalent-mutant paragraph and extended in lockstep to the test-matrix bullet, which already mirrors Agent 5's severity rules. Cross-checked the factual claims against the base source: Agent 7's efficacy probe really does report mutant outcomes as "the outcome of running commands", and the Two details worth noting, both good:
No correctness or convention issues found. Template-literal escaping of the backticked witness line is correct. Test evidence — the PR's own CI, fetched via APIFetched once at review time — the unit suite had not finished yet, so this is a snapshot, not a verdict. No check had failed at fetch time; the checks that matter: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The new assertions live in Not verified: live 中文说明代码审查方案与我独立设想的完全一致:纪律恰好落在变异分析指令所在的位置——追加到 Agent 5 的 equivalent-mutant 段落,并同步扩展到 test-matrix 的条目(它本来就镜像 Agent 5 的严重度规则)。对照基础源码核实了事实性表述:Agent 7 的 efficacy probe 确实以"执行命令的结果"上报变异结论;本 PR 扩展的 两个值得一提的细节,都是优点:
未发现正确性或规范问题。模板字符串中反引号包裹的 witness 行的转义正确。 测试证据——通过 API 获取的 PR 自身 CI审查时一次性获取——单元测试套件尚未跑完,所以这是快照,不是结论。获取时无任何 check 失败;关键 check 见上表。 新增断言位于 未验证:真实 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage: an observed problem with quoted evidence in #9901, a minimal targeted fix, and tests that pin the exact wording. Going back to my independent proposal before reading the diff: the only sensible fix for "reader briefs let unexecuted mutation claims be written in execution-grade language" is to add witness discipline to exactly those briefs — Agent 5's mutation paragraph and the test-matrix bullet that mirrors it — and pin it with regression assertions. That is precisely what this PR does, and nothing more. I can't name a simpler path, and the two factual anchors (Agent 7's probe really runs commands; the extended witness line is the verifier's existing convention) check out against the source. The parts that will age well: the test comments document not just what is pinned but why — including the subtle bit that the rule anchors on ownership rather than capability, because the reader tool table includes the shell and "you have no runner" would be a lie. Six months from now, if someone tries to reword this, the tests will tell them why not. The one thing I can't claim yet: the unit suite is still running on this commit. So approval is deferred until CI lands green on 中文说明置信度:5/5 —— 每个阶段都干净:#9901 中有引用证据的已观测问题、最小且针对性的修复、钉住确切措辞的测试。 回到看 diff 之前的独立设想:对"阅读型 brief 允许用执行级措辞表述未运行的变异"这个问题,唯一合理的修法就是把 witness 纪律加到恰好那些 brief 上——Agent 5 的变异段落和镜像它的 test-matrix 条目——并用回归断言钉住。这个 PR 做的正是这些,没有更多。我说不出更简单的路径,而且两个事实锚点(Agent 7 的 probe 真的执行命令;扩展的 witness 行就是验证者既有的约定)都已对照源码核实。 这个 PR 经得起时间考验的地方:测试注释记录的不只是钉住了什么,还有为什么——包括那个微妙之处:规则锚定在分工而非能力上,因为阅读型工具表包含 shell,"you have no runner" 会是假的。六个月后如果有人想改写这段措辞,测试会告诉他们原因。 唯一此刻还不能断言的:该 commit 的单元测试套件仍在运行。因此批准延迟到 CI 在该提交上变绿后执行——若全部通过,finalize 工作流会自动完成批准;若有失败,会在状态评论中标记。 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Name both banned execution-grade phrasings in the test-matrix bullet (Agent 5 already banned both), switch the nonstandard 'unrung' to the codebase's established 'unrun' vocabulary, and pin the banned-phrase list plus the false-capability guard for both briefs in the lockstep test so a one-sided edit to either copy fails the suite.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Address-review summary — PR #9923, round 2Commit: All four feedback points are Suggestions from the automated reviewer; each claim was verified against the exact code at Findings and dispositions
No finding was declined, deferred, or escalated; nothing required a maintainer decision. Mutation probes (witness verification)Every guard this round commits was probed: the mutation was applied, the focused test was required to FAIL, then the mutation was reverted and the suite re-run to green. All probes used
After the final restore, the full focused file was re-run green (299/299). The round's new assertions also fail against the pre-round branch by construction (the pre-round test-matrix bullet contained no Verification
中文说明处理审查总结 — PR #9923,第 2 轮提交: 四条反馈均为自动审查器提出的 Suggestion;每条声明都在编辑前对照 发现与处置
没有发现被拒绝、推迟或升级;没有任何事项需要维护者决策。 变异探针(见证验证)本轮提交的每一个守卫都经过了探针验证:应用变异,要求聚焦测试失败,然后还原变异并将套件重跑至绿色。所有探针均使用
最终还原后,完整聚焦文件重跑为绿色(299/299)。本轮新增断言对 pre-round 分支也会天然失败(pre-round 的 test-matrix 条目不含 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 3b": none — no check was cut short..
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/agent-prompt.test.ts:2985 — [review] the four shared witness-discipline pins are duplicated…packages/cli/src/commands/review/agent-prompt.test.ts:2971 — [probe] banned-phrasing pins are presence-only; an inverted…
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 3b":none — no check was cut short.。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
The lockstep test pinned the witness tag and both banned phrasings for the test-matrix brief, but not its hypothesis-phrasing clause, while Agent 5's block pins its half — deleting only that clause from the test-matrix bullet shipped green. Pin the clause so both halves of the discipline stay graded identically.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 6c": running cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts — the review worktree has no node_modules and no built dist/ (verified)…; "agent 5": run the vitest file packages/cli/src/commands/review/agent-prompt.test.ts — not performed because the shared review worktree has no node_modules or workspac….
中文说明
未发现问题。LGTM!✅
未探索到全部深度(达到工具调用预算):"agent 6c":running cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts — the review worktree has no node_modules and no built dist/ (verified)…;"agent 5":run the vitest file packages/cli/src/commands/review/agent-prompt.test.ts — not performed because the shared review worktree has no node_modules or workspac…。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round report — no new changes needed (re-verification round)The only actionable finding in this round's feedback — R2-1, posted in both round 2 ( Finding triage
Re-verification evidence at HEAD (
|
| 反馈项 | 处置 |
|---|---|
rc:3844345233(第 2 轮,R2-1)与 rc:3845435629(第 3 轮,R2-1 重发):test-matrix 未钉住假设条款 |
已在代码中解决,位于 HEAD(920ed22394),本轮已通过定向测试运行与变异 probe 复核(证据见下)。已列入 resolved-comments.txt,以便相应线程可被标记为已解决。 |
| 第 3 轮审查正文:收敛姿态下延后的两项("四个共享的 witness-discipline 钉桩存在重复"、"被禁措辞钉桩只做存在性检查") | 本轮不处理——审查者已明确将其记录为"本轮不要求修改"。 |
| 第 3 轮审查正文:"未探索到全部深度(达到工具调用预算)" | 仅作信息记录;审查者对 HEAD 的下一轮(第 4 轮)检查未发现任何问题。 |
| 维护者的回复评论("Fixed in 2367557…"、"Fixed in 920ed22…")及其对应的空审查外壳 | 旧线程上的状态性回复,无需处理。 |
在 HEAD(920ed22394)上的复核证据
- 修复已就位。 test-matrix 的 lockstep 块现在包含
toContain('phrase an unrun mutation as a reasoned hypothesis'),并保留了原有的verified N/N green钉桩与not.toContain('you have no runner')守卫。建议中的代码片段没有被逐字采纳,因为它会丢掉上述两个既有钉桩。 - 定向测试通过。
cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts→Tests 299 passed (299)。 - 变异 probe(witness 校验)。 仅从
agent-briefs.ts的 test-matrix bullet 中删除假设条款,保留被禁措辞与 witness 行——正是该发现所警告的未来编辑。此时定向测试在新的钉桩处失败:Tests 1 failed | 298 passed (299),断言错误位于toContain('phrase an unrun mutation as a reasoned hypothesis')一行。恢复文件后重跑 →Tests 299 passed (299),工作区干净。该钉桩因此拥有有效的 witness;R2-1 所描述的变异不会再绿灯溜过。 - 独立佐证。 自动审查者对同一提交(
920ed22394)的第 4 轮审查报告"未发现问题。LGTM!✅",零发现。
验证
npm run build— 通过(packages/cli 的单元测试构建前置守卫要求先构建才能运行 vitest)cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts— 通过:299 passed (299)- 变异 probe(仅删除
agent-briefs.ts中 test-matrix bullet 的假设条款)— 按要求失败:1 failed | 298 passed (299),失败点正是 lockstep 假设钉桩 - 恢复文件后重跑同一定向测试 — 通过:299 passed (299);
git status干净,无内容可提交
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
yiliang114
left a comment
There was a problem hiding this comment.
Approved. Prompt-only change adding witness discipline to the coverage and test-matrix briefs (unrun mutations phrased as hypotheses, execution-grade phrasings banned, not-run witness required); test pins both briefs in lockstep. No logic surface.
|
Released in v0.22.2. |
What this PR does
Adds witness discipline to the two reader-agent briefs that describe mutation analysis (the test-coverage agent and the whole-diff test-matrix agent): an unrung mutation must be phrased as a reasoned hypothesis, never as an executed result; the exact "ships N/N green" / "verified N/N green" phrasings are named and banned; and a finding whose weight depends on a run that was not performed must carry
witness: not run — <why>in its body, matching the verifier's existing convention. The rule is anchored on ownership — executed mutation verdicts belong to Agent 7's efficacy probe, which really runs mutants in a disposable probe tree — rather than on a capability claim the environment cannot guarantee.Why it's needed
In multi-round dogfood reviews, code-behaviour Criticals were consistently well-evidenced (real reproductions), while the test-coverage findings — the bulk of the finding tail — leaned on unrunnable mutation counts: bodies asserted "the mutation … ships 35/35 green" or "mutant verified 27/27 green" while the same review's gap notice admitted the worktree had no
node_modulesand the suite could not execute. The witness rule the skill documents only gates confirmed Criticals, and coverage findings are Suggestions by design, so the reading-based inference was presented in execution-grade language with nothing to catch it. "Verified" is the one word a maintainer trusts to mean observed; this closes the place the witness rule could be quietly hollowed out.Reviewer Test Plan
How to verify
This is prompt-text with regression assertions, not a runtime behaviour change. Run
cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts— the new assertions pin that Agent 5's brief carries the unrung-mutation paragraph, that both Agent 5 and the test-matrix carrywitness: not run —, that the ownership clause is present, and that the false-capability wording it replaced cannot come back. A reviewer may also read the two changed brief blocks in the agent-briefs source and confirm the wording satisfies the issue's "expected" criteria (either run the mutation and quote the real result, or carrywitness: not run — <why>with hypothesis phrasing — never "verified N/N green").Evidence (Before & After)
N/A — internal review-skill prompt text, no TUI surface. Before: the test-coverage brief let reader agents phrase mutation claims as "verified N/N green" with no run requirement and no witness. After: both reader briefs name and ban the execution-grade phrasing, prescribe the hypothesis form, and extend the
witness: not run — <why>convention to these findings.Tested on
Environment (optional)
Unit tests only (
packages/clivitest suite, 299/299 passing on the target files).Risk & Scope
Linked Issues
Fixes #9901
中文说明
本 PR 做了什么
给两个描述变异分析的阅读型 agent brief(测试覆盖 agent 和全 diff 测试矩阵 agent)加上 witness 纪律:未运行的变异必须以推理假设的措辞表述,绝不能写成已执行的结果;"ships N/N green"、"verified N/N green" 这两种执行级措辞被点名禁止;凡结论的分量依赖一次没有做过的运行的发现,正文必须带
witness: not run — <why>,与验证者既有约定一致。规则锚定在分工(ownership)上——真正执行的变异判定属于 Agent 7 的 efficacy probe(它确实在一次性 probe tree 里跑变异),而不是锚定在一个环境无法保证的能力断言上。为什么需要
在多轮 dogfood 评审中,代码行为类 Critical 一贯证据扎实(真复现),而测试覆盖类 finding(尾部主体)却靠跑不起来的变异计数:正文断言 "the mutation … ships 35/35 green"、"mutant verified 27/27 green",同一份 review 的缺口披露却自曝 worktree 没有
node_modules、套件根本跑不了。skill 文档里的 witness 规则只把关已确认的 Critical,而覆盖类 finding 按设计就是 Suggestion,于是"读代码推理、却用执行式措辞"没有任何机制能拦住。"verified" 恰恰是维护者信任为已观测的那个词;这个修复补上的正是 witness 规则可能被悄悄掏空的那个位置。给审查者的测试计划
如何验证
这是带回归断言的 prompt 文本,不是运行时行为变更。运行
cd packages/cli && npx vitest run src/commands/review/agent-prompt.test.ts——新增断言钉住:Agent 5 的 brief 含未运行变异段落;Agent 5 与 test-matrix 两条路径都含witness: not run —;ownership 条款存在;被它替换的虚假能力措辞不会回来。审查者也可以直接读 agent-briefs 源码里两处改动的 brief 文本,确认措辞满足 issue 的"期望"标准(要么真跑变异并引用真实结果,要么带witness: not run — <why>的假设式措辞——绝不能 "verified N/N green")。证据(Before & After)
N/A——review skill 内部 prompt 文本,无 TUI 界面。修复前:测试覆盖 brief 允许阅读型 agent 用 "verified N/N green" 表述变异结论,没有运行要求也没有 witness。修复后:两条阅读型 brief 点名并禁止执行级措辞,规定假设式表述,并把
witness: not run — <why>约定扩展到这类发现。测试环境
运行环境(可选)
仅单元测试(
packages/clivitest 套件,目标文件 299/299 通过)。风险与范围
关联 Issue
Fixes #9901