ci(autofix): treat Suggestion-level review findings as actionable per AGENTS.md - #7094
Conversation
… AGENTS.md AGENTS.md's review policy: Suggestions ARE addressed during a PR's first ~5 review rounds; only past that are they deferred with a recorded reason. The autofix loop's QWEN_SUGGESTION_FILTER contradicted this by unconditionally hiding /review **[Suggestion]** inline comments from both the scan's feedback count and the agent-facing feedback rendering — at every round. Since the loop's MAX_ROUNDS cap (5) is the same boundary the policy names, every round the loop actually runs is within the address-Suggestions window: drop the filter from both sites, and align the SKILL's Optional triage with the policy (implement valuable suggestions; decline only with a recorded per-finding reason). Contract test pins the filter's absence. 50/50.
|
Thanks for the PR! Template looks good ✓ — all required headings present (What, Why, Test Plan, Risk, Linked Issues, 中文说明). Problem: observed, not theoretical. The filter was live in production and confirmed on PR #6929 — three Direction: aligned with AGENTS.md's documented policy — Suggestions are addressed during a PR's first ~5 review rounds, only deferred past that boundary. The loop's Size: not applicable — no core modules touched (CI workflow + skill docs + contract test). 40 additions / 18 deletions across 3 files. Approach: minimal and focused. Three targeted changes: (1) remove the filter from the scan's Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必需标题齐全(做了什么、为什么、评审验证方案、风险、关联 Issue、中文说明)。 问题:已观测到的问题,非理论性加固。过滤器在生产环境中实际运行,在 PR #6929 上已确认 —— 方向:与 AGENTS.md 文档化政策对齐 —— Suggestion 在 PR 前约 5 轮评审内应被处理,超过该边界才延期。回路的 规模:不适用 —— 未触及核心模块(CI 工作流 + skill 文档 + 契约测试)。3 个文件,40 行新增 / 18 行删除。 方案:最小化且聚焦。三处针对性改动:(1) 从扫描的 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
|
Code review: The diff is clean and focused — three surgical edits that do exactly what the PR describes:
No correctness issues, no security concerns, no regressions. The old Contract test verification (manual — All four contract test assertions pass. YAML parses cleanly. 中文说明代码审查: diff 干净且聚焦 —— 三处精确改动,完全匹配 PR 描述:
无正确性问题、无安全隐患、无回归。旧的 契约测试验证(手动 —— 此环境中 全部四个契约测试断言通过。YAML 可解析。✓ — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; would merge without hesitation. This is a textbook bug-fix PR: a clearly observed problem (Suggestion-level findings invisible to the autofix agent on PR #6929), a minimal fix that directly contradicts documented policy (AGENTS.md says Suggestions are addressed in the first ~5 rounds; the loop's unconditional filter said otherwise), and a contract test that pins the fix. My independent proposal — remove the filter variable and its Every change in the diff is needed. No drive-by refactors, no scope creep, no speculative hardening. The The risk noted in the PR (noisy Suggestion-heavy reviews consuming bot rounds) is real but bounded by the existing Approving. ✅ 中文说明置信度:5/5 —— 各阶段均通过,可毫不犹豫地合并。 这是一个教科书式的 bug 修复 PR:明确观测到的问题(Suggestion 级 finding 对 autofix agent 不可见,在 PR #6929 上确认)、最小化修复(直接矛盾于文档化政策 —— AGENTS.md 规定 Suggestion 在前约 5 轮内处理;回路的无条件过滤器持相反立场)、以及钉住修复的契约测试。 我的独立方案 —— 从扫描和反馈渲染两处移除过滤变量及其 diff 中每一处改动都是必要的。无顺手重构、无范围蔓延、无投机性加固。 PR 中提到的风险(Suggestion 密集的 review 消耗 bot 轮次)真实存在,但受既有 批准合并。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Aligns the autofix loop's treatment of
**[Suggestion]**-level review findings with the review policy documented in AGENTS.md. The loop previously filtered /review Suggestion inline comments out of both the scan's actionable-feedback count and the agent-facing feedback rendering — unconditionally, at every round — so the addressing agent never even saw them. This PR removes that filter from both sites, and updates the agent skill's triage guidance so each suggestion is implemented when valuable and otherwise declined with a recorded per-finding reason. The contract test pins the filter's absence.Why it's needed
AGENTS.md states the policy explicitly: a PR's Suggestions ARE addressed during its first roughly 5 review rounds; only past that boundary should remaining Suggestions be deferred to a follow-up, with each deferral recorded in the PR thread. The loop's
MAX_ROUNDScap (5) is that same boundary — every round the loop actually runs falls inside the address-Suggestions window — so the unconditional filter contradicted the documented policy. Observed live on PR #6929: a /review pass left three Suggestion-level inline comments (test coverage fortool_choicemapping, an unnecessary double cast, a missinggenerateJsontoolConfig test), and the bot's evaluation reported "no inline comment exists to act on" because the renderer had filtered all three out before the agent ever saw the feedback. After this change the agent sees them and triages each one on the record; the existingMAX_ROUNDShandoff remains the defer-to-a-human boundary the policy prescribes.Reviewer Test Plan
How to verify
MAX_ROUNDS(5), so every loop round is in the address window.N_COMMENTSjq and the feedback renderer's "Inline comments" jq no longer carry the**[Suggestion]**+via Qwen Code /reviewexclusion; the SKILL's Optional triage now requires implementing valuable suggestions and recording a per-finding reason when declining.npx vitest run scripts/tests/qwen-autofix-workflow.test.js— 50/50, including the new test that assertsQWEN_SUGGESTION_FILTERand the escaped filter regex are gone from the workflow and that the SKILL forbids silently dropping a finding.run:block passesbash -n.Behavioral expectation after merge: on PR #6929 (or any bot PR), a fresh /review pass whose Suggestion comments post after the last eval marker makes the scan target the PR, and the agent's summary addresses each suggestion explicitly — implement or decline-with-reason — instead of reporting there was nothing to act on.
Evidence (Before & After)
Tested on
Environment (optional)
Contract test via vitest on a worktree checkout of main; YAML/bash static checks locally. The behavior itself runs on
ubuntu-latestin the autofix workflow.Risk & Scope
MAX_ROUNDScap — which is exactly the balloon-guard AGENTS.md prescribes — and by the SKILL's scope guidance (decline out-of-scope suggestions with a reason).**[Critical]**path is unchanged (it was never filtered).Linked Issues
Follow-up to #6998, prompted by the live validation on #6929 where three Suggestion findings were invisible to the loop.
中文说明
本 PR 做了什么
让 autofix 回路对
**[Suggestion]**级 review finding 的处理与 AGENTS.md 文档化的评审政策对齐。此前回路在扫描的可行动反馈计数和 agent 侧反馈渲染两处无条件(任何轮次)过滤掉 /review 的 Suggestion 行内评论 —— 处理 agent 根本看不到它们。本 PR 移除两处过滤,并更新 agent skill 的分诊指引:有价值的建议应当实现,拒绝时必须逐条记录理由。契约测试钉住过滤器的不存在。为什么需要
AGENTS.md 明文规定:PR 的 Suggestion 在前约 5 轮评审内应当被处理;只有超过该边界后才把剩余 Suggestion 延期到后续 issue/PR,且每次延期都要在 PR 线程中记录。回路的
MAX_ROUNDS上限(5)正是同一边界 —— 回路实际运行的每一轮都落在"处理 Suggestion 窗口"内 —— 因此无条件过滤与文档化政策相悖。在 PR #6929 上实测:一次 /review 留下 3 条 Suggestion 行内评论(tool_choice映射的测试覆盖、多余的双重 cast、缺失的generateJsontoolConfig 测试),而 bot 的评估报告"不存在可处理的行内评论" —— 因为渲染器在 agent 看到反馈之前就把三条全滤掉了。本改动后 agent 能看到它们并逐条留痕分诊;既有的MAX_ROUNDS交接仍是政策规定的"移交人类"边界。评审验证方案
如何验证
MAX_ROUNDS(5)的轮次,故回路的每一轮都在处理窗口内。N_COMMENTSjq 与反馈渲染的 "Inline comments" jq 不再带**[Suggestion]**+via Qwen Code /review排除;SKILL 的 Optional 分诊改为"有价值即实现、拒绝须逐条记录理由"。npx vitest run scripts/tests/qwen-autofix-workflow.test.js—— 50/50,含新增测试:断言QWEN_SUGGESTION_FILTER与转义过滤正则已从工作流消失、SKILL 禁止静默丢弃 finding。run:块通过bash -n。合并后的行为预期:在 PR #6929(或任一 bot PR)上,新一轮 /review 的 Suggestion 评论晚于最后 eval 标记时,扫描会盯上该 PR,agent 的总结将逐条明确处理每条建议 —— 实现或带理由拒绝 —— 而不是报告"无可处理项"。
证据(前后对比)
测试情况
环境(可选)
在 main 的 worktree 检出上用 vitest 跑契约测试;本地 YAML/bash 静态检查。行为本身运行于 autofix 工作流的
ubuntu-latest。风险与范围
MAX_ROUNDS上限(正是 AGENTS.md 规定的防膨胀护栏)与 SKILL 的范围指引(超范围建议带理由拒绝)约束。**[Critical]**路径不变(从未被过滤)。关联 Issue
#6998 的后续,由 #6929 的实战验证触发(三条 Suggestion finding 对回路不可见)。