docs(autofix): apply Simplicity First when addressing review feedback - #7643
Conversation
Review rounds ratchet code upward — each round tends to ADD (a guard, a
comment, a test) to satisfy a finding, with no counter-pressure to
simplify, so PRs accrete over-defensive, over-commented bloat. AGENTS.md
already forbids this ("Simplicity First ... No error handling for
impossible scenarios", "Comments: Default to none"), but the
address-review flow's "implement each valuable finding" never invokes it.
Wire it in: when addressing findings, apply Simplicity First and the
Comments rule (smallest change, no impossible-case guards, no
restate-the-code comments) and, since rounds only add, ask each round
what the change lets you REMOVE. A suggestion whose only effect is more
defense, config, or narration is a Decline, not an auto-implement. The
pre-commit self-audit now also rejects bloat, not just defects. Points at
AGENTS.md rather than duplicating it; pins the wording in the SKILL test.
|
@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 冲突,直到移除标签或达到轮次上限。移除 |
|
Thanks for the PR! Template — substance is all here: what/why/how, a reviewer test plan, and risk & scope are all covered. (The headings deviate slightly from the template — Problem — this is a policy/docs change, not a Direction — well aligned. It wires the project's own stated top principle ("Simplicity First … the principle we care about most") into the address-review flow, and does so by pointing at AGENTS.md rather than restating it. No auth/sandbox/model/telemetry/public-contract surface touched, so nothing to escalate. Size — not applicable. The change lives in Approach — already at the minimal set: two prose edits (a framing paragraph before the classification list, and an extension to the pre-commit self-audit) plus three test assertions pinning the new wording. No drive-by changes, and it reuses AGENTS.md instead of duplicating. The new "bloat is a Decline reason" framing reinforces the existing "not worth the diff growth" decline reason rather than contradicting it. Moving on to code review. 🔍 中文说明感谢贡献! 模板 —— 实质内容齐全:what/why/how、评审验证计划、风险与范围都覆盖了。(标题与模板略有出入——用了 问题 —— 这是 policy/docs 改动,不是 方向 —— 对齐。它把项目自己声明的最高原则("Simplicity First……我们最在意的原则")接进 address-review 流,而且是指向 AGENTS.md 而非重述。不触及 auth/sandbox/model/telemetry/公共契约面,无需转交。 规模 —— 不适用。改动在 方案 —— 已是最小集:两处 prose 编辑(分类列表前的框架段,以及提交前自审的扩展)加三条 pin 新措辞的断言。无顺手改动,且复用 AGENTS.md 而非复制。新的"膨胀即 Decline 理由"框架强化了已有的"不值得 diff 增长"decline 理由,而非与之矛盾。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewMy independent take before reading the diff: to counter the additive ratchet you'd add a Simplicity-First lens at the two leverage points in The prose integrates cleanly. The new framing paragraph sits before the Required/Optional classification list and reinforces the already-listed "not worth the diff growth" decline reason rather than contradicting it; the self-audit edit extends the existing "no new defect" check to also reject bloat, grammatically and logically consistent with the surrounding text. The three new assertions ( Real-scenario testingBefore/After is N/A here: this changes prose in an agent skill file that the autofix loop reads — it has no interactive-CLI runtime behavior, so a tmux The full test file (101 tests) also passes — the SKILL.md edit doesn't disturb the other skill-reading assertions — and this test file is the only programmatic consumer of 中文说明代码审查读 diff 前我的独立想法:要对抗加法棘轮,就在 prose 衔接自然。新框架段位于 Required/Optional 分类列表之前,强化了已列出的"不值得 diff 增长"decline 理由而非与之矛盾;自审编辑把已有的"无新缺陷"检查扩展为同时拒绝膨胀,与上下文在语法和逻辑上一致。三条新断言( 真实场景测试Before/After 在此 不适用:本 PR 改的是 autofix 循环读取的 agent skill 文件中的 prose——没有交互式 CLI 运行时行为,所以 tmux — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; minimal, well-motivated, and it makes the bot follow a rule the project already says it cares about most. Stepping back: this is exactly the kind of change the gate should let through quickly. The problem is real and observed (review rounds ratchet code upward; #7471 is a concrete case), and the fix applies pressure at the source — the agent's own instructions — by pointing at AGENTS.md's Simplicity First and Comments rules rather than restating them. It's two prose edits and three assertions; there's no simpler version of this, and nothing in the diff is unrelated. The new "bloat is a Decline reason" framing slots into the existing decline-reason list without contradicting it, and the pre-commit self-audit now rejects bloat alongside defects. The approach matched my independent proposal, the assertions pin the exact wording (so it can't be silently softened), the full 101-test file passes, and prettier is clean. If I'm maintaining this in six months I'd thank the author for keeping the bot's output lean. Approving because it's genuinely good, not because I ran out of reasons to object. 中文说明置信度:5/5 —— 每一阶段都干净;最小、动机充分,且让 bot 遵循项目自己声明最在意的规则。 退一步看:这正是 gate 应快速放行的那类改动。问题真实且已观测(评审轮次把代码往上棘轮;#7471 是具体案例),修复在源头施压——agent 自己的指令——通过指向 AGENTS.md 的 Simplicity First 与 Comments 规则而非重述。就是两处 prose 编辑加三条断言;没有更简的版本,diff 中也没有无关内容。新的"膨胀即 Decline 理由"框架无缝并入已有的 decline 理由列表而不矛盾,提交前自审现在与缺陷一并拒绝膨胀。 方案与我的独立提议一致,断言精确 pin 了措辞(使其不能被悄悄软化),整个 101 项测试文件通过,prettier 干净。半年后维护它我会感谢作者让 bot 输出保持精简。批准是因为它确实好,而非因为找不到反对理由。 — Qwen Code · qwen3.8-max-preview 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.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Well-motivated addition — wires AGENTS.md Simplicity First into the address-review flow to counter the additive ratchet of review rounds. The "what can I REMOVE?" counter-pressure and "nit = decline, not auto-implement" rule are practical improvements. Test pins the key phrases.
— qwen3.8-max-preview via Qwen Code /review
What this PR does
Wires AGENTS.md's Simplicity First and Comments rules into the autofix
address-reviewflow, so the bot stops accreting over-defensive, over-commented bloat as it addresses review feedback round after round.Why
Review rounds ratchet code upward: every round tends to add — a guard, a comment, a test — to satisfy a finding, with no counter-pressure to simplify. Over many rounds a PR grows over-defensive and over-commented (e.g. #7471 went 1972 → 2690 lines). The verification gate rewards "builds/tests/lints pass" — an over-defended, over-commented version passes just as well — so the only place simplicity pressure can come from is the agent's own instructions.
AGENTS.md already states the principle, and strongly ("the principle we care about most"): "Minimum code that solves the problem … No error handling for impossible scenarios … If you write 200 lines and it could be 50, rewrite it" and "Comments: Default to none. Add only when why is non-obvious." The gap is that the address-review flow's "implement each valuable finding, decline only with reason" never invokes it — so the additive bias runs unchecked.
How
Two small prose edits to
Mode: address-review— pointers to the existing AGENTS.md rules, not a re-statement of them:Deliberately minimal: it points at AGENTS.md rather than duplicating it, and adds no gate or metric (a diff-size cap or "complexity linter" would be blunt and unreliable — human/AI review stays the backstop; this just applies the principle at the source).
Reviewer Test Plan
npx vitest run scripts/tests/qwen-autofix-workflow.test.js -t "requires the address path"— the address-path SKILL test now also pins the simplicity wording (Simplicity First,added no bloat,never a reason to bloat the code). Passes.added no bloatfrom the SKILL turns that test red.Risk & Scope
address-reviewbut in different lines; the assertions here live in therequires the address pathtest, not the policy test docs(autofix): let the agent escalate a maintainer's decision, not decide it #7636 edits.中文说明
本 PR 做了什么
把 AGENTS.md 的 Simplicity First 与 Comments 纪律 接进 autofix 的
address-review流,让 bot 在一轮轮处理评审反馈时,不再堆积过度防御、过度注释的膨胀。为什么
评审轮次是只增不减的棘轮:每轮为满足一条 finding 而加东西(guard、注释、测试),却没有简化的反压力。多轮下来 PR 越来越过度防御、过度注释(如 #7471 从 1972→2690 行)。验证 gate 只奖励"build/test/lint 过"——过度防御、过度注释的版本照样过——所以简化压力只能来自 agent 自己的指令。
AGENTS.md 本就写了这原则,且很强("我们最在意的原则"):"解决问题的最少代码……不为不可能的情况做错误处理……200 行能写成 50 行就重写"、"注释:默认没有,只在 why 不显然时加"。缺口在于 address-review 的 "实现每条有价值的 finding、只能带理由 decline" 从没调用它 —— 加法偏置无人约束。
怎么做
对
Mode: address-review两处小 prose 改动 —— 指向现有 AGENTS.md 规则,不重述:刻意最小:指向 AGENTS.md 而非复制;不加任何 gate/度量(diff 行数上限或"复杂度 linter"太钝、不可靠 —— 人工/AI 评审仍是兜底,本 PR 只是在源头应用原则)。
评审验证
-t "requires the address path"—— address-path SKILL 测试现在也 pin 了 simplicity 措辞(Simplicity First、added no bloat、never a reason to bloat the code)。通过。added no bloat→ 该测试红。风险与范围
address-review但不同行;本 PR 断言放在requires the address path测试,而非 docs(autofix): let the agent escalate a maintainer's decision, not decide it #7636 改的 policy 测试。