fix(autofix): Extend suggestion handling to ten rounds - #8247
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
|
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is a concrete, observable design limitation — takeover-managed PRs currently share the same five-round suggestion window as standard autofix, even though takeover is an explicit delegation for larger work. The before/after is clear and testable. Not a theoretical concern. Direction: aligned. Extending the suggestion window for explicitly delegated work is a reasonable, scoped enhancement to the autofix workflow. No CHANGELOG reference needed for internal CI tooling. Size: not applicable — no core module paths touched. Two files changed (workflow + test), 29 additions / 19 deletions. Approach: the scope feels right. One new env var, one runtime selection based on the live takeover label, and consistent use across all three consumption sites (Critical-only activation, feedback budget census, deferred messaging). Every edit serves the stated goal — no unrelated changes. Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个具体、可观测的设计局限——takeover 管理的 PR 目前与普通 autofix 共享相同的五轮 Suggestion 窗口，但 takeover 是针对较大工作的显式委托。修改前后行为清晰且可测试，不是理论性问题。 方向：对齐。为显式委托的工作扩展 Suggestion 窗口是对 autofix 工作流合理且有限度的增强。内部 CI 工具无需 CHANGELOG 引用。 规模：不适用——未触及核心模块路径。两个文件变更（workflow + 测试），29 行新增 / 19 行删除。 方案：范围合理。一个新环境变量、一个基于实时 takeover 标签的运行时选择、三个消费点（Critical-only 启用、反馈预算统计、延后提示）的一致使用。每处改动都服务于既定目标——无无关改动。 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code ReviewIndependent proposal: given the goal "takeover gets ten suggestion-capable rounds, standard stays at five", I would add a second env var for the takeover threshold, select between them at runtime based on the live takeover label, and use the effective value consistently at every site that compares against the round threshold. Update the test to exercise both paths. Comparison with the diff: the PR does exactly this. The implementation matches the independent proposal one-to-one:
Test coverage: the renamed test exercises both standard ( No critical blockers. No convention violations. Clean, minimal change. CI Test EvidenceFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The ubuntu unit suite is still running. macOS/Windows tests and integration tests were skipped (likely path-filtered — this PR only touches Not verified: no live GitHub Actions takeover run was performed (author's note, not independently re-run). The change is in workflow bash logic; the unit test extracts and executes the relevant block directly, which is the appropriate verification level for this kind of change. 中文说明代码审查独立方案： 给定目标"takeover 获得十轮 Suggestion 窗口，标准保持五轮"，我会添加第二个环境变量作为 takeover 阈值，在运行时根据实时 takeover 标签选择，并在所有与轮次阈值比较的位置一致使用有效值。更新测试以覆盖两条路径。 与 diff 的对比： PR 完全按照此方案实现：
测试覆盖： 重命名后的测试通过提取并执行实际 bash 块来覆盖标准（ 无关键阻塞项。无规范违规。干净、最小的改动。 CI 测试证据ubuntu 单元测试套件仍在运行中。macOS/Windows 测试和集成测试被跳过（可能是路径过滤——此 PR 仅触及 未验证：未执行真实 GitHub Actions takeover 流程（作者说明，非独立重跑）。改动在 workflow bash 逻辑中；单元测试通过提取并执行相关代码块来验证，这对这类改动是适当的验证级别。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a textbook minimal change. The PR identifies a real design limitation (takeover and standard autofix sharing the same suggestion window), solves it with one env var and one runtime selection, updates all three consumption sites consistently, and tests both boundaries by executing the actual bash block. Nothing extra, nothing missing. The Approval deferred until CI lands green on 中文说明置信度：5/5 —— 每个阶段都很干净；毫不犹豫地合并。 这是一个教科书式的最小改动。PR 识别了一个真实的设计局限（takeover 和普通 autofix 共享相同的 Suggestion 窗口），用一个环境变量和一个运行时选择解决，一致更新三个消费点，并通过执行实际 bash 块测试两条边界。没有多余内容，没有遗漏。 批准推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
changed the title
yiliang114
approved these changes
Jul 31, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — straightforward threshold bump from 5 to 10 rounds. Variable rename r5 → critical_after improves clarity. Comments and tests updated consistently. No logic changes beyond the constant.
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
Jul 31, 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! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM！✅
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
approved these changes
Jul 31, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 316ff2d. The threshold move is wired consistently: the single CRITICAL_ONLY_AFTER_ROUND drives the mode gate, the per-author budget census (the $r5 → $critical_after rename is a pure readability change — same value, same two comparison arms), and the bilingual deferral notes interpolate the constant rather than hard-coding a number, so nothing can drift. The test moves the behavioral boundary probe with it (modeAt(9) === 'false', modeAt(10) === 'true' replayed against the extracted shell block, not string-pinned) and adds a not.toContain('TAKEOVER_CRITICAL_ONLY_AFTER_ROUND') assertion that pins the single-threshold design. I swept for stale wording: the remaining "five"/"fifth" hits in the workflow and tests are unrelated (site counts, an API-auth ceiling anecdote, a concurrency-case enumeration), none describe this cutoff.
One thing worth stating explicitly, because it is not obvious from the diff and I verified it arithmetically rather than inferring it: CRITICAL_ONLY_AFTER_ROUND is now exactly equal to MAX_ROUNDS (both 10), which makes Critical-only mode unreachable for standard-managed PRs. The scan's cap gate is [[ "${ROUND}" -ge "${EFF_MAX_ROUNDS}" ]] (qwen-autofix.yml:2259) and it continues before any address job is dispatched, while the mode gate is [[ "${ROUND}" -ge "${CRITICAL_ONLY_AFTER_ROUND}" ]] (:3028) in the address job's prepare step. EFF_MAX_ROUNDS is MAX_ROUNDS unless the takeover label raises it (:2009-2010), so on a standard PR the highest ROUND that ever reaches prepare is 9 and the mode gate can never fire. Critical-only, the deferred-feedback rendering, and the per-author budget from #8071 therefore apply to takeover PRs only (cap 100, mode from round 10).
That matches what the PR body says it wants — ordinary PRs get their full automatic window — so I am not treating it as a defect. But the env comment now reads "From round 11 onward, only Critical findings … may drive code changes", and for a standard PR there is no round 11; the sentence describes takeover behavior exclusively. Worth one clause noting that the cutoff only binds under takeover, since the next reader comparing CRITICAL_ONLY_AFTER_ROUND: '10' against MAX_ROUNDS: '10' two lines up will not otherwise see that the mode is dead for the standard cap — and that also means the #7913 suggestion-ping-pong brake no longer covers ordinary PRs at all, which is a deliberate trade but one the comment should own.
Ran the suite locally at this HEAD: 106/107, and the one failure is classifies permanent API failures terminal… — a spawn-heavy test untouched by this diff (grep-confirmed zero hits in the test-file delta) that hits the 30s suite cap under full-file load and passes standalone in 4.4s. The PR's own target test passes. CI green.
Collaborator
|
Released in v0.21.3. |
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
4 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
This PR extends Suggestion-level review handling from five to ten change-producing rounds for all autofix-managed PRs. After ten rounds, only Critical findings, formally requested changes, failed checks, and base conflicts can drive further code changes. The same threshold is used consistently for Critical-only activation, maintainer feedback-budget accounting, and deferred-feedback messaging.
Why it's needed
The five-round cutoff can defer useful Suggestion-level feedback before an autofix-managed PR has finished iterating. Aligning the cutoff with the existing ten-round standard cap gives ordinary PRs their full automatic window and gives takeover-managed PRs ten rounds of regular feedback handling before Critical-only mode applies.
Reviewer Test Plan
How to verify
Run
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t "switches to Critical-only feedback after ten change rounds"and confirm that round 9 remains suggestion-capable while round 10 activates Critical-only mode. Confirm there is a singleCRITICAL_ONLY_AFTER_ROUNDsetting with value10and no takeover-specific duplicate threshold.Evidence (Before & After)
Before: autofix entered Critical-only mode after five change-producing rounds.
After: autofix enters Critical-only mode after ten change-producing rounds.
Tested on
Environment (optional)
macOS with Node.js 22.22.2. The targeted Vitest test passed. Prettier and
git diff --checkalso passed. The complete workflow test file previously executed all 107 assertions successfully, but its Vitest worker reported anonTaskUpdatetimeout during teardown and exited nonzero.Risk & Scope
Linked Issues
N/A
中文说明
此 PR 的改动
此 PR 将所有 autofix 管理 PR 对 Suggestion 级别评审反馈的处理窗口从五个产生改动的轮次扩展到十轮。十轮之后，只有 Critical 发现、正式的 Request changes、失败检查和 base 冲突可以继续驱动代码修改。Critical-only 模式启用、维护者反馈预算统计以及延后反馈提示统一使用同一个阈值。
为什么需要此改动
五轮截止可能在 autofix 管理的 PR 完成迭代前就延后有价值的 Suggestion 级别反馈。将截止阈值与现有普通模式十轮上限对齐，可让普通 PR 使用完整的自动处理窗口，同时让 takeover 管理的 PR 在进入 Critical-only 模式前拥有十轮常规反馈处理机会。
Reviewer Test Plan
如何验证
运行
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js -t "switches to Critical-only feedback after ten change rounds"，确认第 9 轮后仍可处理 Suggestion，而第 10 轮启用 Critical-only 模式。确认仅保留一个值为10的CRITICAL_ONLY_AFTER_ROUND设置，不存在 takeover 专属的重复阈值。证据（修改前后）
修改前：autofix 在五个产生改动的轮次后进入 Critical-only 模式。
修改后：autofix 在十个产生改动的轮次后进入 Critical-only 模式。
测试平台
环境（可选）
macOS，Node.js 22.22.2。定向 Vitest 测试通过，Prettier 与
git diff --check也通过。完整 workflow 测试文件此前的 107 项断言均成功执行，但 Vitest worker 在收尾时报告onTaskUpdate超时，因此进程以非零状态退出。风险与范围
关联 Issue
N/A