fix(ci): classify mixed AutoFix feedback per item - #8359
Closed
yiliang114 wants to merge 1 commit into
Closed
Conversation
github-actions
Bot
added
the
review/self-reported
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. Issue #8358 links the actual failure on PR #8301 — a trusted review carried an overall Direction: aligned. This is qwen-code's own AutoFix automation misclassifying feedback, squarely in CI/CD tooling (the issue carries Size: not applicable — no core paths. The change is a skill-policy doc ( Approach: the scope feels right, and it's genuinely minimal. Since AutoFix is an agent driven by the SKILL.md instructions, the correct lever is the instruction prose: separate merge-readiness (whole review) from actionability (per item), keep a reproduced correctness defect Required, and gate the Risk: no elevated risk signals — none of the changed files match the high-revert paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，非理论性。Issue #8358 链接了 PR #8301 上的真实失败——一条受信评审带有整体 方向：对齐。这是 qwen-code 自身 AutoFix 自动化对反馈的误分类，属于 CI/CD 工具范畴（issue 带有 规模：不适用——未触及核心路径。改动为一个 skill 策略文档（ 方案：范围合理，且确实最小。由于 AutoFix 是由 SKILL.md 指令驱动的 agent，正确的杠杆就是指令文本：把合并就绪（整个评审）与可执行性（逐项）分开，让已复现的正确性缺陷保持 Required，并把 风险：无升级风险信号——改动文件均未命中高回滚路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewI wrote my independent proposal before reading the diff: for an agent driven by SKILL.md instructions, the fix belongs in the instruction prose — tell the address-review agent to classify each feedback item independently of the review's overall verdict, keep a reproduced correctness defect Required, and only allow a no-change outcome once every actionable point has a disposition — plus a test that pins the new wording. The PR does exactly this, and I didn't find a simpler path. The two SKILL.md edits land in the right places: the classification paragraph sits right under "Classify every feedback point:", and the tightened precondition is on the TestingThis run is unattended CI, so per the gate rules I did not build or run any PR code; the evidence below is the PR's own CI, read via the API. The contract test the PR adds runs inside the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 What this evidence does and doesn't show: the contract test pins the policy prose — the actual artifact this PR changes — and fails if any of the four guarantees is removed. It does not, and cannot, prove the model obeys the new instruction in a live takeover round; the PR is upfront that a deployed round is needed for that post-merge E2E confirmation. I'm not naming a 中文说明代码审查我在读 diff 之前先写了独立方案：对于由 SKILL.md 指令驱动的 agent，修复应放在指令文本里——让 address-review agent 独立于评审整体结论逐项分类反馈，让已复现的正确性缺陷保持 Required，并且只有在每个可执行点都有处置后才允许无需改动的结果——外加一个固定新措辞的测试。PR 正是这么做的，我没有找到更简路径。 两处 SKILL.md 编辑都落在正确位置：分类段落紧跟在 "Classify every feedback point:" 之下，收紧的前置条件则放在 测试本次为无人值守 CI，按 gate 规则我没有构建或运行任何 PR 代码；以下证据是 PR 自身的 CI，通过 API 读取。 PR 新增的契约测试运行在 此证据能证明与不能证明的：契约测试固定了策略文本——也就是本 PR 实际改动的产物——并在四条保证中任一被删除时失败。它不能、也无法证明模型在真实 takeover 轮次中遵守新指令；PR 已坦承这需要一次部署后的轮次来做合并后 E2E 确认。我没有点名 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal, correctly targeted fix; the only caveat is inherent to agent-policy changes (real-world effect confirms on the next takeover round) and CI is still settling. Stepping back: this is exactly the right fix for the bug. AutoFix is an agent that does what its SKILL.md tells it, and the failure in #8358 was an instruction gap — nothing told the address-review agent that an overall The reason this is a 4 rather than a 5 is not a defect in the code — it's that the thing being changed is instruction prose, so the test proves the instructions now say the right thing, not that the model follows them. That residual is acknowledged by the PR and only resolves on a deployed takeover round; it's the nature of this kind of fix, not something to block on. Verdict: approve. CI on the reviewed commit is still running (the ubuntu unit suite that carries the new contract test), so I'm not posting the approval in this run — approval is deferred until CI lands green on the reviewed commit 中文说明置信度：4/5 —— 干净、最小、定位准确的修复；唯一的保留是 agent 策略类改动的固有属性（真实效果在下一轮 takeover 才能确认），且 CI 仍在进行。 退一步看：这正是针对该 bug 的正确修复。AutoFix 是一个按 SKILL.md 指令行事的 agent，#8358 的失败是指令缺口——没有任何内容告诉 address-review agent 整体 之所以是 4 而非 5，不是代码有缺陷——而是被改动的对象是指令文本，因此测试证明的是"指令现在说对了"，而非"模型会照做"。这一残留由 PR 坦承，只有在部署后的 takeover 轮次才能解决；这是此类修复的本质，不构成阻断理由。 结论：approve。被审 commit 上的 CI 仍在运行（承载新契约测试的 ubuntu 单元测试套件），因此本次不发布 approve——approve 推迟到 CI 在被审 commit — Qwen Code · qwen3.8-max-preview Reviewed at |
Contributor
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
approved these changes
Aug 2, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Aug 2, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
Collaborator
Author
|
Withdrawing this after re-checking the full #8301 timeline. The referenced maintainer review explicitly classified the near-miss as a pre-existing, non-blocking follow-up and said |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Separates an AutoFix review's overall merge-readiness verdict from the actionability of each feedback point. A reproduced current correctness defect remains Required even when the reviewer approves the PR or describes it as non-blocking, a follow-up, or not a regression; adjacent diagnostics, comments, tests, and hardening requests are still classified independently. A no-change result is now allowed only after every actionable point has an explicit disposition and no verified Required item remains unresolved.
Why it's needed
On PR #8301, a trusted issue-level review comment reached the takeover agent with an overall approval, one probe-confirmed current correctness defect, and three separate optional follow-ups. AutoFix applied the approval to the whole batch and published a no-action result, so merge readiness incorrectly overrode the per-item correctness classification.
Reviewer Test Plan
How to verify
Run
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t 'requires the address path to run verification'and confirm the contract test passes. Review the address-review outcome rules and confirm that removing any of the four new guarantees—independent decisions, reproduced correctness remaining Required, independent adjacent-item classification, or the no-action precondition—makes the test fail.Evidence (Before & After)
Before: the mixed review feedback reached AutoFix, which then published a batch-level no-action result. After: the focused contract test passes and pins the item-level decision policy; the full workflow test file completed all 109 assertions, although Vitest reported an
onTaskUpdatetimeout during worker teardown.Tested on
Environment (optional)
Node.js 22.22.0 on macOS arm64. Prettier, targeted ESLint, the focused test, and
git diff --checkpass. Local build/typecheck reach existing untouched Ink selection API errors on currentmain; no TypeScript or workflow source is changed by this PR.Risk & Scope
APPROVEDreview-body ingestion are unchanged. A deployed takeover round is required for post-merge E2E verification.Linked Issues
Resolves #8358
Related to #8071 and PR #8301
中文说明
本 PR 的内容
将 AutoFix 评审的整体合并就绪结论与每个反馈点的可执行性分开判断。即使评审者同意合并，或将问题描述为非阻断、后续项、非回归，一个已复现的当前正确性缺陷仍保持为 Required；相邻的诊断、注释、测试和加固请求仍分别分类。只有每个可执行反馈点都有明确处置，且没有已验证的 Required 项仍未解决时，才允许输出无需改动。
为什么需要
在 PR #8301 中，一条受信的 issue-level 评审评论已经进入 takeover agent，其中包含整体同意结论、一个经探针确认的当前正确性缺陷，以及三个独立的可选后续项。AutoFix 将整体同意套用到整批反馈并发布无需改动结果，导致合并就绪错误地覆盖了逐项正确性分类。
审阅者测试方案
如何验证
运行
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t 'requires the address path to run verification'，确认契约测试通过。检查 address-review 的结果规则，并确认删除四项新增保证中的任一项——独立决策、已复现 correctness 仍为 Required、相邻项目独立分类、或 no-action 前置条件——都会使测试失败。前后证据
变更前：混合评审反馈 已进入 AutoFix，随后它发布了整批无需处理的结果。变更后：聚焦契约测试通过并固定逐项决策策略；完整 workflow 测试文件的 109 个断言全部完成，但 Vitest 在 worker teardown 阶段报告
onTaskUpdatetimeout。测试平台
环境
macOS arm64，Node.js 22.22.0。Prettier、目标 ESLint、聚焦测试和
git diff --check均通过。本地 build/typecheck 在当前main的未触碰 Ink selection API 错误处停止；本 PR 不修改 TypeScript 或 workflow 源码。风险与范围
APPROVEDreview body 的采集均保持不变。合并后仍需通过一次部署后的 takeover round 做 E2E 验证。关联 Issue
Resolves #8358
Related to #8071 and PR #8301