fix(review): give duplicate-dropped Suggestions their own compose state and body sentence - #9215
fix(review): give duplicate-dropped Suggestions their own compose state and body sentence#9215wenshao wants to merge 3 commits into
Conversation
…te and body sentence A review whose confirmed Suggestions were dropped because the PR already carried them (prior round or concurrent reviewer) had nowhere to record the drop except suggestionsDiscarded — whose body sentence asserts an anchor failure. On #9204 resolve-anchors returned three exact-added matches, the drop reason was duplication, and the posted body claimed the findings "could not be anchored to a changed line" — a public claim the run's own artifacts contradict. Add a suggestionsDroppedAsDuplicates state field: entries name the finding and where it already lives, render as their own body paragraph (bilingual, comment refs linkified), and count toward S exactly like anchor-failure discards so an all-duplicate run never reads as zero-finding. The skill's Step 7 state list now routes duplicate drops to it instead of the count. Co-authored-by: Qwen-Coder <[email protected]>
|
|
|
Thanks for the PR! Template looks good ✓ Problem: Observed misbehavior, not theory — a live Direction: Aligned — this is about the truthfulness of the review pipeline's posted output; drop reason is part of what a posted review owes the author. Internal tooling, no product-surface change. CHANGELOG: no direct reference, but the area is relevant by definition (the repo's own review orchestration). Size: Not applicable — no core-module logic change. Production logic is ~101 lines (78 + 22 in Approach: Scope feels right — a dedicated additive field for duplicate-dropped Suggestions, its own body paragraph with linkified comment anchors, and a one-line routing update in the bundled skill's checklist. The backward-compat claim (absent field → byte-identical compose) is worth checking in code review. One question I'm carrying into Stage 2: whether tagging the drop reason inside the existing discard field could have achieved the same with less surface area. Risk: No elevated risk signals — none of the high-risk paths are touched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的问题，而非理论推测——一次实际 方向：对齐——这关乎 review 流水线发布内容的准确性，丢弃原因本应是提交给作者的真相的一部分。属于内部工具改进，不涉及产品面变化。CHANGELOG：无直接引用，但该领域本身就是仓库自己的 review 编排，天然相关。 规模：不适用——未改动核心模块逻辑。生产逻辑约 101 行（ 方案：范围合理——为重复丢弃的 Suggestion 新增一个可选字段、正文中独立成段并为评论锚点加链接，另在捆绑技能清单中改一行路由。向后兼容声明（字段缺省时组合结果逐字节一致）值得在代码审查中核实。带一个问题进入 Stage 2：在现有丢弃字段内部标记丢弃原因，是否能以更小的改动面达到同样效果。 风险：无升级风险信号——未触及任何高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers found. I derived my own proposal from the title and motivation before reading the diff — additive optional string-list field, reuse the existing validation/linkify/footer helpers, count the drops toward Things I verified individually:
The six new tests pin the behavior matrix against the real composed output: all-duplicates stays COMMENT with the exact sentence and no "could not be anchored" wording; mixed runs carry both sentences; duplicates and anchor-failure discards coexist; bare Test evidenceUnattended CI run — this review quotes the PR's own CI checks via the API; no PR code was built or executed here. At fetch time the ubuntu unit suite (the full profile) is still running; Desktop Shell (both OSes), the dependency CVE audit, and the secret scan are green, with no failures. The macOS/Windows unit legs and the no-sandbox CLI integration job show Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The central claim here — compose output wording and the verdict for duplicate-dropped Suggestions — is a pure function of the input state, and the six new unit tests assert on the exact composed sentences and event. CI running them substantiates the claim; no sandboxed-lane trigger applies. 中文说明代码审查未发现阻断问题。读 diff 前我先根据标题和动机独立推导了自己的方案——新增可选字符串列表字段、复用现有校验/链接化/页脚处理辅助函数、将丢弃数计入 逐项核实：新字段经 六个新测试针对真实组合输出钉住行为矩阵：纯重复保持 COMMENT 且不含"无法锚定"措辞、混合场景两句并存、与锚定失败丢弃共存、裸 comment id 链接化为 测试证据：无人值守 CI 运行——本评论通过 API 引用 PR 自己的 CI 检查结果，未在此构建或运行任何 PR 代码。抓取时 ubuntu 单测套件（full profile）仍在运行；Desktop Shell（双平台）、依赖 CVE 审计、密钥扫描均通过，无失败。macOS/Windows 单测与免沙箱 CLI 集成任务在 PR 事件下按设计跳过（仅 merge_group 触发）。CI 结束后 finalize 工作流会更新上方表格。 核心主张（重复丢弃建议的正文措辞与判定）是输入状态的纯函数，六个新单测对组合出的句子和事件做了精确断言，CI 即可证实，无需沙箱验证通道。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage: the problem was observed in a real run, the fix is minimal, additive, backward-compatible, and reuses existing machinery better than my independent proposal did. Stepping back: I derived my own proposal first — an additive optional list field, reusing the existing validation / linkify / footer-strip helpers, counted toward One thing keeps this from being an approval comment: the ubuntu unit suite on this commit is still running. Approval is deferred until CI lands green on 中文说明Confidence: 5/5 —— 各阶段均干净：问题在实际运行中被观测到，修复最小、纯增量、向后兼容，且对现有机制的复用优于我的独立方案。 回顾：我先独立推导了自己的方案——新增可选列表字段，复用现有校验/链接化/页脚剥离辅助函数，计入 唯一没有直接批准的原因：该 commit 的 ubuntu 单测套件仍在运行。批准延后至 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (suggestionsDroppedAsDuplicates.length > 0) { | ||
| const pr = prIdentityFromPlan(input.planPath); |
There was a problem hiding this comment.
[Suggestion] The new clause-4a block adds a second prIdentityFromPlan(input.planPath) call inside composeReviewBody; the same computation already runs at line 1489 for the cannotTell block, and prIdentityFromPlan is unmemoized — each call does a fresh readFileSync + JSON.parse of the plan file. — Failure scenario: a run with both unresolved-existing-Critical entries and duplicate drops parses the same plan file twice in one compose; the two independent parse sites could linkify comment <id> refs in the same posted body against divergent PR identities if the reads ever disagreed.
Fix: hoist const pr = prIdentityFromPlan(input.planPath); once and pass it to both formatCannotTell (line 1489) and clause 4a.
中文说明
[建议] 新的 4a 子句块在 composeReviewBody 内新增了第二个 prIdentityFromPlan(input.planPath) 调用；同样的计算已在第 1489 行为 cannotTell 块执行，而 prIdentityFromPlan 未做记忆化——每次调用都会重新 readFileSync + JSON.parse 整个 plan 文件。— 失败场景：一次同时带有未决 Critical 条目和重复丢弃的 run 会在一次 compose 中解析同一个 plan 文件两次；两个独立的解析点一旦读取结果不一致，同一份发布的 body 就会基于不同的 PR 身份去链接 comment <id> 引用。
修复建议：将 const pr = prIdentityFromPlan(input.planPath); 提升为一次计算，同时传给 formatCannotTell（第 1489 行）和 4a 子句。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| .map((entry) => `- ${asListLine(entry, pr)}`) | ||
| .join('\n'), | ||
| zh: | ||
| `本轮确认的 ${suggestionsDroppedAsDuplicates.length} 条建议级发现已在 PR ` + |
There was a problem hiding this comment.
[Suggestion] The Chinese half of the new duplicate clause is never rendered by any test — no test composes with a han: true plan plus suggestionsDroppedAsDuplicates, and render() only emits the zh fold on bilingual runs. — Failure scenario: hardcoding the zh count or swapping in suggestionsDiscarded leaves all 233 shipped tests green; on a Han-description PR where 2 Suggestions were dropped as duplicates, such a regression ships and the Chinese fold contradicts the English count — the self-contradicting body this module exists to prevent. Sibling clauses already pin their zh sentences exactly (budget-stop, cannotTell zh tests).
Fix: add one bilingual test — coveredPlan(undefined, { han: true }) with a 2-entry suggestionsDroppedAsDuplicates, asserting the fold contains 本轮确认的 2 条建议级发现已在 PR 上报告过.
Witness: mutation probe — hardcoded the zh count to 1 → Tests 1 failed | 236 passed, the single failure being the added guard test; the guard passes on unmodified code. Probe flips.
中文说明
[建议] 新重复段落的中文句式没有任何测试渲染过——没有用例用 han: true 的 plan 加 suggestionsDroppedAsDuplicates 组装，而 render() 只在双语 run 中输出中文折叠块。— 失败场景：把中文计数硬编码或换成 suggestionsDiscarded，现有 233 个测试全部保持绿色；在一个中文描述的 PR 上若丢弃了 2 条建议，这类回归会带着矛盾发布——中文折叠块与英文计数不一致，而这正是本模块要杜绝的自相矛盾 body。兄弟子句已有先例精确固定中文句式（budget-stop、cannotTell 中文测试）。
修复建议：新增一个双语测试——coveredPlan(undefined, { han: true }) 加 2 条 suggestionsDroppedAsDuplicates，断言折叠块包含 本轮确认的 2 条建议级发现已在 PR 上报告过。
证据：变异探针——把中文计数硬编码为 1 → Tests 1 failed | 236 passed，唯一失败是新增的守卫测试；该守卫在未改动代码上通过。探针翻转成立。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| 'suggestionsDroppedAsDuplicates', | ||
| ) | ||
| .map(stripReviewFooter) | ||
| .filter((entry) => entry.trim() !== ''); |
There was a problem hiding this comment.
[Suggestion] The empty-entry filter (.filter((entry) => entry.trim() !== '')) on the new field has no test — no case passes a footer-only or whitespace-only entry, so deleting the filter is a surviving mutation. — Failure scenario: suggestionsDroppedAsDuplicates: [FOOTER] strips to ''; with the filter it vanishes and the sentence count matches the rendered list; without it, the body renders a dangling empty list item whose count overclaims the visible list. The sibling cannotTellCriticals path tests the same degenerate input ([' ']) and shares the identical filter.
Fix: add a case with suggestionsDroppedAsDuplicates: [FOOTER] (and/or [' ']) asserting the paragraph and sentence are absent when nothing else contributes to S, mirroring the cannotTell ' ' case.
Witness: mutation probe — deleted .filter(...) → all 233 shipped tests green; the guard test observed the mutated body — EVENT COMMENT with a dangling empty list item (...already reported on this PR and are not repeated:\n\n- ), an APPROVE flipped to COMMENT. Probe flips.
中文说明
[建议] 新字段上的空条目过滤器（.filter((entry) => entry.trim() !== '')）没有测试——没有用例传入仅 footer 或仅空白的条目，因此删除该过滤器是一个存活的变异。— 失败场景：suggestionsDroppedAsDuplicates: [FOOTER] 会被剥离为 ''；有过滤器时它消失，句式计数与渲染列表一致；没有过滤器时，body 会渲染出一个悬空的空列表项，计数虚报可见列表。兄弟路径 cannotTellCriticals 对同样的退化输入（[' ']）有测试，且共用同一个过滤器。
修复建议：新增 suggestionsDroppedAsDuplicates: [FOOTER]（和/或 [' ']）的用例，断言在没有其他项计入 S 时段落与句式均不出现，与 cannotTell 的 ' ' 用例对齐。
证据：变异探针——删除 .filter(...) → 现有 233 个测试全绿；守卫测试观察到变异后的 body——EVENT COMMENT 且带悬空空列表项（...already reported on this PR and are not repeated:\n\n- ），APPROVE 被翻转为 COMMENT。探针翻转成立。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
|
🔄 AutoFix is working on this PR — round 3/100. Watch live progress; this round posts its report here when it finishes. 中文说明🔄 AutoFix 正在处理此 PR —— 第 3/100 轮。查看实时进度；本轮结束后会在此发布报告。 |
…nt and empty-entry filter
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9215All three Suggestion findings from the automated reviewer were verified against the code at Findings and dispositions
Changes
Commit: Verification
中文说明Autofix 审查轮次 — PR #9215自动审查器的三条 Suggestion 发现均已在 发现与处置
变更内容
提交： 验证
🦷 Gate advisory — this round's changed tests all pass on the pre-round tree (machine-measured, not agent-authored). Expected for a refactor or coverage addition; if this round was meant to FIX a defect, that defect did not reproduce. · 本轮改动的测试在轮前树上全部通过（门自动测量，非 agent 文本）。对重构或补充覆盖属正常；若本轮意在修复缺陷，则该缺陷未能复现。 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.
Test Plan (not a blocker): 298 tests green — this review observed 19981, 1432, 1555, 285, 3596, 491 passed.
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：298 tests green — this review observed 19981, 1432, 1555, 285, 3596, 491 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const suggestionsDroppedAsDuplicates = toStringList( | ||
| input.suggestionsDroppedAsDuplicates, | ||
| 'suggestionsDroppedAsDuplicates', | ||
| ) |
There was a problem hiding this comment.
[Suggestion] The normalize chain toStringList(...).map(stripReviewFooter).filter((entry) => entry.trim() !== '') is now copied into a fourth site in this file — ~lines 571–574 (ledger build), ~599–601 (bodyCriticals), 610–615 (this addition), ~616–621 (cannotTellCriticals) — instead of one shared helper; the new copy's comment points back at bodyCriticals's rationale instead of at a single implementation. — Failure scenario: the cost is paid every time a list-shaped field is added — this PR just paid it — and the omission is asymmetric: forgetting .map(stripReviewFooter) lets an entry with a forged footer post directly above the canonical footer (the exact injection the chain exists to block), and forgetting .filter(...) lets empty entries inflate s, flipping a clean APPROVE into a COMMENT and rendering a dangling empty list item (the regression this PR's own drops entries that normalize to nothing test pins).
Extract one helper and call it at all four sites:
function strippedList(
input: ComposeReviewInput,
key: 'bodyCriticals' | 'suggestionsDroppedAsDuplicates' | 'cannotTellCriticals',
): string[] {
return toStringList(input[key], key)
.map(stripReviewFooter)
.filter((entry) => entry.trim() !== '');
}中文说明
规范化链 toStringList(...).map(stripReviewFooter).filter((entry) => entry.trim() !== '') 现在被复制到了本文件的第四处——约 571–574 行（ledger 构建）、约 599–601 行（bodyCriticals）、610–615 行（本次新增）、约 616–621 行（cannotTellCriticals）——而不是提取为一个共享 helper；新副本的注释指向 bodyCriticals 的理由，而不是指向单一实现。——失败场景：每新增一个列表型字段都要付一次这个成本——本 PR 刚刚付了——而且遗漏是不对称的：漏掉 .map(stripReviewFooter) 会让带伪造 footer 的条目直接贴在 canonical footer 上方（正是该链要阻止的注入）；漏掉 .filter(...) 会让空条目抬高 s，把干净的 APPROVE 翻成 COMMENT，并渲染出一个悬空的空列表项（本 PR 自己的 drops entries that normalize to nothing 测试所固定的回归）。
建议修复：提取一个 helper（如上）并在全部四处调用。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (suggestionsDroppedAsDuplicates.length > 0) { | ||
| clauses.push({ | ||
| en: |
There was a problem hiding this comment.
[Suggestion] Clause 4a is never rendered under REQUEST_CHANGES, and no test supplies duplicates alongside a Critical — probe-verified: composing one bodyCriticals entry plus two duplicate entries yields event: REQUEST_CHANGES with a body containing only the Critical and the footer, because the REQUEST_CHANGES branch returns a fixed parts list that never includes clause 4a, even though s counted both duplicates and the diff's own comment says "the verdict counted them in s, so the body owes the author a truthful account of where they went." — Failure scenario: a run with 1 confirmed Critical and 2 duplicate-dropped Suggestions posts a REQUEST_CHANGES body with no account of where the two counted-but-unposted Suggestions went; the named mutation if (c === 0 && suggestionsDroppedAsDuplicates.length > 0) survives the whole new suite (measured: Tests 8 passed | 231 skipped under the mutant), so no test guards the clause's event gating in either direction.
Witness: probe run in both arms — unmodified code drops the duplicate account from the REQUEST_CHANGES body; the mutant survives the full duplicate-dropped Suggestions suite, while the downgraded mixed-run body contains the paragraph under unmodified code and not under the mutant.
Decide the intended REQUEST_CHANGES behavior: either include clause 4a in that branch (matching the "truthful account" rationale), or document its exclusion; then add a test composing one Critical plus suggestionsDroppedAsDuplicates and assert the chosen shape.
中文说明
clause 4a 在 REQUEST_CHANGES 下从不渲染，且没有任何测试在提供 duplicate 的同时提供 Critical——已用 probe 验证：组装一条 bodyCriticals 加两条 duplicate 条目会得到 event: REQUEST_CHANGES，body 只含 Critical 与 footer，因为 REQUEST_CHANGES 分支返回固定的 parts 列表、从不包含 clause 4a，尽管 s 计入了两条 duplicate，且 diff 自己的注释写着「裁决已在 s 中计入它们，body 就欠作者一份关于它们去向的真实说明」。——失败场景：一次有 1 条已确认 Critical 和 2 条重复丢弃 Suggestion 的运行，发布的 REQUEST_CHANGES body 对两条已计数但未发布的 Suggestion 的去向只字未提；命名的变异 if (c === 0 && suggestionsDroppedAsDuplicates.length > 0) 在整个新套件中存活（实测：变异下 Tests 8 passed | 231 skipped），因此没有任何测试在任一方向上守卫该 clause 的事件门控。
证据：双臂 probe 运行——未修改代码从 REQUEST_CHANGES body 中丢弃 duplicate 说明；变异体在整个 duplicate-dropped Suggestions 套件中存活，而降级的混合运行 body 在未修改代码下包含该段落、在变异体下不包含。
建议修复：确定 REQUEST_CHANGES 下的预期行为：要么在该分支包含 clause 4a（与「真实说明」的理由一致），要么明确记录其排除；然后新增一个组装一条 Critical 加 suggestionsDroppedAsDuplicates 的测试，断言所选形态。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| }); | ||
| }); | ||
|
|
||
| describe('composeReview — duplicate-dropped Suggestions (#9204: the body claimed an anchor failure that never happened)', () => { |
There was a problem hiding this comment.
[Suggestion] Every new test calls composeReview directly; none carries suggestionsDroppedAsDuplicates through submit.ts's state seam, which strips keys via a destructuring exclusion list (env, prBodyFetcher, draftedComments, submit.ts ~235–251) before spreading the rest — and submit.test.ts contains zero occurrences of the field, while the sibling precedent exists verbatim (the REVIEW fixture at submit.test.ts:102 carries suggestionsDiscarded: 1). — Failure scenario: if suggestionsDroppedAsDuplicates is ever added to that exclusion list, a duplicates-only run silently posts APPROVE with no duplicate paragraph — precisely the incident shape this PR fixes — and all eight new unit tests stay green because they bypass the seam.
Add a seam test, e.g. a review JSON carrying state: { suggestionsDroppedAsDuplicates: ['R1-1 pin gap — already reported (comment 1)'], modelId: 'm' } with comments: [], asserting the posted event is COMMENT and the body contains the "this review confirmed" sentence.
中文说明
所有新测试都直接调用 composeReview；没有任何一个让 suggestionsDroppedAsDuplicates 经过 submit.ts 的 state 接缝——该接缝通过解构排除列表（env、prBodyFetcher、draftedComments，submit.ts 约 235–251 行）剥掉键后再展开其余部分——而 submit.test.ts 中该字段出现次数为零，同期先例却原样存在（submit.test.ts:102 的 REVIEW fixture 带有 suggestionsDiscarded: 1）。——失败场景：一旦 suggestionsDroppedAsDuplicates 被加入该排除列表，全重复的运行就会静默发布 APPROVE 且没有 duplicate 段落——正是本 PR 要修复的事故形态——而全部八个新单元测试依然全绿，因为它们绕过了该接缝。
建议修复：新增一个接缝测试（如示例），断言发布的 event 为 COMMENT 且 body 包含相应句式。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — round 2 (PR #9215)All three Suggestion findings from the automated reviewer (round 1) were implemented. No Critical or Request-changes findings were present. Findings and dispositionsR1-1 — normalize chain copied into a fourth site (rc:3789355766) — IMPLEMENTED. R1-2 — clause 4a never rendered under REQUEST_CHANGES (rc:3789355768) — IMPLEMENTED (include the clause). R1-3 — no test carries the field through submit.ts's state seam (rc:3789355769) — IMPLEMENTED. Not addressed / escalatedNone — all findings resolved in code this round. Verification
中文说明已处理的审查反馈 — 第 2 轮（PR #9215）自动审查者（第 1 轮）提出的三条 Suggestion 发现均已实现。本轮没有 Critical 或 Request-changes 发现。 发现与处理结果R1-1 — 规范化链被复制到第四处（rc:3789355766）— 已实现。 R1-2 — clause 4a 在 REQUEST_CHANGES 下从不渲染（rc:3789355768）— 已实现（选择包含该 clause）。 R1-3 — 没有测试让该字段经过 submit.ts 的 state 接缝（rc:3789355769）— 已实现。 未处理 / 升级无 — 本轮所有发现均已在代码中解决。 验证
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.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 298 tests green — this review observed 19981, 1432, 1555, 494, 3596, 491 passed.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：298 tests green — this review observed 19981, 1432, 1555, 494, 3596, 491 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| planPath: coveredPlan(undefined, { | ||
| ownerRepo: 'QwenLM/qwen-code', | ||
| prNumber: '9204', | ||
| }), |
There was a problem hiding this comment.
[Suggestion] The new duplicate-drop tests' COMMENT event assertions are carried by unrelated coverage caps, not by the duplicate count, so both fixtures survive this PR's core verdict mutation.
Pattern: a duplicate-drop test whose fixture leaves an unrelated cap in place cannot observe the verdict flip it exists to pin — removing suggestionsDroppedAsDuplicates.length from s (the exact regression this PR fixes) leaves the event assertion green because the expected COMMENT is produced by the cap, not by the count.
Occurrences: 2 locations
- here (named-PR fixture):
coveredPlan()records the PR identity but no linked-issue fidelity pass brief, socomposeReviewcaps withunreviewed-dimension— the duplicate paragraph renders because of the cap, and the body pins "Partially reviewed" instead of a clean COMMENT; packages/cli/src/commands/review/submit.test.ts(the new seam test, at the other anchor): the fixture'sstatecarries noplanPath, so the missing-plan fail-closed cap forces COMMENT regardless of C/S.
Failure scenario: with the duplicate count removed from s, the named-PR test here still passes (probe: baseEvent flips to APPROVE, event stays COMMENT via the cap) and all 66 submit tests stay green, while 3 compose-review tests fail — a verdict regression that spared duplicates only on capped/named-PR shapes would ship green through these two tests. The suite is protected elsewhere (three direct tests kill the mutant; the seam test's body assertion catches the exclusion-list regression), but the verdict-side pin these tests' comments advertise is illusory.
Witness (probe): unmutated named-PR fixture: cappedBy: ["unreviewed-dimension"], body opens "Partially reviewed — gaps disclosed."; under the mutation the test passes while the probe shows baseEvent: "APPROVE", event: "COMMENT"; recording the fidelity brief in the fixture makes the mutant fail AssertionError: expected 'APPROVE' to be 'COMMENT'. Seam test: mutation run Tests 66 passed (66); a fix probe with planPath: verifiedPlan() still yields a cap-determined COMMENT (coverage caps), so the fix needs a coverage-satisfying plan.
Suggested fix: give each fixture a plan state that satisfies coverage so the duplicate count alone determines the event (record the fidelity pass brief in the named-PR fixture; give the seam fixture a coverage-satisfying planPath), or assert expect(r.cappedBy).toEqual([]) so the accidental cap fails loudly.
中文说明
[建议] 新的重复丢弃测试中 COMMENT 事件断言是由无关的覆盖上限（cap）兜底的，而不是由重复计数兜底的，因此这两个 fixture 在本 PR 的核心裁决变异下依然通过。
模式： 重复丢弃测试的 fixture 若残留一个无关的 cap，就无法观察到它本应固定的裁决翻转——把 suggestionsDroppedAsDuplicates.length 从 s 中移除（正是本 PR 修复的回归）后事件断言仍为绿色，因为期望的 COMMENT 来自 cap，而非计数。
出现位置： 2 处
- 此处（命名 PR 的 fixture）：
coveredPlan()记录了 PR 身份，但没有记录 linked-issue 保真度检查（fidelity pass）的 brief，因此composeReview被unreviewed-dimension上限兜底——重复段落是因 cap 而渲染的，body 固定为 "Partially reviewed"，而不是干净的 COMMENT； packages/cli/src/commands/review/submit.test.ts（新的接缝测试，另一处锚点）：fixture 的state不带planPath，因此「缺少 plan」的 fail-closed 上限无论 C/S 如何都强制 COMMENT。
失败场景： 把重复计数从 s 中移除后，此处的命名 PR 测试仍然通过（探针：baseEvent 翻转为 APPROVE，event 经由 cap 保持 COMMENT），submit 的全部 66 个测试也仍为绿色，而 compose-review 有 3 个测试失败——一个只在「被 cap/命名 PR」形态下放过重复项的裁决回归，将在这两个测试全绿的情况下带病合入。套件在别处有保护（三个直接测试能杀死该变异体；接缝测试的 body 断言能捕获排除列表回归），但这两个测试注释所宣称的「裁决侧固定」是虚的。
证据（探针）： 未变异的命名 PR fixture：cappedBy: ["unreviewed-dimension"]，body 以 "Partially reviewed — gaps disclosed." 开头；变异下测试通过，而探针显示 baseEvent: "APPROVE", event: "COMMENT"；在 fixture 中记录 fidelity brief 后，变异体失败于 AssertionError: expected 'APPROVE' to be 'COMMENT'。接缝测试：变异运行 Tests 66 passed (66)；用 planPath: verifiedPlan() 的修复探针仍然得到由 cap 决定的 COMMENT（覆盖上限），因此修复需要一个能满足覆盖的 plan。
建议修复： 为每个 fixture 提供能满足覆盖的 plan 状态，使事件仅由重复计数决定（在命名 PR 的 fixture 中记录 fidelity pass brief；为接缝 fixture 提供满足覆盖的 planPath），或断言 expect(r.cappedBy).toEqual([])，让意外的 cap 响亮地失败。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(posted().event).toBe('COMMENT'); | ||
| expect(posted().body).toContain( | ||
| '1 Suggestion-level finding(s) this review confirmed', |
There was a problem hiding this comment.
[Suggestion] The new duplicate-drop tests' COMMENT event assertions are carried by unrelated coverage caps, not by the duplicate count, so both fixtures survive this PR's core verdict mutation.
Pattern: a duplicate-drop test whose fixture leaves an unrelated cap in place cannot observe the verdict flip it exists to pin — removing suggestionsDroppedAsDuplicates.length from s (the exact regression this PR fixes) leaves the event assertion green because the expected COMMENT is produced by the cap, not by the count.
Occurrences: 2 locations
- here (seam test): the fixture's
statecarries noplanPath, so the missing-plan fail-closed cap forces COMMENT regardless of C/S — the event assertion this test's comment justifies ("if it ever joins the exclusion list, a duplicates-only run silently posts APPROVE") is structurally unobservable in this fixture; packages/cli/src/commands/review/compose-review.test.ts(the named-PR linkify test, at the other anchor):coveredPlan()records the PR identity but no linked-issue fidelity pass brief, socomposeReviewcaps withunreviewed-dimensionand the duplicate paragraph renders because of the cap.
Failure scenario: with the duplicate count removed from s, all 66 submit tests stay green — including this test's expect(posted().event).toBe('COMMENT') — while 3 compose-review tests fail (probe-verified). A verdict regression that spared duplicates only on plan-less/capped shapes would ship green through this test. The body assertion below still catches the exclusion-list regression, so nothing escapes the suite today — but the verdict-side pin advertised here is illusory.
Witness (probe): mutation run: submit.test.ts: Tests 66 passed (66) with the seam test's event assertion green under the mutation; compose-review.test.ts: Tests 3 failed | 233 passed with AssertionError: expected 'APPROVE' to be 'COMMENT'. Fix probe: planPath: verifiedPlan() alone does NOT restore the pin — the verifiedPlan fixture satisfies no coverage, so the event stays cap-determined (coverage caps); the fix needs a coverage-satisfying plan.
Suggested fix: give the fixture a plan whose transcripts satisfy coverage (the coveredPlan() scaffolding lives in compose-review.test.ts), or rewrite the event assertion/comment to claim only what the body assertion pins.
中文说明
[建议] 新的重复丢弃测试中 COMMENT 事件断言是由无关的覆盖上限（cap）兜底的，而不是由重复计数兜底的，因此这两个 fixture 在本 PR 的核心裁决变异下依然通过。
模式： 重复丢弃测试的 fixture 若残留一个无关的 cap，就无法观察到它本应固定的裁决翻转——把 suggestionsDroppedAsDuplicates.length 从 s 中移除（正是本 PR 修复的回归）后事件断言仍为绿色，因为期望的 COMMENT 来自 cap，而非计数。
出现位置： 2 处
- 此处（接缝测试）：fixture 的
state不带planPath，因此「缺少 plan」的 fail-closed 上限无论 C/S 如何都强制 COMMENT——本测试注释所论证的事件断言（「一旦该字段进入排除列表，纯重复运行会悄悄发布 APPROVE」）在此 fixture 中结构上不可观察； packages/cli/src/commands/review/compose-review.test.ts（命名 PR 的 linkify 测试，另一处锚点）：coveredPlan()记录了 PR 身份，但没有记录 linked-issue 保真度检查的 brief，因此composeReview被unreviewed-dimension上限兜底，重复段落是因 cap 而渲染的。
失败场景： 把重复计数从 s 中移除后，submit 全部 66 个测试仍为绿色——包括本测试的 expect(posted().event).toBe('COMMENT')——而 compose-review 有 3 个测试失败（探针验证）。一个只在「无 plan/被 cap」形态下放过重复项的裁决回归，将在本测试全绿的情况下带病合入。下方的 body 断言仍能捕获排除列表回归，所以今天没有任何回归漏网——但此处所宣称的「裁决侧固定」是虚的。
证据（探针）： 变异运行：submit.test.ts: Tests 66 passed (66)，接缝测试的事件断言在变异下为绿色；compose-review.test.ts: Tests 3 failed | 233 passed，失败于 AssertionError: expected 'APPROVE' to be 'COMMENT'。修复探针：仅加 planPath: verifiedPlan() 并不能恢复固定效果——verifiedPlan fixture 不满足任何覆盖，事件仍由 cap 决定（覆盖上限）；修复需要一个能满足覆盖的 plan。
建议修复： 为 fixture 提供其转录能满足覆盖的 plan（coveredPlan() 脚手架位于 compose-review.test.ts），或重写事件断言/注释，只宣称 body 断言实际固定的内容。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| en: | ||
| `${suggestionsDroppedAsDuplicates.length} Suggestion-level ` + | ||
| `finding(s) this review confirmed are already reported on this PR ` + |
There was a problem hiding this comment.
[Suggestion] The duplicates paragraph renders Suggestion-level finding text into the review body, contradicting the "Suggestion text must never appear in the review body" rule in the same SKILL.md this PR edits (Step 7), whose stated rationale is that qwen-autofix.yml does not filter review bodies and hands body content to the autofix bot as actionable work. The paragraph renders on every event, so the entries enter exactly the body channel that rule exists to keep them out of.
Failure scenario: verified by running the workflow's own filters verbatim against a composed fixture: with critical_only=false the all-duplicates COMMENTED body is dumped verbatim (all entries) into the autofix fix agent's actionable "## Reviews" section; with critical_only=true a REQUEST_CHANGES body carrying this paragraph is kept whole, so Suggestion-level items reach the fix agent despite the Critical-only brake; and an entry naming a Suggestion whose original inline comment is past the autofix watermark can resurrect it as new feedback through the body channel. (One claimed consequence did not hold: pre-PR recorded-duplicate runs already posted COMMENT, so there is no APPROVE→COMMENT wake-up delta for that shape.) The realistic cost is a wasted/no-op triage pass and a rule-consistency breach rather than new wrong work — the entries self-describe as "already reported… not repeated" with a link — but the contradiction with the rule this PR's own file states is direct.
Witness (probe): critical_only=false → [COMMENTED] @qwen-code-ci-bot: … 3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated: - R1-1 … (comment 3788857375) dumped into the actionable section; critical_only=true → the same RC body printed whole with its duplicate entries while the COMMENTED review was deferred.
Suggested fix: mark the paragraph so the autofix feedback filter excludes it (as the existing bot-comment marker family does), or amend the SKILL.md never-in-body rule to carve out this paragraph and cap entries at the finding name + location so actionable Suggestion text cannot ride them.
中文说明
[建议] 重复丢弃段落把建议级（Suggestion）发现的文本渲染进了 review body，与本 PR 所修改的同一份 SKILL.md 中的规则「建议文本绝不能出现在 review body 中」（第 7 步）相矛盾——该规则写明理由：qwen-autofix.yml 不过滤 review body，会把 body 内容交给 autofix 机器人当作可执行工作。该段落在每种事件下都会渲染，因此条目进入的正是该规则要避免其进入的 body 通道。
失败场景： 用该工作流自己的过滤器对组装好的 fixture 逐字运行验证：critical_only=false 时，纯重复的 COMMENTED body 被逐字（含全部条目）倒入 autofix 修复代理的可执行 "## Reviews" 区；critical_only=true 时，携带本段落的 REQUEST_CHANGES body 被整体保留，因此尽管有「仅 Critical」刹车，建议级条目仍会到达修复代理；此外，若某条目提及的建议其原行内评论已在 autofix 水位线之前，它可能经由 body 通道作为新反馈复活。（此前声称的一个后果不成立：PR 之前记录了重复丢弃的运行本来就发布 COMMENT，因此该形态不存在 APPROVE→COMMENT 的唤醒差异。）实际代价是一次浪费/空转的分拣过程和对规则一致性的破坏，而非新的错误工作——条目自带「已报告过……不再重复」及链接——但与本 PR 所修改文件自身陈述的规则直接矛盾。
证据（探针）： critical_only=false → [COMMENTED] @qwen-code-ci-bot: … 3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated: - R1-1 … (comment 3788857375) 被倒入可执行区；critical_only=true → 同一 RC body 连同其重复条目被整体打印，而 COMMENTED review 被延后处理。
建议修复： 给该段落加标记，使 autofix 反馈过滤器将其排除（如同现有机器人评论标记家族的做法），或修订 SKILL.md 的「不得进入 body」规则，为本段落开一个豁免口，并把条目限定为「发现名称 + 位置」，使可执行的建议文本无法搭载其间。
— qwen3.8-max via Qwen Code /review (v0.21.12)
What this PR does
Gives
/reviewsubmissions a dedicated state field for Suggestion findings that were confirmed but not re-posted because the PR already carries them — from a prior review round, or from a concurrent reviewer that posted while the run was in flight. Each entry names the finding and where it already lives; the composed review body renders them as their own bilingual paragraph ("already reported on this PR and are not repeated") with each entry's bare comment id linkified to its GitHub anchor, instead of folding them into the anchor-failure count whose sentence claims the findings "could not be anchored to a changed line". The two drop reasons still count identically toward the Suggestion total, so an all-duplicate run composes as Comment rather than reading as a zero-finding Approve. The bundled skill's submission checklist now routes duplicate drops to the new field instead of the count.Why it's needed
A manual deep review of a PR ran while the repository's CI review pipeline posted its own findings on the same commit. The manual run confirmed three Suggestions, dropped all of them as duplicates, and submitted — and the posted body said the three "could not be anchored to a changed line and were dropped", because the only state field that kept them counting toward the verdict was the anchor-failure one. The anchor resolver's own output for that run recorded three exact, unambiguous matches. The body was publicly contradicting the run's artifacts, and the author reading it would infer an anchoring bug that never happened. Drop reason is part of the truth a posted review owes the author; the verdict machinery had no channel for it.
Reviewer Test Plan
How to verify
Behavior matrix (unit-pinned in the compose-review suite):
COMMENT, neverAPPROVE, and its body carries the new sentence plus one list entry per finding; it does not contain "could not be anchored".comment <id>becomes a link to that comment's GitHub anchor (#discussion_r<id>).suggestionsDroppedAsDuplicatestype error, same as the other model-written list fields.Evidence (Before & After)
Non-UI change; N/A — see the unit tests (6 new cases in the duplicate-dropped Suggestions describe block).
Tested on
Environment (optional)
Local dev tree;
npx vitest runon the compose-review and submit suites (298 tests green), reponpm run typecheck, eslint and prettier clean on the changed files.Risk & Scope
suggestionsDiscarded's existing semantics.Linked Issues
This PR closes none of them — it fixes the body-wording defect directly, surfaced by a maintainer dogfood run on a live PR. The four open issues below came out of that same run's retrospective and cover the adjacent machinery this PR deliberately leaves untouched:
--new-findingsrejected the findings artifact whose path the skill's own example collides with.noConflict, so the truthfulness fix had to be driven by manual dedup upstream.中文说明
本 PR 做了什么
为
/review提交新增一个专用 state 字段，用于记录「已确认但因 PR 上已存在而未重发」的 Suggestion 级发现——来源可以是上一轮审查，也可以是运行期间并发发布的其他审查者。每条记录为：render 成自己的双语段落（"already reported on this PR and are not repeated"），并把条目中的裸comment <id>自动链接到该评论的 GitHub 锚点，而不是折进锚定失败计数——那个计数的句式会宣称发现 "could not be anchored to a changed line"。两种丢弃原因对 Suggestion 总数的计入完全相同，因此全重复的 run 会组装为 Comment，而不是被读成零发现的 Approve。bundled skill 的提交清单现在把重复丢弃路由到新字段，而不是那个计数。为什么需要
一次 PR 的手动深度审查运行期间，仓库的 CI 审查流水线在同一 commit 上发布了它自己的发现。手动运行确认了三条 Suggestion、全部作为重复丢弃后提交——而发布的 body 写着这三条 "could not be anchored to a changed line and were dropped"，因为唯一能让它们继续计入裁决的 state 字段就是锚定失败那个。该 run 的锚点解析器自己的输出记录的是三次精确、无歧义的匹配。body 公然与 run 自身的产物相矛盾，读到的作者会推断出一个从未发生的锚定 bug。丢弃原因是发布出去的 review 欠作者的事实的一部分；裁决机制此前没有承载它的通道。
审查者测试计划
如何验证
行为矩阵（由 compose-review 套件的单元测试固定）：
COMMENT，绝不可能是APPROVE；body 带新句式外加每条发现一个列表项；不包含 "could not be anchored"。comment <id>变为指向该评论 GitHub 锚点（#discussion_r<id>）的链接。suggestionsDroppedAsDuplicates类型错误被拒绝，与其他模型写入的列表字段一致。证据（Before & After）
非 UI 变更；N/A——见单元测试（duplicate-dropped Suggestions describe 块新增 6 个用例）。
测试平台
环境（可选）
本地开发树；对 compose-review 与 submit 套件执行
npx vitest run（298 个测试全绿）、仓库级npm run typecheck、变更文件 eslint 与 prettier 干净。风险与范围
suggestionsDiscarded现有语义的任何改动。关联 Issue
本 PR 不关闭其中任何一个——它直接修复的是措辞与事实不符的缺陷，由维护者在真实 PR 上的 dogfood 运行发现。下面四个 open issue 来自同一次运行的复盘，覆盖本 PR 刻意不动的相邻机制：
--new-findings拒绝了路径与 skill 自身示例冲突的 findings 产物。noConflict，迫使本真性修复的上游依赖手工查重。