feat(autofix): per-source feedback budget in Critical-only mode - #8071
Conversation
Critical-only mode (after 5 change-producing rounds) classifies feedback lexically: only a literal **[Critical]** tag or a CHANGES_REQUESTED review survives; everything else is deferred before the agent reads a word of it. That rule was built to stop the review bot's suggestion ping-pong, but it catches maintainers too. Observed four times in two days (#8037, #7944, #7885, #7799): a maintainer's review with explicit merge-blocking findings — #8037's said 'I'd fix before merge' on a correctness bug and a security-adjacent one — was wholesale-deferred as one 'non-Critical item', and the bot then reported 'No Critical feedback. The Issue-level comments sections are empty', which was lexically true and substantively false. The bot's own advertised definition ('correctness bugs, security issues, or formally requested changes') is exactly what the deferred comments contained; the agent that could have applied that definition never saw them. The lexical test now applies exclusively to the review bot's output: - All three actionable filters (reviews, inline, issue-level) pass anything not authored by the review bot straight through in Critical-only mode — the agent judges maintainer feedback on content, as everywhere else. - All three deferred-list builders keep only review-bot items, so a maintainer comment can never appear as an 'audit record'. - The deferral note says what is actually deferred (the automated reviewer's non-Critical suggestions), states that maintainer feedback is never deferred, and names the exit (@qwen-code /retry opens a fresh counting window). - SKILL.md's Critical-only policy now marks everything rendered in the actionable sections as in scope, so the agent does not re-refuse what the filter passed through. Behavioral test updated both ways: maintainer comments/reviews stay actionable in Critical-only mode across all three sources, bot suggestions still defer, and structural pins hold the bypass and the bot-only select in all six filters.
|
|
|
Thanks for the PR! Template: the headings differ from the template ( Problem: observed bug with concrete evidence — four PRs (#8037, #7944, #7885, #7799) where maintainer feedback was lexically classified as non-Critical and deferred wholesale, with the bot then reporting "No Critical feedback." The bot's own Critical definition (correctness bugs, security issues, formally requested changes) is exactly what those comments contained, but the agent never saw them because the deterministic filter ran first. Direction: aligned. The Critical-only mode exists to stop review-bot suggestion ping-pong (#7913), not to filter out maintainer judgment. Scoping the lexical test to the bot's own output preserves the anti-churn purpose while fixing the false positive. Size: not applicable — no core module paths touched. Approach: the scope is right. Three actionable filters get a Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板：标题与模板不同（ 问题：已观测到的 bug，有具体证据——四个 PR（#8037、#7944、#7885、#7799）中维护者反馈被词法分类为非 Critical 并整体延后，bot 随后报告"No Critical feedback"。bot 自己定义的 Critical（正确性 bug、安全问题、正式修改请求）恰恰就是那些评论的内容，但 agent 根本没看到，因为确定性过滤器先于 agent 运行。 方向：对齐。Critical-only 模式的目的是阻止 review bot 的建议乒乓（#7913），不是过滤维护者的判断。将词法测试限定为 bot 自身产出，既保留了反乒乓目的，又修复了误判。 规模：不适用——未触及核心模块路径。 方案：范围合理。三个 actionable 过滤器加 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I would scope the lexical Comparison: the PR does exactly this. The approach matches my independent proposal. The trust boundary is maintained: the actionable filters still run the The structural pins in the tests (counting occurrences of No correctness issues, no security concerns, no convention violations. TestingFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The macOS/Windows tests and integration tests are skipped (expected — this PR touches workflow YAML, skill docs, and test scripts, not package source). The ubuntu test is still running. The author reports 102/102 + 14/14 tests passing locally with The tests in this PR are the behavioral verification: they extract the real jq filters from the workflow YAML and execute them against fixture data with 中文说明代码审查独立方案： 我会将词法 对比： PR 完全这样做了，与我的独立方案一致。 信任边界保持不变：actionable 过滤器在 Critical-only 条件之前仍运行 测试中的结构 pin（计算 actionable 过滤器中 无正确性问题、无安全隐患、无规范违反。 测试ubuntu 测试仍在运行中。macOS/Windows 测试和集成测试被跳过（符合预期——本 PR 仅触及工作流 YAML、技能文档和测试脚本，不涉及包源码）。作者报告本地 102/102 + 14/14 测试通过， 本 PR 的测试即为行为验证：它们从工作流 YAML 中提取真实 jq 过滤器，对 fixture 数据在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a well-motivated, minimal fix from a maintainer who observed the bug firsthand across four PRs in two days. The Critical-only mode's lexical filter was doing exactly what it was designed to do — stop bot suggestion ping-pong — but the design didn't account for the fact that the same filter also catches maintainer feedback. The fix scopes the lexical test to the bot's output only, which is the right boundary: the trust filter already ensures only trusted authors reach this point, so "not the bot" means "trusted human whose judgment the agent should evaluate on content." The implementation is six lines of jq (three bypass additions, three deferred-builder narrowings) plus documentation and test updates. Every change is necessary. The tests extract and execute the real jq filters, not mocks, and the structural pins guard against regression. No scope creep, no drive-by changes. Approval deferred until CI lands green on 中文说明置信度：5/5 —— 每个阶段都干净，毫不犹豫可以合并。 这是一个动机充分、改动最小的修复，来自一位在两天内四个 PR 中亲眼观察到 bug 的维护者。Critical-only 模式的词法过滤器完全按设计运行——阻止 bot 建议乒乓——但设计没有考虑到同一过滤器也会拦截维护者反馈。修复将词法测试限定为仅作用于 bot 产出，这是正确的边界：信任过滤器已确保只有受信作者到达此处，因此"非 bot"意味着"受信人类，其判断应由 agent 按内容评估"。 实现是六行 jq（三处旁路添加、三处延后构建器收窄）加文档和测试更新。每处改动都是必要的。测试提取并执行真实 jq 过滤器而非 mock，结构 pin 防止回归。无范围蔓延，无顺手改动。 审批延迟至 CI 在 — Qwen Code · qwen3.8-max-preview 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. |
Follow-up to the author-based split, prompted by the obvious counterexample: a human account can host an automated reviewer loop with the exact regeneration property the review bot has — feedback re-generated after every push at zero marginal cost — so 'not the bot' cannot mean 'never throttled'. An account is an accountability unit, not a throttle; the brake has to key on measured regeneration. Unified model: once Critical-only engages, every source has a bounded budget of untagged feedback batches per counting window. The review bot's budget is zero (all deferred, as before). A human's is CRITICAL_ONLY_HUMAN_BATCHES (2) CONSUMED batches: feedback items are bucketed into the (prev marker ts, marker ts] span that evaluated them, only spans from Critical-only rounds count, and an author needs K distinct consumed spans before their new untagged feedback defers. Fresh unevaluated feedback never counts against its own author, and the census is window-scoped, so /retry resets the budget with the window. The observed cases (#8037/#7944/#7885/#7799 — one or two late verification reports each) stay fully served under K=2; a looped reviewer is throttled after 5+K driven rounds instead of grinding to the 100-round cap. Past the budget, continuing requires one conscious act — **[Critical]**, a Request changes review, or /retry — which is precisely what separates intent from automation. Over-budget authors are named in the deferral note with those exact escapes. Tests: the six filter replays gain over-budget cases both ways (the tagged/CR escapes survive even over budget), and the budget census itself is replayed over fixture files — two consumed critical-tail batches list the author; one batch, pre-Critical batches, unconsumed feedback, untrusted authors, and command comments never count.
wenshao
changed the title
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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 crashed before it could evaluate the feedback (attempt 1/100) — it will retry on the next scan. AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why). Run log: https://github.com/QwenLM/qwen-code/actions/runs/30508501686 🧠 Handled by Qwen Code · model/模型 |
| | {lo: (if $i == 0 then "" else ($ms[$i - 1].ts) end), hi: .ts}) ]) as $spans | ||
| | ([ ($rv | add)[] | {at: (.submitted_at // ""), login: (.user.login // ""), assoc: (.author_association // "")} ] | ||
| + [ ($rc | add)[] | {at: (.created_at // ""), login: (.user.login // ""), assoc: (.author_association // "")} ] | ||
| + [ ($ic | add)[] | select((.body // "") | test("^\\s*@qwen-code /") | not) |
There was a problem hiding this comment.
[Suggestion] The census feedback collection has no state / **[Critical]** / reply-to-Critical filter, so tagged feedback counts toward the per-author budget — but the env comment (lines 122-130) describes the budget as "untagged feedback batches" and names **[Critical]**, a Request changes review, and /retry as the escapes that cut through "past it". — Concrete cost: reproduced against the real census jq — a maintainer who submits only a CHANGES_REQUESTED review (consumed span 1) and a **[Critical]** comment (consumed span 2), with zero untagged feedback, is flagged over-budget (["maintainer"]), so their next plain comment defers even though no untagged batch was ever consumed. Impact is bounded — tagged items still cut through via the actionable-filter escapes, and the maintainer can tag the plain comment to lift it — but it contradicts the documented design.
Fix — either narrow the census collection to untagged items (mirror the escapes the actionable filters already recognise), e.g. add to the $rv branch | select((.state // "") == "COMMENTED") | select(((.body // "") | contains("**[Critical]**")) | not) plus analogous exclusions on $rc/$ic, and a two-span fixture asserting a tagged-only author is NOT listed; or update the env comment and SKILL.md to state that all consumed batches (tagged or not) count.
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Valid finding — the census does count tagged items (a **[Critical]** comment or a CHANGES_REQUESTED review) toward the per-author budget, while the CRITICAL_ONLY_HUMAN_BATCHES env comment describes the budget as untagged batches with those tagged items as the escapes that cut through "past it". I confirmed the reproduction against the census jq. I'm leaving this thread open because the fix is a semantic call about what the brake should count, and the two resolutions have different behavioral consequences:
- Narrow the census to untagged items (mirror the escapes the actionable/deferred filters already recognise: drop
CHANGES_REQUESTED/**[Critical]**reviews, Critical comments, replies-to-Critical, and CR-associated inline comments from the collected batches), plus a two-span fixture asserting a tagged-only author is NOT listed. This matches the documented design and this PR's intent to protect maintainer feedback from over-eager deferral, but it is a non-trivial change to a carefully-tested jq program and carries census regression risk. - Update the env comment and SKILL.md to state that all consumed batches (tagged or not) count. Low-risk, but it redefines a deliberately-worded design statement; impact is bounded either way since tagged items still cut through the actionable-filter escapes and a maintainer can tag a plain comment to lift it.
My recommendation is option 1 (it matches the documented intent), but only with maintainer eyes on the jq change. Which semantic do you want the budget to implement?
中文说明
发现成立——census 确实会把带标签的条目（**[Critical]** 评论或 CHANGES_REQUESTED review）计入每位作者的预算，而 CRITICAL_ONLY_HUMAN_BATCHES 的环境变量注释把预算描述为未带标签的批次，并将这些带标签条目作为「超过预算后」仍可穿透的逃生口。我已对照 census 的 jq 复现了这一点。我把这个线程保持开放，因为修复方式是一个关于「刹车应统计什么」的语义抉择，两种方案有不同的行为后果：
- 将 census 收窄到未带标签条目（镜像 actionable/deferred 过滤器已识别的逃生口：从统计的批次中剔除
CHANGES_REQUESTED/**[Critical]**review、Critical 评论、对 Critical 的回复，以及关联 CR 的行内评论），并新增一个跨两个 span 的断言用例，验证仅提交带标签反馈的作者不会被列入。这符合文档化的设计，也符合本 PR 保护维护者反馈不被过度延后的意图，但它是对于一个经过充分测试的 jq 程序的非平凡改动，带有 census 回归风险。 - 更新环境变量注释与 SKILL.md，声明所有被消费的批次（无论是否带标签）都计数。风险低，但这会重新定义一段刻意措辞的设计陈述；无论哪种方案影响都有限，因为带标签条目仍能通过 actionable 过滤器的逃生口穿透，维护者也可以给普通评论加标签来解除限制。
我的推荐是方案 1（它符合文档意图），但希望由维护者审视该 jq 改动。你希望预算实现哪种语义？
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Addressed three of the four findings in code and escalated the fourth (a semantic call) to a maintainer. Findings
Verification
中文说明本轮以代码方式处理了四项发现中的三项，并将第四项（一个语义层面的抉择）升级给维护者决定。 发现
验证
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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round summaryFeedback: census window-isolation guard untested (rc:3680384414)[Suggestion] The census fixture markers all carried the same window key ( Decision: Implemented. Changes:
No conflict resolution needed ( Verification
中文说明本轮总结反馈：census 窗口隔离守卫未被测试覆盖 (rc:3680384414)[建议] census 测试夹具中的所有 marker 都使用相同的窗口键（ 决定： 已实现。 变更：
无需冲突解决（ 验证
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/模型 |
|
@qwencodeqaq review this pr |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
LGTM. The per-source budget model is the right fix for the observed problem (maintainer "fix X before merge" silently deferred on #8037/#7944 etc.).
What works well:
- Keying the brake on consumed batches rather than identity is sound — it lets a human's first K feedback rounds through unconditionally while still capping an automated loop running under a human login.
- The census jq is complex but well-structured: window-scoped, span-based counting means
/retrycleanly resets everything, and fresh (not-yet-evaluated) feedback never counts against its author. - Escape hatches (
**[Critical]**, CHANGES_REQUESTED,/retry) are preserved at every filter site — no path where a human is truly locked out. - The
bash -nparse test is a good addition; the quoting idiom in the deferred-feedback echo is exactly the kind of thing that silently breaks at runtime while jq-filter tests stay green. - Test coverage is thorough: stale-window isolation, multi-source census (ic + rv + rc), and all five filter sites exercised with both
over=[]andover=['maintainer'].
Minor observations (non-blocking):
- The census jq's
2> /dev/null || echo '[]'swallows all stderr — if the jq program itself has a bug after a future edit, it'll silently produce an empty budget rather than failing loudly. Acceptable for now since the test suite exercises the exact program text, but worth noting. - The deferred-feedback.md prose is getting long; if it grows further, consider splitting the policy explanation into a linked doc.
No blockers. Ship it.
|
Done — approved PR #8071. Summary: The PR adds a per-source feedback budget to the autofix workflow's Critical-only mode. Previously, all non-Critical feedback (including maintainer instructions like "fix X before merge") was silently deferred after round 5. Now, only the bot's suggestions are always deferred; human feedback gets a budget of K=2 consumed batches per window before deferring. Escape hatches (Critical tag, Request Changes, The implementation is solid — well-tested (105 tests), window-scoped census logic, and a new |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action takenNo code changes were made this round. The feedback newer than the last
The PR is approved with no blockers and no outstanding change requests, so 中文说明Autofix 审查轮次 —— 未采取任何操作本轮未做任何代码改动。自上次评估以来的新反馈中没有任何可处理的内容：
该 PR 已获批准、没有任何阻塞项，也没有未处理的改动请求，因此没有需要修复、推迟或升级的内容。分支保持原样，未做改动。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 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.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：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
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summaryBoth inline suggestions from the automated reviewer are addressed in code. No conflict ( Feedback points
Conflict notesNone — Verification
No settings source changed, so 中文说明Autofix 审查轮次总结自动化审查者的两条行内建议均已在代码中处理。无冲突（ 反馈点
冲突说明无—— 验证
未改动任何 settings 源文件，因此无需运行 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
qwen-code-ci-bot
left a comment
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
OverviewFixes a real and well-evidenced bug: in Critical-only mode (round ≥ 5) the deferral filters keyed on Three parts:
The span mechanism is genuinely careful. I traced the Verification: I replayed the census with my own fixtures and ran the full file — 105/105 pass (3 needed Findings1. The census counts feedback that is never deferrable, re-creating the very bug this PR fixes 🔴
Replaying the real census block verbatim against fixtures where an author posted nothing but protected feedback: Concrete failure: a maintainer tags This contradicts the design as written — Mirroring the deferral predicates fixes it. Verified against both fixture sets: - | ([ ($rv | add)[] | {at: (.submitted_at // ""), login: ...
- + [ ($rc | add)[] | {at: (.created_at // ""), login: ...
+ | ([ ($rv | add)[] | select((.state // "") == "COMMENTED")
+ | select(((.body // "") | contains("**[Critical]**")) | not)
+ | {at: (.submitted_at // ""), login: ...
+ + [ ($rc | add)[]
+ | select(((.body // "") | contains("**[Critical]**")) | not)
+ | {at: (.created_at // ""), login: ...
+ [ ($ic | add)[] | select((.body // "") | test("^\\s*@qwen-code /") | not)
+ | select(((.body // "") | contains("**[Critical]**")) | not)Two related items in the same family, worth folding in: inline replies rooted at a Critical comment and inline comments under a Please extend the census test with a Critical-only and a CR-only author asserting they stay absent — the current fixtures use only untagged bodies (the comment at the 2. Empty "Deferred non-Critical feedback" section on most rounds ≥ 5 🟡
Pre-existing, but this PR makes it the common case: human items no longer populate the list, so any round ≥ 5 with no new review-bot output now posts a header, a ~600-char explanatory paragraph, and a 3. Takeover interaction worth a decision 🟡
Strictly better than Smaller notes
AssessmentThe core direction is right and the span-consumption model is thoughtfully built — the round-parity handling in particular is subtle and correct. Finding 1 should be fixed before merge: as written, the census re-introduces the silent-deferral-of-maintainer-feedback bug for exactly the maintainers who follow the documented |
Local real-environment verification —
|
| 版本 | run block 能否解析 | 维护者未打标反馈 | 评审 bot 建议 |
|---|---|---|---|
main（PR 前） |
是 | DEFERRED（即本 PR 要修的 bug） | DEFERRED |
508a598d（PR 中间态） |
否 — 语法错误 | n/a | n/a |
7d2c5380（PR head） |
是 | ACTIONABLE ✅ | DEFERRED ✅ |
这正是 PR 描述所声称的行为：Critical-only 模式下，维护者未打 **[Critical]** 标记的反馈重新变为可执行，而评审 bot 的非 Critical 建议仍然延后。
15 个场景全部通过（要点）
预算刹车按实测再生频度而非身份生效，且各条逃生通道都有效：**[Critical]** 标记、Request changes 评审、/retry 开新窗口均能穿透预算；窗口隔离、哨兵时间戳、命令型评论排除、K-1 边界、未受信任作者、census 失败向开放侧降级（维护者反馈仍可执行，且 jq 报错会显示在日志中而非被吞掉）——逐项均验证通过。
变异测试（8 个变异体）
第一轮：6 个被捕获，2 个存活。两个存活的都是 census 中的防护判断，PR 自带的 105 条测试无法区分。补 2 个 fixture 场景后，8 个全部被捕获。
发现的问题
- 两处 census 防护缺少测试覆盖（非阻塞）：哨兵时间戳过滤
.ts != "9999-…"与评审 bot 排除.login != $rb。两者在当前代码中都正确，但改坏了 CI 不会红。后者仅在评审 bot 账号带有受信任 association 时才承担作用——否则assoc | IN($trust[])已经把它挡掉了。 - PR 中间提交
508a598d会让每一次 autofix 运行直接失败（已在本 PR 内修复）：延后反馈那行 echo 有 bash 语法错误。ddeb14566已修复并新增bash -n守护测试。我验证了该守护测试在508a598d上失败、在 head 上通过——守护是真的有效。需要指出的是，评审 bot 记录的「无正确性问题」结论是针对601e1a4（早于该问题引入）作出的。 - 一处潜在但当前不可达的多计（仅作稳健性备注）：首个计数区间的下界是空串（下方无界）。若某个窗口的第一个 marker 就已处于 Critical-only 区间，该作者窗口之前的所有历史评论都会落进这一个区间。实测确认：轮次计数是按窗口重置的，因此窗口内第一个 marker 不可能落在 Critical-only 区间——现实场景（下表 B）正确判定为未超预算。若将来轮次重置的不变量发生变化，这里会开始多计。
未覆盖范围
- 未在真实 GitHub Actions runner 上跑完整 workflow（无
act/Docker）；GitHub API 返回值为 fixture。 - 未验证 agent 侧对
SKILL.md文案改动的实际反应（需要真实模型运行）。 .github/scripts/check-autofix-contracts.sh在我的 worktree 中因node_modules为符号链接而报错（workspace 解析失败）；在正常安装下npm run check-i18n通过，且本 PR 未触及 i18n 或tool-names.ts，故该门禁与本 PR 无实质关系。
Central claim — A/B/C on one fixture
Same fixture in all three cells: round 5 (Critical-only ON), one review-bot suggestion, one maintainer comment in plain English with no **[Critical]** tag.
| revision | run block parses (bash -n) |
maintainer's untagged msg | review-bot msg |
|---|---|---|---|
main (pre-PR) |
yes | DEFERRED ← the reported bug | DEFERRED |
508a598d (mid-PR) |
NO — syntax error | n/a | n/a |
7d2c5380 (PR head) |
yes | ACTIONABLE ✅ | DEFERRED ✅ |
That is exactly the claim in the PR description: in Critical-only mode a maintainer's untagged feedback becomes actionable again, while the review bot's non-Critical suggestions still defer.
Behaviour matrix — 15 scenarios, 44 checks, all green
The round predicate is the subtle part and it checks out: acted=true writes round = ROUND + 1 and acted=false writes round = ROUND, so (acted && round > 5) || (!acted && round >= 5) selects exactly the markers whose run had CRITICAL_ONLY=true. Verified against the marker writers at lines 3685 / 3706 / 4270.
| # | scenario | result |
|---|---|---|
| S1 | round 4: census skipped, everything actionable | ✅ |
| S2 | under budget: bot deferred, maintainer actionable | ✅ |
| S3 | at budget (2 consumed spans): untagged defers + author named | ✅ |
| S4 | over budget + **[Critical]** → cuts through |
✅ |
| S5 | over budget + Request changes → cuts through |
✅ |
| S6 | window isolation: stale-window batches don't count | ✅ |
| S7 | only Critical-only rounds consume budget | ✅ |
| S8 | boundary: K−1 consumed spans is still under budget | ✅ |
| S9 | @qwen-code /… commands are not feedback batches |
✅ |
| S10 | sentinel-ts markers open no spans | ✅ |
| S11 | review-bot budget is zero: never in census, always deferred | ✅ |
| S12 | untrusted (NONE) author out of scope for the census |
✅ |
| S13 | review-bot exclusion holds even if the bot is a MEMBER |
✅ |
| S14 | sentinel-ts guard changes the span count (discriminating) | ✅ |
| S15 | census failure fails OPEN — human feedback stays actionable, jq error surfaced | ✅ |
S15 confirms both halves of 7d2c5380's claim: dropping 2> /dev/null means a census jq error is now visible in the log, and the || echo '[]' fallback degrades toward reading maintainer feedback rather than silently deferring it. That is the right direction to fail.
Fresh, not-yet-evaluated feedback never counts against its own author (S2), and the whole census is window-scoped so /retry resets the budget along with the window (S6).
Mutation matrix — do the tests actually protect the census?
I broke one invariant at a time in the real workflow file and asked two independent oracles whether the break was caught: (A) this PR's own 105-test suite, (B) the 15-scenario harness.
| id | broken invariant | round 1 | round 2 |
|---|---|---|---|
| M1 | budget off-by-one (>= K → > K) |
suite+harness | suite+harness |
| M2 | window isolation dropped | suite+harness | suite+harness |
| M3 | sentinel-ts guard dropped from census | SURVIVED | harness |
| M4 | acted round == threshold also consumes |
suite | suite |
| M5 | census counts @qwen-code / commands |
suite+harness | suite+harness |
| M6 | review-bot exclusion dropped from census | SURVIVED | harness |
| M7 | span bucketing dropped (all feedback counts) | suite+harness | suite+harness |
| M8 | budget never bites | suite+harness | suite+harness |
Round 1: 6 caught, 2 survived. Round 2 adds S13/S14 and catches all 8.
Findings
1 · Two census guards have no test coverage — non-blocking, both correct as shipped.
select(.ts != "9999-12-31T23:59:59Z") (M3) and .login != $rb (M6) can both be deleted with the full 105-test suite staying green. Two fixture scenarios close the gap:
- The review-bot exclusion only becomes load-bearing when the bot account carries a trusted association — otherwise
select(.assoc | IN($trust[]))already screens it out, which is why M6 survives. Giveqwen-code-ci-botMEMBERand, without.login != $rb, the bot gets named in the human-facing callout and advised to "tag**[Critical]**" — advice that means nothing to a bot. Defence-in-depth worth keeping, and worth pinning. - Dropping the sentinel guard makes the census under-count (the
(T_prev, 9999]span swallows later feedback and collapses two spans into one), so the failure direction is safe — but it does change outcomes, so it is not decoration.
2 · Mid-PR commit 508a598d would have broken every autofix run — already fixed inside this PR.
The deferred-feedback echo had a genuine bash syntax error; the run block does not parse, so the step would have aborted on every run while the jq-filter tests stayed green. ddeb14566 fixed it and added the bash -n guard. I confirmed the guard has teeth: it fails on 508a598d (exit 2) and passes on head. Flagging it only because the review bot's "no correctness issues" assessment was recorded against 601e1a4, which predates the break — the guard test is the durable answer and it is in this PR.
3 · Latent-but-unreachable over-count — robustness note only, no action needed.
The first counted span uses lo = "", unbounded below. If a window's earliest marker were already in Critical-only territory, every pre-window comment by an author would bucket into that single span, so one genuine batch plus history would read as two. I tried to make it fire and could not: round counting is window-scoped, so the first marker in a window is never past the threshold. Measured directly — hypothetical window opening in Critical-only → ["maintainer"]; realistic window with the round reset → []. Correct today, and dependent on the round-reset invariant staying true.
Repo suite
scripts/tests/qwen-autofix-workflow.test.js — 105/105 pass on head. The Unhandled Error visible in the capture is a vitest worker onTaskUpdate RPC timeout under my sandboxed worktree, not a test failure; the Read-only file system line is the same environment. Both are artifacts of my setup, not the PR.
Rendered output, as the agent will see it
Real bytes from deferred-feedback.md at round 5 with the author at budget — note the apostrophe in window's, which is precisely what the '"'"' quoting fix had to get right:
- @maintainer is at this window's regular-feedback budget — to continue: tag
**[Critical]**, submit a Request changes review, or comment `@qwen-code /retry`
for a fresh window. / @maintainer 本窗口常规反馈预算已用完——继续请标
**[Critical]**、提交 Request changes、或评论 `@qwen-code /retry` 开新窗口。
Not covered
- The full workflow was not run on a real GitHub Actions runner (no
act/Docker on this machine); GitHub API responses are fixtures. What was real: the shell, jq, the file I/O, and the jq programs themselves. - No agent-side check that the
SKILL.mdwording change lands as intended — that needs a live model run. .github/scripts/check-autofix-contracts.shcould not complete in my worktree:node_modulesis a symlink, which breaks npm workspace resolution (ajv/dist/2020.js).npm run check-i18npasses in a normal install, and this PR touches no i18n files and notool-names.ts, so the gate is not materially exercised by this diff.- Harness scripts live outside the repo (session scratchpad) and are not proposed for check-in.
🔬 Local maintainer verification · bash 5.3.15 · jq 1.7.1 · head 7d2c5380 · evidence hosted on pr-assets/8071-verify
#8071) The Critical-only per-author budget census counted every trusted review, inline comment, and issue comment, including feedback the deferred renderer would never defer: **[Critical]**-tagged comments, Request changes / APPROVED reviews, inline replies rooted at a Critical comment, and inline comments attached to a Request changes review. A maintainer who followed the documented escape hatches (tag Critical, request changes) thereby spent their own budget and had later untagged feedback silently deferred — the exact bug this PR fixes, re-created one level down. Mirror the three deferred-builder predicates in the census item filter so a batch is counted only when it is actually deferrable. Extend the census replay test with protected authors (Critical-only, Request changes, APPROVED, Critical-rooted replies, Request-changes-review inlines, the review bot as a trusted MEMBER, and a sentinel-ts marker probe) that each carry two consumed-span batches yet must stay absent, so dropping any one exclusion now fails the suite. Also fold bash's stderr into the bash -n guard assertion so a future quoting regression reports the syntax error, not just a non-zero exit.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8071One required correctness fix landed, two suggestions were addressed in the same Required — fixedComment 1 · Finding 1 (🔴 census counts never-deferrable feedback). Fixed. The census item filter now mirrors the three deferred-builder predicates exactly,
The Tests. Extended the census behavioral-replay test ( Suggestions — addressed
Suggestions — deferred / declined (recorded)
Needs a maintainer's decision — left open
Informational — acknowledged, no action
Conflict notes
VerificationCommands actually run this round (the touched behavior is exercised only through
中文说明Autofix 评审轮次总结 — PR #8071本轮落地了一项必需的 correctness 修复，同一提交中处理了两条建议，另有两条 必需项 — 已修复评论 1 · Finding 1（🔴 census 统计了永不可延后的反馈）。 已修复。预算 现在 census 的条目过滤精确镜像了三个延后构造器的判定条件，因此只有真正可延后的
测试。 扩展了 census 行为重放测试（ 建议 — 已处理
建议 — 延后／拒绝（均附理由记录）
需维护者决策 — 保持开放
信息性 — 已知悉，无需处理
冲突说明
验证本轮实际运行的命令（被测行为仅通过 workflow 测试夹具验证；未改动任何 settings
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
qwen-code-ci-bot
left a comment
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
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 80c53cb. The core risk in this change — census and renderers drifting apart — is held shut from both sides: the budget census now excludes every never-deferrable feedback type (state == "COMMENTED" only, **[Critical]** bodies, replies rooted at a Critical comment, inline comments attached to a CHANGES_REQUESTED review), so the open bot thread's probe scenario (a maintainer with only tagged feedback flagged over-budget) is fixed at this HEAD — the head commit is exactly that fix, landed after the thread (04:18 → 10:31), and I verified the exclusion clauses in the census jq directly; the thread can be resolved. Span bucketing implements (prev ts, ts] precisely with the sentinel-ts guard preventing an unbounded tail span, only Critical-only-territory markers form spans (acted==true && round > R5 or acted==false && round >= R5), and unmatched feedback collapses to empty — fresh feedback cannot burn its own author's budget. Budget keys on the same LIVE_REARM_KEY as the round census and the stale-window guard discards superseded jobs, so /retry genuinely resets the budget with the window. Trust gating is consistent: census counts only TRUSTED_ASSOC non-bot logins, actionable filters keep the existing association gate, and the review bot is excluded by login (budget 0) even if it carries a trusted association — covered by a dedicated fixture. All six filter sites (3 actionable + 3 deferred) carry the budget clauses with the count pinned at exactly 3+3, so a seventh site can't drift in silently. The deferral note interpolates login strings only — no user-controlled body content, no injection surface. SKILL.md matches the filter's contract (everything rendered actionable is in scope). Tests replay the real extracted jq over fixtures for the census and all six filters, covering sentinel, stale-window, untrusted, command-comment, and every tagged-feedback protection case. Ran the changed test file locally at this HEAD: 105/105 passed. CI green except review-pr still running at time of review.
|
Released in v0.21.2. |
Problem
Critical-only mode (after
CRITICAL_ONLY_AFTER_ROUND=5change-producing rounds) classifies feedback lexically, before the agent reads a word of it: only a literal**[Critical]**tag or aCHANGES_REQUESTEDreview survives; everything else is deferred wholesale.That rule was built to stop the review bot's suggestion ping-pong (#7913) — but it catches maintainers too. Observed four times in two days (#8037, #7944, #7885, #7799): a maintainer's E2E-verified review saying "I'd fix before merge" on a correctness bug and a security-adjacent one was deferred as one "non-Critical item", and the bot reported "No Critical feedback. The Issue-level comments sections are empty" — lexically true, substantively false. The agent that could have applied the bot's own advertised Critical definition never saw the comment.
The naive fix — "never defer humans" — fails an obvious counterexample: a human account can host an automated reviewer loop with the exact regeneration property the bot has (feedback re-generated after every push, zero marginal cost). An account is an accountability unit, not a throttle.
Change: a per-source budget keyed on measured regeneration
Once Critical-only engages, every source has a bounded budget of untagged feedback batches per counting window:
CRITICAL_ONLY_HUMAN_BATCHES(2) consumed batches. A batch counts only when a Critical-only round actually consumed it: items are bucketed into the(prev marker ts, marker ts]span that evaluated them; only spans from Critical-only rounds count; an author needs K distinct consumed spans before their new untagged feedback defers. Fresh, not-yet-evaluated feedback never counts against its own author.Effects:
**[Critical]**, a formal Request changes review, or/retry(which rotates the window and resets the budget with it) — which is precisely what separates intent from automation. These escapes cut through at any time, for anyone.SKILL.md's Critical-only policy marks everything rendered actionable as in scope, so the agent doesn't re-refuse what the filter passed through.Tests
critical_onlyboth ways) gain over-budget cases in both directions: an over-budget maintainer's plain comment defers across all three sources, while their reply-to-Critical, CR-review-associated, and formal CHANGES_REQUESTED items still cut through; within-budget maintainers are untouched; bot suggestions defer as before.check-autofix-contracts.shclean; workflow YAML parses.中文说明
问题
Critical-only 模式（5 轮后）在 agent 读到反馈前做纯词法分类：只有字面
**[Critical]**或CHANGES_REQUESTEDreview 能通过。该规则本为掐断 review bot 的建议乒乓（#7913），却把维护者一并挡掉——两天内实锤四例（#8037/#7944/#7885/#7799）：明确写"合并前要修"的正确性/安全问题被整条打包延后，bot 随即宣称"No Critical feedback"。而朴素修法"人类永不延后"过不了一个显然的反例：人类账号完全可以挂自动评审循环，具备与 bot 相同的再生性质（每次 push 重新生成、零边际成本）。账号是问责单位，不是节流阀。
改动：按实测再生频度计费的每来源预算
Critical-only 生效后，每个来源在每个计数窗口内有一个未标记反馈批次预算：review bot 为 0（现状不变）；受信人类为
CRITICAL_ONLY_HUMAN_BATCHES（2）个已消化批次——反馈按消化它的(上一标记 ts, 标记 ts]区间分桶，只计 Critical-only 轮次的区间，作者需累计 K 个不同的已消化区间后，其新的未标记反馈才开始延后；尚未被消化的新反馈永远不计入其作者。效果：四个实测案例在 K=2 下完全不受影响；挂循环的 reviewer 在驱动 ~5+K 轮后被节流，而不是把接管 PR 磨向 100 轮上限；超预算后继续推进需要一次有意识的动作——
**[Critical]**、正式 Request changes、或/retry（轮转窗口、预算随之重置）——这正是意图与自动化的分界；超预算作者在延后说明中被点名并附上这三条通道；SKILL 策略同步声明 actionable 区内一切都在范围内。测试
六个过滤器回放双向覆盖超预算场景（超预算者的普通评论在三个来源全部延后，其 reply-to-Critical / CR-review / 正式 Request changes 仍然穿透；预算内维护者不受影响；bot 建议照旧延后）；预算普查本体用 fixture 文件回放（2 个已消化 critical-tail 批次入列；1 批、pre-Critical 批、未消化反馈、非受信作者、命令评论均不计）；结构 pin 锁死六处子句与五处命令排除站点。102/102 + 14/14 通过；contracts 通过；YAML 正常。