feat(autofix): brake review-round diff growth with per-window src/test budgets - #8981
Conversation
…t budgets Managed PRs bloat while still under the round threshold: #8853 grew from 315 to 1393 net lines in four bot rounds (86% of the growth was test lines; one 'harden per review feedback' round alone added 609), and #8276 grew ~2700 net lines under management. Every push regenerates review suggestions, and every window re-arm reopens the five suggestion-capable rounds, so the round-based Critical-only brake never binds on the size dimension. The prepare step now measures the branch's net diff vs the merge base, split into test lines (*.test.* / *.spec.* files, __snapshots__/, test-utils/, integration-tests/) and source lines, anchors a per-counting-window baseline marker (autofix-growth-base, first-wins, riding the window's first report comment like autofix-redcheck), and engages Critical-only mode early once either dimension outgrows its budget (vars.QWEN_AUTOFIX_GROWTH_BUDGET_{SRC,TEST}_LINES, default 400). Two budgets rather than one because the measured bloat concentrates in tests; a single budget cannot be tightened on tests without strangling source fixes. The deferral preamble names the actual cause, and /retry or re-engaging takeover re-anchors the baseline with the fresh window. Critical findings, Request changes reviews, in-budget maintainer feedback, failed checks, and conflict resolution flow exactly as before.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @wenshao. The growth-brake design reads well, but the PR body doesn't follow the PR template, so I have to gate it before code review.
- Missing required headings:
## What this PR does,## Why it's needed,## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on),## Risk & Scope,## Linked Issues. The body uses## Problem/## Change/## Verificationinstead. - Not just heading names: the
Risk & Scopebullets (main risk/tradeoff, not validated / out of scope) and theTested onOS matrix have no counterpart in the current body at all — those need real content, not just a rename. - The substance otherwise maps cleanly: Problem → Why it's needed, Change → What this PR does, Verification → How to verify. So this is a reshape, not a rewrite.
- Consistency: the sibling PRs in this series all follow the template — e.g. #7913, #8731, #8895 — so this one stands out.
Once the body follows the template, push a commit or re-run with @qwen-code /triage and I'll take it from the top.
中文说明
感谢贡献，@wenshao。增长刹车的设计看起来不错，但 PR 正文没有遵循 PR 模板，所以在进入代码审查前必须先卡在这里。
- 缺少必需标题：
## What this PR does、## Why it's needed、## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)、### Tested on）、## Risk & Scope、## Linked Issues。当前正文用的是## Problem/## Change/## Verification。 - 不只是改标题名：
Risk & Scope（主要风险/取舍、未验证/超出范围）和Tested on操作系统矩阵在当前正文里完全没有对应内容——这两处需要补充实际内容，而不只是重命名。 - 其余内容可以平滑映射：Problem → Why it's needed，Change → What this PR does，Verification → How to verify。所以是重排结构，不是重写。
- 一致性：这一系列的同类 PR 都遵循模板——例如 #7913、#8731、#8895——本 PR 是个例外。
正文按模板调整后，push 一个 commit 或用 @qwen-code /triage 重跑，我会从头开始审查。
— Qwen Code · qwen3.8-max
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. |
Lockfiles (root and nested package-lock.json, npm-shrinkwrap.json) and the
regenerated settings schema arrive hundreds of lines at a time from a
single command and are skimmed rather than reviewed, so counting them
would burn the source budget on churn that carries no review burden. The
exclusion list names generated artifacts exactly — a broad glob would
silently exempt hand-written files from the budget. The fixture test now
proves a root lockfile ('**/' glob-magic at depth zero), a nested one,
and the exact schema path all stay out of the measured nets.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Unresolved, please confirm:
- [Critical] Triage gate review 4913246232 (CHANGES_REQUESTED by @qwen-code-ci-bot): the asserted blocker is the PR body not following the PR template — a process gate on PR metadata, not a code defect. It stands unresolved at the reviewed commit (the body still uses ## Problem / ## Change / ## Verification) and cannot be resolved by this diff; cannot be ruled on from the code. Re-check via @qwen-code /triage after the body is reshaped.
Not explored to full depth (tool budget reached): PR #8981 adds a growth brake to the qwen-autofix workflow...: vitest could not be executed directly (no node_modules in the worktree); I substituted a faithful manual replication of the test's regex extractions and bash/…; PR #8981 adds a growth brake to the qwen-autofix workflow...: none — I completed all planned checks. Items examined:; PR #8981 adds a growth brake to the qwen-autofix workflow...: did not execute scripts/tests/qwen-autofix-workflow.test.js to measure real added suite runtime — no node_modules in the review worktree and a monorepo inst….
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.
中文说明
已审查。 建议见行内评论。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）：PR #8981 adds a growth brake to the qwen-autofix workflow...：vitest could not be executed directly (no node_modules in the worktree); I substituted a faithful manual replication of the test's regex extractions and bash/…；PR #8981 adds a growth brake to the qwen-autofix workflow...：none — I completed all planned checks. Items examined:；PR #8981 adds a growth brake to the qwen-autofix workflow...：did not execute scripts/tests/qwen-autofix-workflow.test.js to measure real added suite runtime — no node_modules in the review worktree and a monorepo inst…。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
- Anchor the baseline marker under the window key prepare READ it with
(LIVE_REARM_KEY), not the matrix WINDOW: supersede-exempt conflict
rounds could write the live window's first marker under a dead key,
letting the round's pushed growth escape the budget for the window.
- Apply GENERATED_EXCLUDES to the test-side measurement too: a lockfile
under integration-tests/ would otherwise be excluded from NET_TOTAL but
counted in NET_TEST, corrupting the NET_SRC subtraction.
- Count __tests__/ as test code, matching AGENTS.md's triage rule and
repo-hygiene's PROD_EXCLUDE; suffix-less helpers there were charged to
the source budget.
- Reject zero-padded budget values in the sanitize guard: [[ -gt ]]
parses them as octal ('0400' brakes 144 lines early, '0900' silently
disables the brake).
- Render signed growth values without a hardcoded '+' ('+-120' read like
a misfire in the cause preamble, both languages).
- Fail open to zero when the three-dot diff has no merge base (orphan-
history branches via fork takeover/adoption), mirroring the merge-tree
conflict probe's fail-open.
- Retry the report post (3 attempts): that one comment carries the
round's entire persisted state — watermark, round, redcheck head, and
now the growth baseline — and the push has already landed by then.
- Behaviorally replay the sanitize fallback and the cause construction
(three engagement shapes, both languages, sign rendering) instead of
text-pinning them; extend the measurement fixture with a __tests__
helper and a lockfile under a test directory.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 81 passed · 0 failed · 81 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：81 通过 · 0 失败 · 81 总计 Verification reportVerification report — PR #8981 (feat(autofix): brake review-round diff growth with per-window src/test budgets)Verdict: 中文摘要
Central claim + A/BCentral claim: when either src or test net-diff growth beyond the per-window baseline exceeds its budget, Critical-only mode engages early (before the round threshold), reusing the round brake's deferral machinery, and the deferred preamble names the actual cause. Blocks under test were sliced verbatim from the YAML-parsed
Witness: Secondary claims, each replayed against real fixtures:
CorrectionsNone needed — the description's mechanism claims (exclusive boundary, first-wins, LIVE_REARM_KEY-keyed write, both-sides excludes, fail-open) all reproduced exactly as stated. FindingsNo blocking findings. Informational:
Mutation matrix (PR's own suite, two affected tests; control green in the same worktree)
Positive controls: unmutated control green; M4's kill quoted above is the intended behavioral assertion with expected-vs-actual values. No mutant regressed from killed to survived. Not covered
MethodologyEnvironment: Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Re-checked the gate against the new head
Moving on to code review. 🔍 中文说明在新 head
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review of the four commits since What the delta does, and what I checked:
Open but non-blocking: thirteen Suggestion-severity threads from the review rounds remain open (the R4 batch — retry dedup, second merge path, unreplayed producer jq, the cap rationale — plus R6-3/R6-4/R6-6, the handoff-comment retry coverage, and R7-2). None contradicts the shipped logic; the loop is converging — round 7 produced a single Suggestion. Worth a follow-up issue, not a gate. Testing evidence — unattended CI run: I did not build or execute any PR code. The evidence is the PR's own CI on the reviewed commit, fetched via the API:
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 All 160 check-runs on this head are completed — 14 success, 143 skipped, and three cancelled 中文说明
增量做了什么、核对了什么：
开放但非阻塞： 审查轮次尚有十三条 Suggestion 级线程未关闭（R4 批次 —— 重试去重、第二条合并路径、未回放的 producer jq、上限理由 —— 以及 R6-3/R6-4/R6-6、handoff 评论重试覆盖、R7-2）。均不与已交付逻辑冲突；循环正在收敛 —— 第 7 轮只产出 1 条 Suggestion。值得开后续 issue，不构成门槛。 测试证据 —— 无人值守 CI 运行：未构建或执行任何 PR 代码。 证据为 PR 自身 CI 在被审 commit 上的结果（经 API 获取，见上表）。 此 head 上全部 160 个 check-run 均已完成 —— 14 成功、143 跳过，另有三个被取消的 — Qwen Code · qwen3.8-max Reviewed at |
Stage 1a template gate resolved: the PR body now follows the template in full (re-run of @qwen-code /triage on 50c1c25). Dismissing so the stale request-changes no longer blocks this PR; full staged review continues in the thread comments.
|
Confidence: 4/5 — clean review across all nine commits; what keeps this from 5 is the thirteen open Suggestion-severity threads from the review rounds (the loop is converging — round 7 produced one), none of which block. Stepping back: since the last pass this PR went through four more managed rounds, and the four commits that landed are all closure work — +89/−10 total. My independent proposal from the first pass (measure in prepare → anchor a marker in the window's first report → trip Critical-only on budget breach via the existing deferral path) still matches what shipped; no design moved. The one genuinely substantive change in the delta is the right call: making "unmeasurable" a state instead of a zero removes the whole phantom-growth class (a bogus 0/0 anchor, or growth manufactured against an existing anchor) rather than papering over it, and it is pinned by replays on both trigger paths. The shadowed-base guard is the kind of corner you only find by running the loop — a fork head literally named If I'm maintaining this in six months: the replay-suite discipline holds — every new branch of behavior in this delta executes under real bash/jq/git, so regressions fail loudly. The residual caveats are documented in the code where they live (post-after base-update anchors, the conflict round's own merge of main) and both recover with one Housekeeping: the earlier direct approval on Verdict: approve. CI is green on exactly this commit (including the job that runs the replay suite), no PR-event workflow runs are pending, and the approval is pinned to the reviewed commit. 中文说明置信度：4/5 —— 九个 commit 的审查均干净；未到 5 是因为审查轮次尚有十三条 Suggestion 级线程开放（循环正在收敛 —— 第 7 轮只产出 1 条），均不阻塞。 整体看：上次审查后本 PR 又经历了四轮托管，落地的四个 commit 全部是收尾工作 —— 合计 +89/−10。我在首轮形成的独立方案（prepare 中测量 → 窗口首个报告锚定标记 → 超预算经既有延后路径触发 Critical-only）与落地方案仍然一致，设计没有移动。增量中唯一实质性的改动方向正确：把"不可测量"从"零"改成一种状态，整类消除了幻影增长（伪造的 0/0 锚点、相对既有锚点制造的增长），且两条触发路径都有回放钉住。影子 base 守卫是只有真正跑循环才会发现的角落 —— fork head 恰好名为 若半年后由我维护：回放套件的纪律保持住了 —— 本增量中每一条新行为分支都在真实 bash/jq/git 下执行，回归会响亮地失败。残留注意点已就地文档化（base 更新后才发布的锚点、冲突轮自身合并 main），两者都可用一次 杂务：此前对 结论：批准。 CI 恰在此 commit 上全绿（含运行回放套件的 job），无未决的 PR 事件 workflow 运行，批准绑定被审 commit。 — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed — one new commit ( 本次 re-run 已完成 —— 距上次审查新增了一个 commit（ The stage comments above were updated in place with the latest result. View workflow run. 上方各阶段评论已就地更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): PR #8981 adds a net-diff growth brake to the autofix revi...: exhaustive audit of every autofix-eval /redcheck marker consumer in the scan step (lines ~2600-3500) for sensitivity to duplicate comments — only checked the c….
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.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：PR #8981 adds a net-diff growth brake to the autofix revi...：exhaustive audit of every autofix-eval /redcheck marker consumer in the scan step (lines ~2600-3500) for sensitivity to duplicate comments — only checked the c…。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
- Spell the growth marker's window field key= instead of win=: the same report comment can legitimately carry a different window key than its autofix-eval marker (supersede-exempt conflict round after a re-arm), and three censuses attribute comments to windows by the whole-body substring win=<key> -->, which would double-attribute that comment to both windows (probe-flipped PRIOR_TIMEOUTS, WIN_HEADS, PRIOR_HEADS). A distinct token immunizes every such census without touching them. - Make the deferred preamble's batch-budget sentence conditional: the OVER_BUDGET census only builds spans in round-brake territory, so a growth-only engagement below the threshold now states that maintainer feedback flows unaffected instead of promising accounting the census cannot produce. - Special-case the report-post retry's final attempt: no trailing 'retrying' + 10s sleep before giving up. - Include __tests__/ in the env comment's test-line enumeration (the tunables doc must match the pathspec). - Replay coverage for everything the mutation probes showed unpinned: the baseline wiring block (parseable/empty/malformed baselines), the no-merge-base fail-open (0/0/0 under -eo pipefail with the origin ref deleted), the report-post retry (single post on success; exactly three attempts, 'giving up', exit 1 on outage), the writer→scanner marker round-trip (negative src rendered from the real template and parsed back), and the budget-sentence branches in both languages.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): PR #8981 adds a net-diff growth brake to the autofix revi...: none** — all checks I started are concluded above.; PR #8981 adds a net-diff growth brake to the autofix revi...: none — all planned checks completed within budget.; PR #8981 adds a net-diff growth brake to the autofix revi...: running scripts/tests/qwen-autofix-workflow.test.js green in this worktree — no node_modules present and dependency installation was skipped; findings rest ….
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.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：PR #8981 adds a net-diff growth brake to the autofix revi...：none** — all checks I started are concluded above.；PR #8981 adds a net-diff growth brake to the autofix revi...：none — all planned checks completed within budget.；PR #8981 adds a net-diff growth brake to the autofix revi...：running scripts/tests/qwen-autofix-workflow.test.js green in this worktree — no node_modules present and dependency installation was skipped; findings rest …。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 132 passed · 0 failed · 132 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：132 通过 · 0 失败 · 132 总计 Verification reportVerification report — PR #8981 (feat(autofix): brake review-round diff growth with per-window src/test budgets) — FOLLOW-UP ROUNDVerdict: This is a follow-up round. The previous round verified head 中文 — 判定：✅ 通过 · 可合入（agent 判定）· 跟进轮跟进轮沙箱验证：132/132 脚本断言通过，PR 自带行为套件 163/163 全绿，突变矩阵 11/12 被杀（唯一存活者 M7 为已知展示级缺口）。
Previous-finding status (carried forward from the round at
|
| # | Finding (round 2) | Severity | Status at 92a075b |
|---|---|---|---|
| 1a | M8 survivor: ZH budget parenthetical not pinned by the suite | Informational | FIXED — the budget sentence is now conditional on the engagement cause and replayed in both languages; the rewording mutant dies on expected '纯增长触发期间维护者反馈不受影响…' to contain '照常流动' (mutant M8, evidence/04-mutation-matrix-11-of-12-killed.png). |
| 1b | M7 survivor: 📏 log wording not pinned |
Informational | STANDS — re-measured: the reworded log line survives the two affected tests AND the full 163-test suite (logs/mutant-M7-full.log). Display-only; the numbers in the line are exercised by the harnesses. Completeness reporting, not a merge condition. |
| 2 | Non-npm lockfile churn (yarn.lock, pnpm-lock.yaml) counts as source growth |
Informational (declined-with-rationale) | STANDS, decline agreed — GENERATED_EXCLUDES unchanged and deliberately exact-named (a broad glob would silently exempt hand-written files); the repo is npm-only. My independent fixture includes pnpm-lock.yaml/yarn.lock decoys and confirms they ARE measured as source, as documented. |
| 3 | Working tree carried a pre-existing local modification reverting the PR's SKILL.md wording | Environmental note | STANDS — still present at the new head (git status: M .qwen/skills/autofix/SKILL.md, reverting the brake wording). Left untouched; every block under test was extracted from git show HEAD: blobs, so the verified artifact is the submitted PR. |
Declined-and-deferred rows were re-measured, not diffed against the old report: M7/M8 mutants were re-applied at the new head and re-run; GENERATED_EXCLUDES was re-exercised through the measurement fixture; the SKILL.md residue was re-inspected with git diff.
Central claim + A/B (re-measured at the new head and the new base)
Central claim: when either src or test net-diff growth beyond the per-window baseline exceeds its budget, Critical-only mode engages early (before the round threshold), reusing the round brake's deferral machinery, and the deferred preamble names the actual cause.
Blocks sliced verbatim (js-yaml-parsed run: scripts, harness/slice.mjs with single-match anchors) from head and base, executed under the step's shell contract (bash --noprofile --norc -eo pipefail, the workflow's defaults.run.shell: bash).
| Cell (ROUND, growth src/test, budgets 400/400) | HEAD CO ROUNDS GROWTH |
BASE CO |
Witness |
|---|---|---|---|
| r2, 500/12 (src trip, early round) | true false true |
false |
evidence/01-ab-decision-head-vs-base.png |
| r2, 400/400 (AT budget) | false false false |
false |
boundary exclusive, both arms |
| r2, 401/0 · r2, 0/401 | true false true |
false |
either dimension trips |
| r2, −900/−900 | false false false |
false |
shrink never trips |
| r5, 0/0 | true true false |
true |
round brake preserved (A/A control) |
| r5, 401/0 | true true true |
true |
both causes coexist |
| r4, 400/400 · r4, 0/401 · r0, 0/0 | as expected | as expected | 10 cells × 2 arms = 20/20 |
The deferred-feedback half (23/23, evidence/02-ab-deferred-preamble-head-vs-base.png): growth-only at r2 emits "Critical-only mode is active: the PR's diff grew src 500 / test -120 …" with no rounds clause, no +- rendering, and the conditional budget sentence "Maintainer feedback continues to flow unaffected…" (EN/ZH); rounds-only and both-causes shapes correct in both languages; the deferred item list (3 fixture items: review, inline, PR comment; Critical/bot/marker//retry excluded) is byte-identical head vs base — the mechanism is reused unchanged. Base positive control renders the old wording at r5; base at r2 with growth 500 writes no deferred section at all — the load-bearing contrast.
Secondary claims, replayed against real fixtures (all carried forward and re-run):
| Surface | Result | Witness |
|---|---|---|
Measurement, independent fixture (decoys pnpm-lock.yaml/yarn.lock/near-miss schema path, __tests__ helper, root+nested+test-dir lockfiles, exact schema path, binary, deletion) |
total/test/src = 29/19/10; corroborated by raw git diff --numstat arithmetic (second instrument: 48 − 19 excluded = 29); churn-only 0/0/0; deletions −8/−0/−8; orphan fail-open 0/0/0 exit 0 under -eo pipefail AND plain -e |
evidence/03-measure-independent-fixture.png (8/8) |
| Baseline parse (real jq 1.6 over fixture ic.json) | first-wins by created_at regardless of array order; spoof/wrong-key/malformed rejected; legacy win= marker rejected by the key= scanner (rename regression cell); negative baseline round-trips; empty window re-anchors |
h4 (8/8) |
| Sanitize + octal hazard | 0400/0900/garbage/empty/negative/8-digit/decimal → 400 with 2 warnings; 0 and 7-digit pass; positive controls: unguarded [[ 300 -gt 0400 ]] ENGAGES (octal 256), [[ 1000 -gt 0900 ]] errors and compares false |
h5 (10/10) |
| Marker write cells + round-trip | pushed/noop branches write exactly one marker under the READ key (not matrix WINDOW); GROWTH_BASE_WIN empty → WINDOW → none; false writes nothing; template renders src=-5 and the real scanner parses it back (-5 0); rendered marker carries no win= |
h6 (8/8) |
| Report-post retry | success → 1 call / 0 sleeps / exit 0; fail-fail-ok → 3 calls / 2 sleeps / exit 0; outage → 3 calls / 2 sleeps / giving up / exit 1; real-sleep cell elapsed 20s |
evidence/05-retry-final-attempt-delta.png (14/14) |
| Baseline wiring | parseable/empty/malformed/negative baselines: growth = net − base, fresh anchor on empty/malformed | h9 (4/4) |
Delta verification — commit 92a075b (the round-2 follow-ups)
The checkout is depth-2, so the previous head 50c1c25e is not locally available and the delta was verified claim-by-claim against the head code instead of by direct diff:
key=marker spelling (census double-attribution) — VERIFIED. The three censuses (PRIOR_TIMEOUTSline 4447,WIN_HEADSline 5242,PRIOR_HEADSline 5705) select comments by whole-bodycontains("win=" + $key + " -->"). Running the REALPRIOR_TIMEOUTS/WIN_HEADSjq over a comment carrying eval markerwin=WINSTALE+ growth markerkey=WINLIVE: attributed to WINLIVE 0 times, to WINSTALE 1 time. Counterfactual with the old spellingwin=WINLIVE: attributed to WINLIVE 1 time AND still to WINSTALE — one comment counted in two windows (evidence/06-key-marker-census-no-double-attribution.png, 5/5).LIVE_REARM_KEYcan never be empty (jq// "none"floor), so thekey=${GROWTH_BASE_WIN:-${WINDOW:-none}}fallback chain only matters when prepare exited early — which also suppresses the marker (growth_base_newempty ≠true).- Conditional budget sentence — VERIFIED.
BUDGET_EN/ZHare gated onCRITICAL_ONLY_ROUNDS: rounds engagement keeps the batch-budget accounting sentence; growth-only says maintainer feedback flows unaffected (matching the OVER_BUDGET census, which only builds spans past the round threshold). Both branches replayed in both languages (h2 cells + suite mutant M11 killed on the flipped condition). - Retry final attempt — VERIFIED. Outage cell: exactly 3
ghcalls, exactly 2 sleeps and 2retryingwarnings, then::error::… giving upand exit 1 — no trailing sleep on the final attempt (sleep count asserted via an instrumented stub; the mixed path additionally timed at 20s with realsleep). __tests__/in the env-comment enumeration — VERIFIED. All six pathspec entries (*.test.*,*.spec.*,__snapshots__,__tests__,test-utils,integration-tests) appear in bothTEST_PATHSPECand the workflow env comment; the pathspec has exactly 6 glob entries (h10, 7/7).- New replay coverage for previously unpinned axes — VERIFIED via the mutation matrix below: baseline wiring, no-merge-base fail-open under pipefail (M12 killed), report-post retry semantics (M9 killed), writer↔scanner round-trip (M10 killed), budget-sentence branches both languages (M8/M11 killed).
Corrections
None needed — every mechanism claim in the description and the new commit's message reproduced exactly as stated.
Findings
No blocking findings. Informational:
- M7 survivor persists (completeness reporting, not a merge condition). Rewording the
📏 net diff …job-log line survives the full 163-test suite — the log wording is rendered but pinned nowhere. Display-only; the numeric content is exercised by the harnesses. Carried from round 2, re-measured at the new head. If the author wants it pinned, atoContain('📏 net diff src')on the prepare step closes it. - The suite's retry replay covers constant-outcome stubs only (always-succeed, always-fail). The fail-fail-success path is exercised by this round's harness (3 calls, 2 sleeps, exit 0, plus the 20s real-sleep timing cell) and is transitively pinned by the suite's success-cell call-count assertion; no defect, noted for the coverage map.
- Harness caveats found and fixed during the round (transparency): my first h2 item-list comparison had a sort bug (fixed, re-run clean) and my first matrix driver misread ANSI-colored vitest output (fixed with
NO_COLOR=1, full matrix re-run) — both re-measured, no effect on the PR under test. Separately,bash -non this bash 5.2 exits 0 for errors inside[[ ]]while reporting them (rc=2 only for structural errors) — the liveness plants used structural errors.
Mutation matrix at the new head (control green: 2 passed | 161 skipped, in the same scratch worktree)
| Mutant | Result | Killing assertion (or survival evidence) |
|---|---|---|
M1 -gt→-ge |
KILLED | mode-block shape pin (expected undefined to be truthy) |
M2 ||→&& |
KILLED | same pin |
M3 growth-if never sets CRITICAL_ONLY |
KILLED | same pin |
M4 drop __tests__/ from TEST_PATHSPEC |
KILLED | behavioral: expected '22 16 6' to be '22 18 4' |
| M5 drop GENERATED_EXCLUDES from NET_TEST side | KILLED | behavioral: expected '22 24 -2' to be '22 18 4' |
M6 growth_base_win=${WINDOW} instead of ${LIVE_REARM_KEY} |
KILLED | contract pin growth_base_win=${LIVE_REARM_KEY} |
M7 reword 📏 log |
SURVIVED (coverage gap) | green on the two affected tests AND the full 163 suite |
| M8 reword ZH growth-only budget sentence | KILLED | toContain('照常流动') — round-2 survivor closed |
| M9 drop retry final-attempt special case | KILLED | expected '::warning::…' to contain 'giving up' |
M10 marker field key=→win= |
KILLED | marker occurrence pin expected +0 to be 2 (round-trip would fail next) |
M11 budget condition flipped (!=) |
KILLED | budget-block shape pin |
M12 remove fail-open || NET_TOTAL=0 |
KILLED | orphan replay: Command failed: bash -c set -eo pipefail |
No mutant regressed from killed to survived versus round 2; one previously surviving mutant (M8) is now killed. Witness: evidence/04-mutation-matrix-11-of-12-killed.png. Positive control: unmutated head green in the same worktree immediately before the matrix.
Reviewer Test Plan walk-through
Every step of the PR's own plan was performed: the behavioral suite runs the Critical-only block across the round/growth combinations (boundary exclusive ✓, either dimension ✓, both causes ✓, negative growth never ✓); the measurement block runs against a real fixture repo yielding 22 18 4 in the PR's fixture and 29 19 10 in my independent one ✓; the baseline jq replays over fixture comments (bot-only, window-keyed, first-wins, negative nets, spoof rejection, empty window ✓); sanitize fallback and cause construction execute ✓; marker write sites / env plumbing / output writes contract-pinned ✓ (evidence/07-targeted-suite-head-163-green.png). npm run test:scripts itself: the autofix file green (163/163); the only red is install-script.test.js failing COLLECTION because the container lacks zip — the file is byte-identical on both arms (0-line diff), so the failure is environmental, and 1037/1037 collected tests pass.
Not covered
- Real
gh pr commentposting and GitHub-sidevars/scan behavior (no credentials; retry replayed with a stubgh, real bash, realsleepon the timing cell; jq 1.6 here vs 1.7 on ubuntu-latest —scan()already load-bearing for existing markers, and h4 ran the exact new expression). yamllint: no pip in this container (python3 -m pip→ no module); the file passes actionlint 1.7.7 clean (liveness-proven by a plantedsteps.NOSUCHSTEP.outputs, caught at the PR's env block, line 4881) andbash -nclean on all four extracted step scripts (liveness-proven by a planted missing-fi, rc=2).shellcheckover the embedded workflow bash: absent from the container, and the repo's own gate only scans tracked.shfiles; embedded bash covered bybash -n+ behavioral replay instead.- Per-commit attribution: depth-2 checkout (local
rev-list HEAD^1..HEAD^2= 1 vs 5 commits in the metadata snapshot); the aggregateHEAD^1..HEADdiff was verified and the new commit's five enumerated changes were each verified against head code and behavior (Delta section above). - End-to-end workflow execution on Actions runners; the
/retryre-arm lifecycle end-to-end (its mechanism chain — key-scoped parse, empty-window re-anchor, marker written under the read key — was verified link by link). This replay reproduces the wire shape the workflow will emit, not a live Actions run. - SKILL.md doc hunk (docs-only, read).
Methodology
Environment: node:22-bookworm CI verify container; merge-ref checkout (HEAD = merge 3c7e9264, HEAD^1 = base tip f2de42dec4, HEAD^2 = verified head 92a075b6). The two changed steps' run: scripts were dumped verbatim via js-yaml from git show HEAD:/git show HEAD^1: blobs (harness/extract.mjs); blocks sliced between single-match anchors (harness/slice.mjs) and executed under the step's shell contract with fixture ic.json/rv.json/rc.json files and fresh mktemp git repos carrying refs/remotes/origin/main — never stubbing the code under test. Mutation matrix applied to a scratch worktree (git worktree add --detach tmp/mut-tree HEAD, since removed) whose nested node_modules resolution walks up to the repo root; each mutant verified single-occurrence before replacement. Harnesses h1–h10 + gates emitted TAP (logs/*.tap); mutation and gate logs in logs/; the ledger was computed from those logs by harness/tally.mjs (nothing hand-counted) → assertions.json. Evidence captured with scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
- Invalidate growth anchors older than the latest stale-base auto-update: the update merges main into the branch and moves the merge base the nets are measured against, so an earlier anchor is no longer comparable — the next round re-anchors at the post-update size instead of misattributing overlap-resolution deltas to review growth. - Pin the merge-base (three-dot) semantics: the measurement fixture now advances main past the divergence, so a two-dot regression changes the expected numbers instead of shipping green. - Pin the sanitize guard's 7-digit cap (9999999 accepted, 10000000 falls back): past it bash integer literals wrap at 64 bits. The census-side hazard (whole-body win= attribution vs multi-key comments) is declined for this PR with the invariant documented at the scanner: the growth marker's key= token cannot match any win= census, and hardening the three censuses to positional attribution is queued as its own change.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...: did not execute the new vitest test to measure its real wall-clock runtime (or confirm it passes) — no node_modules in the review worktree and none in the paren…; PR #8981 (QwenLM/qwen-code), reviewed commit 149b8a1...: none — the walk is complete.; PR #8981 (QwenLM/qwen-code), reviewed commit 149b8a1...: none — all planned checks completed within budget..
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.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...：did not execute the new vitest test to measure its real wall-clock runtime (or confirm it passes) — no node_modules in the review worktree and none in the paren…；PR #8981 (QwenLM/qwen-code), reviewed commit 149b8a1...：none — the walk is complete.；PR #8981 (QwenLM/qwen-code), reviewed commit 149b8a1...：none — all planned checks completed within budget.。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| REPORT_POSTED='false' | ||
| for attempt in 1 2 3; do | ||
| if gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md"; then |
There was a problem hiding this comment.
[Suggestion] R4-2: The report-post retry loop re-POSTs a non-idempotent comment create with no dedup guard — an attempt that commits server-side but fails client-side (connection reset while reading the response, post-commit 502) leads the next attempt to post a duplicate report comment carrying the same autofix-eval/redcheck/growth-base markers — Failure scenario: transient network failure between the server commit and the response read on attempt 1 → attempt 2 posts a twin → the PR shows the same round report twice, and the takeover milestone digest counts comments per window by occurrence (WIN_HEADS first lines → N_PUSHED/N_NOOP/N_TOTAL via grep -c), so the digest reports one more pushed/no-op round than actually happened. Verified non-consequences (so this is a Suggestion, not a Critical): watermark/round/red-head reads are max/last-based, the growth baseline is first-wins, and the retry wraps only success-shape report posts (the failure handoff is single-shot), so the CONSEC_FAIL/TIMEOUT_N breakers — failure/timeout headlines only — cannot advance on a twin. Rare trigger; audit-record inflation only. Suggested fix: accept and document the duplicate as the worst case, or make the post idempotent — before attempt 2/3, check whether the newest bot comment already contains this round's autofix-eval ts=${NEWEST} marker and break out as posted; or dedupe the census by the eval round= field instead of headline occurrence.
中文说明
[Suggestion] R4-2：报告发送重试循环重复 POST 的是一个非幂等的评论创建操作，且没有去重防护——如果某次尝试已在服务端提交但客户端收到失败（读取响应时连接被重置、提交后的 502），下一次尝试会再发一条携带相同 autofix-eval/redcheck/growth-base 标记的重复报告评论 —— 触发场景：第 1 次尝试在服务端提交与响应读取之间发生瞬时网络故障 → 第 2 次尝试发出孪生评论 → PR 上同一轮报告出现两次，而 takeover 里程碑摘要按出现次数统计每个窗口的评论（WIN_HEADS 首行 → grep -c 得到 N_PUSHED/N_NOOP/N_TOTAL），于是摘要会多报一轮 pushed/no-op。已验证无其他后果（因此是 Suggestion 而非 Critical）：水位/轮次/red-head 读取基于 max/last，增长基线 first-wins，且重试只包裹成功形态的报告发送（失败 handoff 是单次的），所以 CONSEC_FAIL/TIMEOUT_N 断路器——只统计失败/超时标题——不会被孪生评论推进。触发概率低；仅影响审计记录的计数。建议修复：接受并把重复作为最坏情况写入注释；或让发送幂等——第 2/3 次尝试前检查最新 bot 评论是否已包含本轮的 autofix-eval ts=${NEWEST} 标记，若已存在则按已发送处理；或让摘要按 eval 的 round= 字段去重而不是按标题出现次数。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| BASE_UPD_AT="$(jq -r --arg ab "${AUTOFIX_BOT}" ' | ||
| [ .[] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains("<!-- autofix-base-updated")) |
There was a problem hiding this comment.
[Suggestion] R4-3: Baseline invalidation keys solely on the bot's autofix-base-updated marker, which is written at exactly one site (the scan-side update path, ~line 3005) — but a second merge path exists: the stale-base retry in Report dry-run / failure (~5578-5586: gate rejection + behind/diverged → update-branch API merge, STALE_BASE_RETRY=true) merges main and posts a handoff comment carrying only an autofix-eval marker, no base-updated marker — Failure scenario: round N anchors the baseline against merge base M0; round N+1 the gate rejects while the branch is behind, the stale-base retry merges main (base moves to M1) and posts no marker; round N+2 BASE_UPD_AT is still empty/stale, the round-N anchor passes .at > $baseupd, and GROWTH = (net vs M1) − (baseline vs M0) — the delta contains the base shift, not agent growth, so the brake engages early on phantom growth (or masks real growth) and the deferred preamble misattributes it to window bloat. Related variant: the line-3005 marker post itself can fail and be swallowed into a ::warning:: after update-branch already succeeded, leaving even a scan-side update invisible to this filter. Harm is bounded by the main/PR overlap and /retry re-anchors; the diff's rationale block argues the scan-side case and the conflict-round residual, but not this third path. Suggested fix: post the same <!-- autofix-base-updated --> marker on the stale-base retry's handoff comment (or a dedicated marker this filter also matches), and consider failing loud when the line-3005 post fails after a successful update-branch; a comment here naming both merge paths keeps the invariant honest.
中文说明
[Suggestion] R4-3：基线失效只认 bot 的 autofix-base-updated 标记，而该标记只在一个位置写入（scan 侧更新路径，约第 3005 行）——但还存在第二条合并路径：Report dry-run / failure 中的 stale-base 重试（约 5578-5586：门拒绝 + behind/diverged → update-branch API 合并，STALE_BASE_RETRY=true）会合并 main 并发出一条只带 autofix-eval 标记、不带 base-updated 标记的 handoff 评论 —— 触发场景：第 N 轮把基线锚定在 merge base M0 上；第 N+1 轮门拒绝且分支落后，stale-base 重试合并 main（base 移到 M1）但不发标记；第 N+2 轮 BASE_UPD_AT 仍为空/过期，第 N 轮的锚点通过 .at > $baseupd，于是 GROWTH = (相对 M1 的净值) − (相对 M0 的基线) —— 差值里包含 base 漂移而非 agent 增长，刹车会因幻影增长提前触发（或掩盖真实增长），且 deferred 前言会把它错误归因为窗口膨胀。相关变体：第 3005 行的标记发送本身也可能失败，且在 update-branch 已经成功之后被吞进一条 ::warning::，连 scan 侧更新也会对该过滤器不可见。危害受 main/PR 重叠量限制，/retry 可重锚；diff 的说明块论证了 scan 侧情形与冲突轮残差，但未覆盖这第三条路径。建议修复：在 stale-base 重试的 handoff 评论中同样发送 <!-- autofix-base-updated --> 标记（或一个该过滤器同样匹配的专用标记），并考虑在 update-branch 成功后第 3005 行发送失败时让步骤显式失败；在此处加一条注释点名两条合并路径，使该不变量表述完整。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| BASE_UPD_AT="$(jq -r --arg ab "${AUTOFIX_BOT}" ' | ||
| [ .[] | select((.user.login // "") == $ab) | ||
| | select((.body // "") | contains("<!-- autofix-base-updated")) |
There was a problem hiding this comment.
[Suggestion] R4-4: This BASE_UPD_AT producer jq (the timestamp used to invalidate anchored baselines) is extracted and replayed by no test, while its consumer IS replayed (the new test feeds baselineFor a fabricated baseupd argument) — Failure scenario: probe-verified mutation — flipping max → min at line ~4113 ships green (the full suite passes 163/163), while a canary mutation on the covered consumer (.[0]→.[-1]) fails the suite, so the harness is live and the producer is genuinely uncovered. With min, after two stale-base auto-updates in one window the filter uses the FIRST update's timestamp, so an anchor recorded between the two updates — measured against a merge base that no longer exists — stays in force, and growth is compared across incomparable bases for the rest of the window, engaging or masking the brake by exactly the merge shift. A future edit dropping the bot-login select or misspelling the autofix-base-updated substring ships green the same way. Suggested fix: extract this producer jq the same way the test already extracts baselineJq, and replay it against a fixture ic.json containing two bot autofix-base-updated comments (different timestamps) plus one spoofed from another login — asserting it returns the LATER created_at, "" when no marker exists, and ignores the spoof; that kills max→min and the filter mutations.
中文说明
[Suggestion] R4-4：这个 BASE_UPD_AT 生产者 jq（用于失效已锚定基线的时间戳）没有任何测试提取并回放，而它的消费者有回放（新测试给 baselineFor 传入构造的 baseupd 参数）—— 触发场景：经探针验证的变异——把约第 4113 行的 max → min 翻转后整套测试仍然全绿（163/163 通过），而对照消费者侧的金丝雀变异（.[0]→.[-1]）会让测试失败，说明测试装置是有效的、该生产者确实未被覆盖。若为 min，同一窗口发生两次 stale-base 自动更新后，过滤器会使用第一次更新的时间戳，于是记录在两次更新之间的锚点——其测量所依据的 merge base 已不存在——继续生效，窗口余下时间里增长会跨不可比的 base 计算，刹车恰好按 base 漂移的量提前触发或被掩盖。未来若有人删掉 bot-login 的 select 或拼错 autofix-base-updated 子串，同样会全绿通过。建议修复：像测试已提取 baselineJq 那样提取这个生产者 jq，对包含两条 bot autofix-base-updated 评论（时间戳不同）外加一条来自其他 login 的伪造评论的 fixture ic.json 回放——断言返回较晚的 created_at、无标记时返回 ""、忽略伪造项；即可杀死 max→min 与过滤器变异。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if [[ ! "${GROWTH_BUDGET_SRC_LINES}" =~ ^(0|[1-9][0-9]{0,6})$ ]]; then | ||
| echo "::warning::GROWTH_BUDGET_SRC_LINES='${GROWTH_BUDGET_SRC_LINES}' is not a plain line count; using 400" | ||
| GROWTH_BUDGET_SRC_LINES=400 |
There was a problem hiding this comment.
[Suggestion] R4-5: The 7-digit cap rejects every well-formed line count ≥ 10,000,000 with a silent fallback to 400, and the pinned rationale — the new test comment says the cap is load-bearing because past it "bash integer literals wrap at 64 bits and comparisons go silently wrong" — is factually wrong — Failure scenario: measured: $(( 1000000000000000000 )) evaluates fine; bash arithmetic wraps only near 9.2×10^18 (~19 digits), so the cap is stricter than the arithmetic requires by ~11 orders of magnitude, justified by a false comment. The env block promises "a malformed value falls back to its default at the read site", implying well-formed values are honored — an operator setting QWEN_AUTOFIX_GROWTH_BUDGET_SRC_LINES=100000000 (well-formed, e.g. to relax the brake for a large-PR campaign) gets the fallback 400 instead: the brake engages at 400 lines, the opposite of intent, with only a ::warning:: in the run log as the signal — and the false rationale is now pinned into the suite (sanitized('9999999', '10000000') → '9999999 400'), so a future maintainer "correcting" the cap has wrong arithmetic to reason from. Suggested fix: correct the rationale (state the cap is a policy limit, not a 64-bit guard — in the env comment and the test comment), and/or widen the cap toward what the arithmetic actually requires (e.g. {0,17} stays below 2^63−1) — if widened, update both budgets and the test pin together.
中文说明
[Suggestion] R4-5：7 位上限会把所有 ≥ 10,000,000 的合法行数静默回退为 400，而被钉住的依据——新测试注释说该上限是承重项，因为超过它 "bash 整数会在 64 位回绕、比较会静默出错"——在事实上是错的 —— 触发场景：实测 $(( 1000000000000000000 )) 计算正常；bash 算术要到约 9.2×10^18（约 19 位）才回绕，因此该上限比算术所需严格了约 11 个数量级，且由一条错误注释背书。env 块承诺"非法值在读取处回退默认"，言下之意合法值会被采纳——运维若设置 QWEN_AUTOFIX_GROWTH_BUDGET_SRC_LINES=100000000（合法值，例如为大 PR 批次放宽刹车），实际得到的却是回退值 400：刹车在 400 行就触发，与意图相反，唯一的信号是运行日志里的一条 ::warning:: ——而且错误依据如今已被测试钉住（sanitized('9999999', '10000000') → '9999999 400'），未来想"修正"该上限的维护者只能依据错误的算术推理。建议修复：更正依据（在 env 注释与测试注释中写明该上限是策略限制而非 64 位防护），和/或把上限放宽到算术实际所需（如 {0,17} 仍在 2^63−1 之内）——若放宽，两个预算与测试钉值需一并更新。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| NET_TOTAL="$(git diff --numstat origin/main...HEAD -- "${GENERATED_EXCLUDES[@]}" 2> /dev/null | sum_numstat)" || NET_TOTAL=0 | ||
| NET_TEST="$(git diff --numstat origin/main...HEAD -- "${TEST_PATHSPEC[@]}" "${GENERATED_EXCLUDES[@]}" 2> /dev/null | sum_numstat)" || NET_TEST=0 |
There was a problem hiding this comment.
[Suggestion] R4-6: The fail-open fallback substitutes zero nets when the three-dot diff fails (no merge base, exit 128) — the inline comment promises "an unmeasurable PR skips the brake", but that only holds when no baseline is anchored; with an existing anchor the zero flows into GROWTH = NET − BASE and can engage the brake on a phantom — Failure scenario A (probe-executed end-to-end against the real extracted blocks): a window anchored at a NEGATIVE baseline (a net-shrinking refactor, e.g. src=-500 — the marker template round-trips negatives) whose branch is later force-pushed to an orphan history: both measurements fail open to 0, GROWTH_SRC = 0 − (−500) = 500 > 400, Critical-only engages, and the preamble announces the PR "grew src 500 net lines beyond the baseline" — factually backwards. Scenario B (probe-executed): an orphan branch admitted at window start anchors at src=0 test=0; when a merge base later appears via a merge that posts no base-updated marker (the stale-base retry path, or a manual merge), the next round measures the PR's ENTIRE net size as growth — exceeding the diff's stated overlap-bound residual, since the anchor is 0. Both self-heal only via /retry. Suggested fix (probe-verified): add a NET_MEASURED flag (true normally, false in the || …=0 fallbacks) and (i) gate the growth -gt engagement on it, (ii) suppress GROWTH_BASE_NEW/the marker write when false, and (iii) leave an already-anchored baseline in place without engaging against it while unmeasurable — matching the documented intent.
中文说明
[Suggestion] R4-6：三点 diff 失败（无 merge base，exit 128）时 fail-open 回退把净值替换为零——行内注释承诺"不可测量的 PR 跳过刹车"，但这只在尚无基线锚点时成立；已有锚点时，零会流入 GROWTH = NET − BASE，可能因幻影触发刹车 —— 触发场景 A（对真实提取的块做了端到端探针执行）：某窗口的基线锚点为负数（净瘦身重构，如 src=-500——标记模板可往返负值），其分支随后被强推为孤儿历史：两次测量都 fail-open 为 0，GROWTH_SRC = 0 − (−500) = 500 > 400，Critical-only 触发，且前言宣称该 PR "超出基线净增 src 500 行"——与事实完全相反。场景 B（探针执行）：窗口开始时被接纳的孤儿分支把基线锚定在 src=0 test=0；之后某次不发送 base-updated 标记的合并（stale-base 重试路径或人工合并）使 merge base 出现，下一轮会把 PR 的全部净体积测成增长——由于锚点是 0，这超出了 diff 声明的"受重叠量限制"的残差边界。两者都只能通过 /retry 自愈。建议修复（已探针验证）：增加 NET_MEASURED 标志（正常为 true，在 || …=0 回退处置为 false），并 (i) 用它门控增长 -gt 触发，(ii) 为 false 时抑制 GROWTH_BASE_NEW/标记写入，(iii) 保留已有锚点但在不可测量期间不对其触发——与文档承诺的意图一致。
— qwen3.8-max via Qwen Code /review (v0.21.10)
…ed main Prepare's fork path re-points refs/remotes/origin/main at the fork head for a fork:main PR, so the three-dot measurement would compare the branch against itself and report 0/0 every round — silently disabling the brake while appearing to run. Unmeasurable is unmeasurable: skip and say so, matching the no-merge-base fail-open.
|
Round status: all Critical findings from the latest review are fixed and replied in-thread. The remaining open Suggestion threads (test-depth pins, class-granularity refinements, rendering polish) are acknowledged and deliberately deferred — this PR is at the point where absorbing further optional rounds grows the diff faster than it improves it, which is precisely the dynamic this work exists to brake. They stay open as the follow-up backlog; substantive ones will land as their own change. 中文：最新一轮 Critical 已全部修复并逐条回复；其余 Suggestion 线程按本 PR 自身的收敛哲学明示搁置、保持 open 作为后续清单。 🤖 via Claude Code · model |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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 via Qwen Code /review (v0.21.10)
| git('update-ref', '-d', 'refs/remotes/origin/main'); | ||
| const orphan = execFileSync( |
There was a problem hiding this comment.
[Suggestion] The BRANCH == 'main' skip — the headline addition of the reviewed commit — is inside the extracted measureBlock, but no replay defines BRANCH, so only the else branch ever executes. Mutant-verified at this commit: changing == 'main' to == 'master' (or deleting the block outright) ships 163/163 green — the extraction regex still matches the reduced block and both existing replays still pass. Failure scenario: a future edit removes or restructures the skip (someone 'simplifies' the if/else, or the update-ref changes and the guard is deemed obsolete) → fork-main PRs silently regress to measuring the branch against itself (0/0 forever, brake dead) — precisely the regression the skip was added to prevent, shipping green. Suggested fix: add one replay before the origin/main ref is deleted — set BRANCH=main while the fixture still carries growth, run measureBlock, and assert '0 0 0' plus the skip echo — so deleting or rewording the guard fails the suite.
中文说明
[Suggestion] BRANCH == 'main' 跳过——受审 commit 的核心新增——位于被提取的 measureBlock 之内，但没有任何回放定义 BRANCH，因此只有 else 分支会被执行。已在本 commit 做变异验证：把 == 'main' 改成 == 'master'（或整块删除）后整套测试 163/163 全绿——提取正则对缩减后的块仍然匹配，两个既有回放依然通过。触发场景：未来某次编辑移除或重构该跳过（有人"简化" if/else，或 update-ref 变更后该守卫被认为过时）→ fork-main PR 会静默退化为分支与自身比较（永远 0/0，刹车失效）——正是该跳过要防止的回归，却会全绿通过。建议修复：在删除 origin/main ref 之前增加一个回放——fixture 仍有增长量时设置 BRANCH=main 并执行 measureBlock，断言 '0 0 0' 与跳过日志——使删除或改写该守卫会让测试失败。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| REPORT_POSTED='false' | ||
| for attempt in 1 2 3; do |
There was a problem hiding this comment.
[Suggestion] The bounded retry added here protects only one of the two marker-bearing comment posts. The handoff comment (posted at ~5871) carries the autofix-eval watermark (~5861), the conditional autofix-redcheck head marker (~5866), and the gate-rejection block — yet keeps the bare || echo "::warning::". One transient GitHub API failure loses the comment, and every comment-occurrence census (PRIOR_TIMEOUTS ~4470, the PRIOR_HEADS consecutive/cumulative breakers ~5727-5774) goes blind to that round. The retry's own rationale — this one comment carries the round's entire persisted state — applies equally to the handoff comment. Failure scenario: a timeout-cap handoff whose comment is lost to one transient API failure → the breakers never count the round → the loop keeps spending full agent runs on a PR the timeout cap exists to stop; a lost gate-rejection also means the retry agent repeats the refused fix. Suggested fix: extend the same bounded retry to the handoff post (or extract a shared post helper), or amend the retry's comment to state why the handoff site is deliberately warning-only, so the asymmetry is a decision and not an oversight.
中文说明
[Suggestion] 此处新增的有界重试只保护了两个携带标记的评论发送点之一。handoff 评论（约 5871 行发送）携带 autofix-eval 水位（约 5861）、条件性的 autofix-redcheck head 标记（约 5866）与门拒绝块，却仍是裸的 || echo "::warning::"。一次瞬时 GitHub API 故障就会丢失该评论，所有按评论出现次数统计的普查（PRIOR_TIMEOUTS 约 4470、PRIOR_HEADS 连续/累计断路器约 5727-5774）都会对该轮失明。重试自身的理由——这条评论携带该轮全部持久化状态——同样适用于 handoff 评论。触发场景：某次超时上限 handoff 的评论因一次瞬时 API 故障丢失 → 断路器永远计不到该轮 → 循环会继续为一个本应被超时上限拦下的 PR 烧完整的 agent 运行；丢失的门拒绝还意味着重试 agent 重复已被拒绝的修复。建议修复：把同样的有界重试扩展到 handoff 发送（或提取共享发送 helper），或在重试注释中说明 handoff 发送点有意只告警，使该不对称是决定而非疏漏。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| # for base" inside [[ ]], which under an if-condition silently | ||
| # evaluates false — the brake never engages. Both violate the | ||
| # documented fallback promise, so pad-shaped values fall back too. | ||
| if [[ ! "${GROWTH_BUDGET_SRC_LINES}" =~ ^(0|[1-9][0-9]{0,6})$ ]]; then |
There was a problem hiding this comment.
[Suggestion] R4-5 (re-report from round 4; no reply, mechanism unchanged at this commit): the 7-digit cap rejects every well-formed line count ≥ 10,000,000 with a silent fallback to 400, and the pinned rationale — 'past it bash integer literals wrap at 64 bits and comparisons go silently wrong' (test lines ~6008-6009) — is factually wrong. Measured: $(( 10000000 > 400 )) = 1; bash arithmetic is exact to 2^63−1 (~19 digits); the sibling QWEN_TIMEOUT_MS comment (~4654) states the correct 19+ digit threshold. Failure scenario: an operator sets QWEN_AUTOFIX_GROWTH_BUDGET_SRC_LINES=100000000 (well-formed, e.g. to relax the brake for a large-PR campaign) → gets the fallback 400 instead: the brake engages at 400 lines, the opposite of intent, with only a ::warning:: as signal — and the false rationale is pinned into the suite (sanitized('9999999', '10000000') → '9999999 400'), training a future maintainer to reason from wrong arithmetic. Suggested fix: correct the rationale in both the env comment and the test comment (the cap is a policy bound, not a 64-bit guard), and/or widen the cap toward what the arithmetic actually requires (e.g. {0,17} stays below 2^63−1), updating both budgets and the test pin together.
中文说明
[Suggestion] R4-5（第 4 轮重新报告；无回复，机制在本 commit 未变）：7 位上限会把所有 ≥ 10,000,000 的合法行数静默回退为 400，而被钉住的依据——"超过它 bash 整数会在 64 位回绕、比较会静默出错"（测试约 6008-6009 行）——在事实上是错的。实测：$(( 10000000 > 400 )) = 1；bash 算术要到 2^63−1（约 19 位）才不精确；同文件的 QWEN_TIMEOUT_MS 注释（约 4654 行）给出的 19 位以上阈值才是正确的。触发场景：运维设置 QWEN_AUTOFIX_GROWTH_BUDGET_SRC_LINES=100000000（合法值，例如为大 PR 批次放宽刹车）→ 实际得到回退值 400：刹车在 400 行触发，与意图相反，唯一信号是一条 ::warning:: ——且错误依据已被测试钉住（sanitized('9999999', '10000000') → '9999999 400'），未来想修正上限的维护者只能依据错误的算术推理。建议修复：在 env 注释与测试注释中更正依据（该上限是策略限制而非 64 位防护），和/或把上限放宽到算术实际所需（如 {0,17} 仍在 2^63−1 之内），两个预算与测试钉值一并更新。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| NET_TOTAL=0 | ||
| NET_TEST=0 | ||
| else | ||
| NET_TOTAL="$(git diff --numstat origin/main...HEAD -- "${GENERATED_EXCLUDES[@]}" 2> /dev/null | sum_numstat)" || NET_TOTAL=0 |
There was a problem hiding this comment.
[Suggestion] R4-6 (re-report from round 4; no reply, no NET_MEASURED flag added — grep-verified at this commit): the fail-open fallbacks substitute zero nets whenever the three-dot diff fails; the inline comment promises 'an unmeasurable PR skips the brake', but with an existing anchor the zero flows into GROWTH = NET − BASE and can engage the brake on a phantom. Probe-executed against the real extracted wiring block: scenario A — a window anchored at a negative baseline (net-shrinking refactor, src=-500; negatives round-trip through the marker) whose branch is later force-pushed to an orphan history → both measurements fail open to 0 → GROWTH_SRC = 0 − (−500) = 500 > 400 → Critical-only engages with the preamble announcing growth that is factually backwards. Scenario B — an orphan branch anchored at 0 0 at window start later gains a merge base via a merge that posts no base-updated marker → the next round measures the PR's entire net size as growth. The degraded path also emits no signal distinguishing measurement failure from a legitimate zero. Both self-heal only via /retry. Suggested fix (probe-verified in the R4-6 thread): add a NET_MEASURED flag (true normally, false in the || …=0 fallbacks) — gate the growth -gt engagement on it, suppress GROWTH_BASE_NEW/the marker write when false, leave an already-anchored baseline in place without engaging while unmeasurable, and emit a ::warning:: naming the failure.
中文说明
[Suggestion] R4-6（第 4 轮重新报告；无回复，本 commit 经 grep 验证仍未加 NET_MEASURED 标志）：三点 diff 失败时 fail-open 回退把净值替换为零；行内注释承诺"不可测量的 PR 跳过刹车"，但已有锚点时，零会流入 GROWTH = NET − BASE，可能因幻影触发刹车。对真实提取的接线块做了探针执行：场景 A——某窗口基线锚点为负（净瘦身重构，src=-500；负值可经标记往返），其分支随后被强推为孤儿历史 → 两次测量都 fail-open 为 0 → GROWTH_SRC = 0 − (−500) = 500 > 400 → Critical-only 触发，且前言宣称的增长与事实完全相反。场景 B——窗口开始时被接纳的孤儿分支锚定在 0 0，之后某次不发送 base-updated 标记的合并使 merge base 出现 → 下一轮把 PR 的全部净体积测成增长。该退化路径也不发出任何信号来区分测量失败与合法的零。两者都只能通过 /retry 自愈。建议修复（已在 R4-6 线程探针验证）：增加 NET_MEASURED 标志（正常为 true，|| …=0 回退处置 false）——用它门控增长 -gt 触发；为 false 时抑制 GROWTH_BASE_NEW/标记写入；保留已有锚点但不可测量期间不对其触发；并发出一条点名失败原因的 ::warning::。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
The mechanism this re-report targets was removed in 45006cb: fail-open no longer substitutes zeros — NET_MEASURED=false skips anchoring and engagement entirely, so no phantom GROWTH = 0 − BASE can arise. Leaving the thread open only for the residual test-depth ask per the convergence note.
🤖 via Claude Code · model claude-fable-5
Zero-substitution anchored a bogus 0/0 baseline on the window's first round (and manufactured phantom growth against an existing anchor). NET_MEASURED now gates the whole brake: no anchor written, no growth computed, no engagement — for both the no-merge-base and fork-head- named-main cases, which are replayed with the skip line and flag asserted.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...: none — all planned checks completed, including executing the suite.; PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...: I could not execute scripts/tests/qwen-autofix-workflow.test.js — no node_modules in worktree or parent checkout; skipped npm ci for budget reasons. Findings re…; PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...: could not execute scripts/tests/qwen-autofix-workflow.test.js — neither the worktree nor the parent checkout has node_modules , and I skipped a full npm ci ….
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.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...：none — all planned checks completed, including executing the suite.；PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...：I could not execute scripts/tests/qwen-autofix-workflow.test.js — no node_modules in worktree or parent checkout; skipped npm ci for budget reasons. Findings re…；PR #8981 (QwenLM/qwen-code) adds a net-diff growth brake ...：could not execute scripts/tests/qwen-autofix-workflow.test.js — neither the worktree nor the parent checkout has node_modules , and I skipped a full npm ci …。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| 'bash', | ||
| [ | ||
| '-c', | ||
| `GROWTH_BASELINE='${baseline}'\nNET_MEASURED=true\nNET_SRC=${netSrc}\nNET_TEST=${netTest}\n${wiringBlock}\nprintf '%s %s %s %s %s' "$GROWTH_BASE_NEW" "$BASE_SRC" "$BASE_TEST" "$GROWTH_SRC" "$GROWTH_TEST"`, |
There was a problem hiding this comment.
[Suggestion] R6-3: The NET_MEASURED='false' branch of the baseline wiring (workflow ~4139-4142 — BASE zeroed with GROWTH_BASE_NEW staying 'false') is never replayed: this harness hardcodes NET_MEASURED=true, and the measurement replay stops at NET_SRC. The test's own comment claims the invariant ("Unmeasured is a STATE: no bogus anchor may be written") but no assertion checks what the wiring does with the flag. — Failure scenario: mutation-verified at this commit — deleting the unmeasured wiring branch (turning the elif into if) survives the full 163-test suite; replaying the mutated wiring with the unmeasured state's inputs yields GROWTH_BASE_NEW=true BASE_SRC=0 BASE_TEST=0 — the bogus 0/0 anchor the head commit's NET_MEASURED gate exists to prevent. Once the PR later becomes measurable, a round parses the 0/0 baseline, computes growth equal to the full net size, and the brake engages on phantom growth. This is the regression-guard gap on the head commit's core fix. Suggested fix: extend wire() to accept the flag and assert NET_MEASURED=false (empty baseline) → false 0 0 0 0 — no re-anchor, no marker.
中文说明
[Suggestion] R6-3：基线接线的 NET_MEASURED='false' 分支（workflow 约 4139-4142 行——BASE 置零且 GROWTH_BASE_NEW 保持 'false'）从未被回放：此测试壳硬编码 NET_MEASURED=true，而测量回放止于 NET_SRC。测试自己的注释声称了该不变量（"Unmeasured is a STATE: no bogus anchor may be written"），但没有任何断言检查接线对该标志的处理。触发场景：已在本 commit 上做变异验证——删除未测量接线分支（把 elif 变成 if）后全部 163 个测试仍然通过；用未测量状态的输入回放变异后的接线得到 GROWTH_BASE_NEW=true BASE_SRC=0 BASE_TEST=0——正是 head commit 的 NET_MEASURED 门要防止的虚假 0/0 锚点。一旦该 PR 之后变为可测，某一轮会解析出 0/0 基线，算出等于全部净体积的增长，刹车因幻影增长而触发。这是 head commit 核心修复上的回归防护缺口。建议修复：让 wire() 接受该标志，并断言 NET_MEASURED=false（空基线）→ false 0 0 0 0——不重锚、不写标记。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| | map(select(.win == $key)) | ||
| | map(select($baseupd == "" or (.at > $baseupd))) | sort_by(.at) |
There was a problem hiding this comment.
[Suggestion] R6-4: Anchor invalidation keys on autofix-base-updated comment timestamps, but an anchor measured BEFORE a base update and POSTED after it survives this .at > $baseupd filter — the window then compares growth across a moved merge base; manual "Update branch" posts no marker at all. — Failure scenario: the stale-base update-branch lives in review-scan, whose concurrency group degrades to per-run-id on the scheduled lane (empty pr_number), so it is not serialized with review-address there. A scheduled scan merges main and posts the marker at t_u during an address round; if that lands after prepare's measurement but before the window's first report post (anchor time t_r > t_u), the anchor passes the filter and first-wins pins it for the window. Growth is then measured against the moved merge base — systematically under-counted — so the brake silently never engages on a genuinely bloating PR, with no log line to explain it. A maintainer clicking "Update branch" manually posts no marker, so BASE_UPD_AT never moves for that path. Suggested fix: stamp the geometry into the marker instead of trusting comment timestamps — add base=$(git rev-parse "$(git merge-base origin/main HEAD)") at write time, and drop anchors at read time whose recorded base differs from the live git merge-base origin/main HEAD; this closes the scan race, the conflict-round residual, and manual update-branch with one check.
中文说明
[Suggestion] R6-4：锚点失效只认 autofix-base-updated 评论的时间戳，但在 base 更新之前测量、在其之后发布的锚点会通过本 .at > $baseupd 过滤器——窗口随后跨着已移动的 merge base 比较增长；手动 "Update branch" 则根本不发标记。触发场景：stale-base update-branch 位于 review-scan，其并发组在 schedule 通道（pr_number 为空）退化为按 run-id，因此该通道上与 review-address 不串行。一次 scheduled scan 在 address 轮进行期间合并 main 并于 t_u 发出标记；若它落在 prepare 测量之后、窗口首个报告发布之前（锚点时间 t_r > t_u），锚点通过过滤器并被 first-wins 在整个窗口内钉死。此后增长相对已移动的 merge base 测量——系统性缩水——刹车对真实膨胀的 PR 静默永不触发，日志中也没有任何线索。维护者手动点 "Update branch" 不发标记，BASE_UPD_AT 对该路径永不移动。建议修复：把几何信息钉进标记而非信任评论时间戳——写入时附加 base=$(git rev-parse "$(git merge-base origin/main HEAD)")，读取时丢弃记录 base 与实时 git merge-base origin/main HEAD 不一致的锚点；一个检查同时关闭 scan 竞态、冲突轮残差与手动 update-branch。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| NET_TOTAL="$(git diff --numstat origin/main...HEAD -- "${GENERATED_EXCLUDES[@]}" 2> /dev/null | sum_numstat)" || NET_MEASURED='false' | ||
| NET_TEST="$(git diff --numstat origin/main...HEAD -- "${TEST_PATHSPEC[@]}" "${GENERATED_EXCLUDES[@]}" 2> /dev/null | sum_numstat)" || NET_MEASURED='false' | ||
| fi | ||
| NET_SRC=$(( NET_TOTAL - NET_TEST )) |
There was a problem hiding this comment.
[Suggestion] R6-6: A file moved across the test/source classification boundary can manufacture phantom growth in one budget dimension without any new code: NET_SRC = NET_TOTAL − NET_TEST with NET_TEST classified by HEAD-side paths only. — Failure scenario: probe-verified — a round moves a 500-line src/__tests__/helper.ts into production src/ with enough edits to fall below rename similarity, so numstat records delete+add (0 500 + 500 0) instead of a rename: NET_TOTAL stays 0, NET_TEST = −500, hence NET_SRC = +500 — phantom source growth exceeding the 400-line budget engages Critical-only mode with zero real growth, closing the suggestion channel for the rest of the window. The bias is one-directional (false engagement, never a missed trip), recovery is @qwen-code /retry (re-anchors), and a pure git mv with unchanged content is exempt (rename rows report 0/0). Suggested fix: classify each numstat row by both endpoints (old and new path) and neutralize cross-category moves — drop such rows from both nets (or count only the edit delta); alternatively accept the artifact and note in the window preamble that cross-boundary moves consume budget.
中文说明
[Suggestion] R6-6：跨越测试/源码分类边界的文件移动可以在没有任何新代码的情况下，在某个预算维度上制造幻影增长：NET_SRC = NET_TOTAL − NET_TEST，而 NET_TEST 仅按 HEAD 侧路径分类。触发场景：已探针验证——某一轮把一个 500 行的 src/__tests__/helper.ts 移入生产 src/，且改动量足以跌破 rename 相似度，numstat 记为删除+新增（0 500 + 500 0）而非 rename：NET_TOTAL 仍为 0，NET_TEST = −500，于是 NET_SRC = +500——幻影源码增长超过 400 行预算，在零真实增长的情况下进入 Critical-only 模式，关闭窗口余下时间的建议通道。偏差是单向的（只会误触发、不会漏触发），恢复手段是 @qwen-code /retry（重锚），内容不变的纯 git mv 豁免（rename 行报 0/0）。建议修复：按两端路径（旧路径与新路径）分类每条 numstat 记录并中和跨类移动——将此类记录从两侧净值中剔除（或只计编辑增量）；或接受该假象并在窗口前言中说明跨界移动会消耗预算。
— qwen3.8-max via Qwen Code /review (v0.21.10)
- A local head branch literally named 'origin/main' shadows the remote ref in rev disambiguation, so the measurement would silently self-compare with NET_MEASURED still true — guard it alongside 'main'. - The unmeasured state now announces itself instead of printing the same 0/0 line as a genuinely empty PR. - SKILL: the batch-budget sentence is scoped to round-threshold engagements, matching the workflow's cause-aware preamble. R6-1 (.gitattributes steering numstat) is declined in-thread: the brake is takeover-quality tooling on the accountability axis — a collaborator with push access holds overt equivalents (removing the label), and a .gitattributes flip is itself a visible diff.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "PR #8981 adds a net-diff growth brake to the autofix…": I couldn't run the new vitest test itself to measure actual suite-time impact (because node_modules isn't installed in this worktree and npm install is dispropo…; "PR #8981 adds a net-diff growth brake to the autofix…": I couldn't run the new vitest test to measure actual wall-clock suite-time impact (because node_modules is absent, and installation was out of budget). Instead …; "PR #8981 adds a net-diff growth brake to the autofix…": could not execute the new vitest test to measure its actual suite-time contribution — node_modules is absent in this worktree and a full monorepo install was ou…; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "PR #8981 adds a net-diff growth brake to the autofix…": none — I completed every check I planned..
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.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："PR #8981 adds a net-diff growth brake to the autofix…"：I couldn't run the new vitest test itself to measure actual suite-time impact (because node_modules isn't installed in this worktree and npm install is dispropo…；"PR #8981 adds a net-diff growth brake to the autofix…"：I couldn't run the new vitest test to measure actual wall-clock suite-time impact (because node_modules is absent, and installation was out of budget). Instead …；"PR #8981 adds a net-diff growth brake to the autofix…"：could not execute the new vitest test to measure its actual suite-time contribution — node_modules is absent in this worktree and a full monorepo install was ou…；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"PR #8981 adds a net-diff growth brake to the autofix…"：none — I completed every check I planned.。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ], | ||
| { encoding: 'utf8', cwd: dir }, | ||
| ); | ||
| expect(forkMain.split('\n').pop()).toBe('false 0'); |
There was a problem hiding this comment.
[Suggestion] R7-2: The R6-2 fix at this commit widens the shadow guard to if [[ "${BRANCH}" == 'main' || "${BRANCH}" == 'origin/main' ]] (workflow ~4104), but the suite replays the measurement block only with BRANCH=main (the forkMain invocation above); no replay defines BRANCH=origin/main. Mutant-verified: deleting the || "${BRANCH}" == 'origin/main' disjunct leaves the suite green (163/163 both ways); replaying the extracted block with BRANCH=origin/main in the shadowing fixture flips between the mutant (true 0 0 — bogus self-compare, NET_MEASURED stays true) and the original (false 0 0 — loud skip). The disjunct added specifically to close round 6's Critical is pinned by nothing. — Failure scenario: a future edit drops the disjunct and the suite stays green; a managed fork head literally named origin/main then shadows refs/remotes/origin/main in rev disambiguation, git diff origin/main...HEAD silently self-compares, nets read 0/0 forever, the baseline anchors at zero, and the growth brake can never engage for that PR — unbounded bloat with no signal, the exact outcome the skip exists to prevent.
| expect(forkMain.split('\n').pop()).toBe('false 0'); | |
| expect(forkMain.split('\n').pop()).toBe('false 0'); | |
| // A fork head literally named 'origin/main' shadows the remote ref in | |
| // rev disambiguation — same loud skip as BRANCH=main. | |
| const forkOriginMain = execFileSync( | |
| 'bash', | |
| [ | |
| '-c', | |
| `set -eo pipefail\nBRANCH=origin/main\n${measureBlock}\nprintf '\\n%s %s' "$NET_MEASURED" "$NET_TOTAL"`, | |
| ], | |
| { encoding: 'utf8', cwd: dir }, | |
| ); | |
| expect(forkOriginMain.split('\n').pop()).toBe('false 0'); |
中文说明
[Suggestion] R7-2：本 commit 的 R6-2 修复把守卫扩展为 if [[ "${BRANCH}" == 'main' || "${BRANCH}" == 'origin/main' ]]（workflow 约 4104 行），但测试里只有 BRANCH=main 的回放（上方 forkMain 调用），没有任何回放定义 BRANCH=origin/main。已做突变体验证：删除 || "${BRANCH}" == 'origin/main' 这一支后套件仍然全绿（两种情况均为 163/163）；在影子夹具上以 BRANCH=origin/main 回放提取出的测量块，突变体与原代码结果翻转（突变体 true 0 0 —— 虚假自比、NET_MEASURED 保持 true；原代码 false 0 0 —— 显式跳过）。专门为关闭第 6 轮 Critical 而新增的这一分支目前没有任何测试钉住。触发场景：未来某次编辑删掉该分支而套件依旧全绿；名为 origin/main 的托管 fork head 会在 ref 消歧时遮蔽 refs/remotes/origin/main，git diff origin/main...HEAD 静默自比，净值永远 0/0，基线锚定在零上，增长刹车对该 PR 永不触发——无信号无限膨胀，正是该跳过分支要防止的结果。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 121 passed · 0 failed · 121 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：121 通过 · 0 失败 · 121 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 121 passed · 0 failed · 121 total (harnesses h1–h9); targeted suite 163/163 green at head; mutation matrix 10/13 killed (3 non-blocking survivors). 中文 — 判定：✅ 通过 · 可合入（agent 判定）· 跟进轮跟进轮沙箱验证（head
Verification reportVerification report — PR #8981 (feat(autofix): brake review-round diff growth with per-window src/test budgets) — FOLLOW-UP ROUNDVerdict: This is a follow-up round. The previous round verified head Previous-finding status (carried forward from the round at
|
| # | Finding (round at 92a075b) | Severity | Status at 2d71a0f8 |
|---|---|---|---|
| 1a | M8 survivor: ZH budget parenthetical not pinned | Informational | FIXED (stands fixed) — the batch-budget sentence is conditional on the engagement cause and replayed in both languages (h2 + suite budgetFor); re-measured, still pinned. |
| 1b | M7 survivor: 📏 log wording not pinned |
Informational | STANDS — re-measured as mutant M12: rewording the 📏 net diff … line survives the two affected tests and the full 163 suite. Display-only; the numbers are exercised by the harnesses. Not a merge condition. |
| 2 | Non-npm lockfile churn (yarn.lock, pnpm-lock.yaml) counts as source growth |
Informational (declined-with-rationale) | STANDS, decline agreed — GENERATED_EXCLUDES unchanged and deliberately exact-named. My independent h3 fixture includes yarn.lock/pnpm-lock.yaml decoys and confirms they ARE measured as source (+5), as documented. |
| 3 | Working tree carried a pre-existing local modification reverting the PR's SKILL.md wording | Environmental | STANDS — still present at the new head (git status: M .qwen/skills/autofix/SKILL.md). Left untouched; every block under test was extracted from git show HEAD: blobs, so the verified artifact is the submitted PR. |
Declined/deferred rows were re-measured, not diffed: M12 re-applied at the new head; GENERATED_EXCLUDES re-exercised through the h3 fixture; SKILL.md residue re-inspected.
Central claim + A/B (re-measured at the new head and new base)
Central claim: when either src or test net-diff growth beyond the per-window baseline exceeds its budget, Critical-only mode engages early (before the round threshold), reusing the round brake's deferral machinery, and the deferred preamble names the actual cause.
Blocks sliced verbatim (js-yaml-parsed run: scripts, single-match anchors) from head and base, executed under the step's shell contract (bash --noprofile --norc -eo pipefail, the workflow's defaults.run.shell: bash).
| Cell (ROUND, growth src/test, budgets 400/400) | HEAD CO ROUNDS GROWTH |
BASE CO |
Witness |
|---|---|---|---|
| r2, 500/12 (src trip, early round) | true false true |
false |
evidence/01-decision-ab.png |
| r2, 400/400 (AT budget) | false false false |
false |
boundary exclusive, both arms |
| r2, 401/0 · r2, 0/401 | true false true |
false |
either dimension trips |
| r2, −900/−900 | false false false |
false |
shrink never trips |
| r5, 0/0 | true true false |
true |
round brake preserved (A/A control) |
| r5, 401/0 | true true true |
true |
both causes coexist |
| r4, 400/400 · r4, 0/401 · r0, 0/0 | as expected | as expected | 10 cells × 2 arms = 20/20 (h1) |
Deferred-preamble half (14/14, h2): growth-only at r2 emits the growth clause with no rounds clause and the conditional budget sentence ("Maintainer feedback continues to flow unaffected…" / "…照常流动"); rounds-only and both-causes shapes correct in both languages; the deferred item list (review + inline + PR-comment fixtures, Critical/bot/marker//retry excluded) is byte-identical head vs base — the mechanism is reused unchanged. Base positive control renders the old round-only wording at r5; base at r2 with growth 500 writes no deferred section at all — the load-bearing contrast.
Delta verification — commits 149b8a19, f0f224cb, 45006cb5, 2d71a0f8
The checkout is depth-2, so the previous head 92a075b is not locally available; each new commit's claims were verified against the head code and behavior:
- Stale-base anchor invalidation (
149b8a19) — VERIFIED. TheBASE_UPD_ATjq picks the latestautofix-base-updatedcreated_at;GROWTH_BASELINEthen drops anchors with.at <= $baseupd(strict>). h5 replays the real jq end-to-end: post-update anchor survives, pre-update dropped (200 80); all-pre-update → empty (re-anchor); two updates → only anchors after the LATEST survive; same-second anchor invalidated (strict>). Mutant M6 (drop the filter) KILLED by the suite (expected '50 -60' to be '100 200'). - 7-digit sanitize cap (
149b8a19) — VERIFIED.9999999passes,10000000falls back to 400 (h6). Rationale demonstrated on unguarded bash:[[ 300 -gt 0400 ]]ENGAGES (octal 256),[[ 1000 -gt 0900 ]]errors and compares false, and an uncapped 20-digit literal wraps negative at 64 bits making500 > 10^19TRUE. Mutant M5 (cap{0,7}) KILLED. - Three-dot (merge-base) pin (
149b8a19) — VERIFIED on an independent fixture (h3): advancing main past the divergence leaves the nets unchanged (three-dot), while a two-dot diff of the same tips differs — so the fixture discriminates and pins merge-base semantics. - Fork head named
main(f0f224cb) — VERIFIED (h4):BRANCH=mainskips measurement out loud, zero nets,NET_MEASURED=false, exit 0. - Unmeasurable is a STATE (
45006cb5) — VERIFIED (h4 + h7): orphan (no merge base), fork-main, and shadowed-origin/mainall yield zero nets withNET_MEASURED=false, write no anchor (GROWTH_BASE_NEWstaysfalseeven against an existing baseline — no phantom growth), zero the growth, and still exit 0 under-eo pipefail. Mutant M13 (flip theNET_MEASUREDgate) KILLED. - R6 shadowed-base guard + loud skip (
2d71a0f8) — VERIFIED (h4): a LOCAL branch literally namedorigin/mainresolves overrefs/remotes/origin/main(shadow proven viarev-parse), so the guard is load-bearing;BRANCH=origin/mainskips out loud. The loudUNAVAILABLE/shadowsannouncements print, and the[[ ]] && echopattern does not abort the step when measurement succeeded (set -e).
Corrections
None needed — every mechanism claim in the description and the new commits' messages reproduced exactly as stated.
Findings
No blocking findings. Informational:
- M10 survivor: the R6
origin/mainshadow guard is not pinned by the suite (coverage gap, Suggestion). Removing|| "${BRANCH}" == 'origin/main'leaves all 163 tests green — the suite only exercisesBRANCH=main. The guard is nonetheless correct and load-bearing: h4 proves a local branch namedorigin/mainshadows the remote ref (self-compare) and that the guard skips it. If the author wants it pinned, a one-lineBRANCH=origin/mainreplay (mirroring the existingforkMaincell) closes it. Not a merge condition — behavior is verified correct by the harness. - M11/M12 survivors: log wording not pinned (display-only). Rewording the
UNAVAILABLEannouncement (M11) or the📏 net diffline (M12) survives the suite — both are outside the suite'smeasureBlockslice (which ends atNET_SRC). Informational; the loud-skip behavior itself is exercised by h4. - Rename detection is live in the measurement diff (note, not a defect).
git diff --numstat(default rename detection) conserves net line counts; my fixture accidentally produced a rename pair (identical 4-line files) and the production block still accounted it correctly (26/14/12). Only a rename across an excluded-churn boundary after anchoring could under-report growth (a false-negative bounded by the rename size) — pathological and self-correcting on re-anchor. No action needed.
Mutation matrix at the new head (control green: 2 passed | 161 skipped, scratch worktree)
| Mutant | Result | Killing assertion (or survival evidence) |
|---|---|---|
M1 -gt→-ge (decision) |
KILLED | boundary cell flips inclusive |
M2 ||→&& (decision) |
KILLED | both-dimensions-required breaks either-trip |
M3 drop __tests__/ from TEST_PATHSPEC |
KILLED | behavioral: nets shift |
| M4 drop GENERATED_EXCLUDES from NET_TEST side | KILLED | behavioral: test-dir lockfile corrupts NET_SRC |
M5 sanitize cap {0,6}→{0,7} |
KILLED | 10000000 no longer falls back |
| M6 drop baseupd invalidation filter | KILLED | expected '50 -60' to be '100 200' |
M7 wiring net − base→net + base |
KILLED | growth subtraction wrong |
M8 marker key=→win= |
KILLED | census double-attribution + template pin |
M9 retry final-attempt ==3→==4 |
KILLED | giving up / retrying-count break |
M10 drop origin/main shadow half |
SURVIVED (coverage gap) | green 2 passed; load-bearing per h4 |
| M11 reword UNAVAILABLE line | SURVIVED (display) | outside measureBlock slice |
M12 reword 📏 net diff line |
SURVIVED (display) | outside measureBlock slice |
| M13 flip NET_MEASURED wiring gate | KILLED | expected 'false 0 0 120 40' to be 'false 50 -60 70 100' |
No mutant regressed from killed to survived versus the prior round. Witness: evidence/04-mutation-matrix.png.
Reviewer Test Plan walk-through
Every step performed: the behavioral suite runs the Critical-only block across round/growth combinations (boundary exclusive ✓, either dimension ✓, both causes ✓, negative never ✓); the measurement block runs against a real fixture (suite's 22 18 4, my independent 26 14 12 ✓); the baseline jq replays over fixture comments (bot-only, window-keyed, first-wins, negative, spoof/malformed/legacy-win= rejection, empty window, baseupd invalidation ✓); sanitize fallback and cause construction execute ✓; marker write sites / env plumbing / output writes contract-pinned ✓ (evidence/03-measure-fixture.png).
Not covered
- Real
gh pr commentposting and GitHub-sidevars/scan behavior (no credentials; retry replayed with stubgh, real bash, realsleepon the timing cell; jq 1.6 here vs 1.7 on ubuntu-latest). yamllint(no pip) andshellcheck(absent); embedded bash covered bybash -n+ behavioral replay instead.- Per-commit attribution: depth-2 checkout; the aggregate
HEAD^1..HEADdiff was verified and each new commit's enumerated changes verified against head code and behavior. - End-to-end workflow execution on Actions runners;
/retryre-arm lifecycle end-to-end (its mechanism chain verified link by link). This replay reproduces the wire shape the workflow will emit, not a live Actions run. - SKILL.md doc hunk (docs-only, read).
Methodology
Environment: node:22-bookworm CI verify container; merge-ref checkout (HEAD = merge 3a7090ff4e, HEAD^1 = base tip 52cfb18972, HEAD^2 = verified head 2d71a0f851). The changed steps' run: scripts were dumped verbatim via js-yaml from git show HEAD:/git show HEAD^1: blobs (harness/extract.mjs); blocks sliced between single-match anchors (harness/blocks.mjs, validated byte-equivalent to the suite's captures modulo indentation by harness/slice-check.mjs) and executed under the step's shell contract with fixture ic.json/rv.json/rc.json and fresh mktemp git repos carrying refs/remotes/origin/main — never stubbing the code under test. Harnesses h1–h9 emitted TAP (logs/*.tap); the ledger was computed from those logs by harness/tally.mjs (nothing hand-counted) → assertions.json. Mutation matrix applied to a scratch worktree (git worktree add --detach tmp/mut-tree HEAD, since removed); each mutant verified single-occurrence before replacement. Evidence captured with scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Review handling summarySix review rounds, 33 findings total, all dispositioned in-thread: R1 9/9 fixed; R2 8/8 fixed (growth marker moved to a `key=` token no `win=` census can match); R3 3 fixed + 1 declined (census positional attribution queued as its own change); R4-R5 6 fixed incl. making "unmeasurable" a state (`NET_MEASURED`) that skips anchoring entirely instead of zero-substituting; R6 2 fixed + 1 declined with reasoning (.gitattributes steering is an accountability-axis concern with overt equivalents; queued for #8996's footprint classes). Every fix ships with a behavioral replay, not a text pin. Convergence: rounds went 9→8→4→6→7→0 findings; the Critical channel has been empty for two rounds and the latest round reported nothing. Per this PR's own philosophy, remaining open Suggestion threads (test-depth pins, anchor timing, cross-boundary moves) are the deferred follow-up backlog, kept open deliberately. Current CI red is two `shell-ast-parser-lazy` cases on main's code (zero overlap with this diff; #8996 is green on the same areas) — rerun queued. 中文说明六轮评审共 33 条 finding,全部逐条处置:R1 修 9;R2 修 8(growth 标记改用 `key=` token,任何 `win=` 整体匹配统计都无法误配);R3 修 3 拒 1(census 位置化归属排为独立改动);R4-R5 修 6,含把"不可测"改为状态(`NET_MEASURED`)整体跳过锚定,而非置零替代;R6 修 2、有据拒 1(.gitattributes 操纵属问责轴问题,已排入 #8996 足迹类别的 follow-up)。所有修复均配行为回放测试而非文本钉。 收敛:各轮发现数 9→8→4→6→7→0,Critical 通道连续两轮清空,最新一轮零新发现。按本 PR 自身的哲学,剩余 open 的 Suggestion 线程(测试深度、锚点时序、跨界移动)为有意保留的 follow-up 清单。当前 CI 红为 main 代码上的两个 `shell-ast-parser-lazy` 用例(与本 diff 零交集,同区域的 #8996 全绿),已重跑。 🤖 via Claude Code · model `claude-fable-5` |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed head 2d71a0f (full 3-file diff + the round-4→7 finding thread). Approving — no P0/P1 at this commit.
Verified against the diff:
- Counting: three-dot
git diff --numstat origin/main...HEAD(merge-base semantics pinned by a fixture that advances main), TEST_PATHSPEC (*.test.*/*.spec.*/__snapshots__/__tests__/test-utils/rootintegration-tests), GENERATED_EXCLUDES applied symmetrically to BOTH nets soNET_SRC = NET_TOTAL − NET_TESTstays clean (fixture'sintegration-tests/package-lock.jsonlands in neither; math checks at 22/18/4). - Unmeasurable is a STATE: fork heads named
mainororigin/main(R6-2 guard, ~4104) and no-merge-base PRs setNET_MEASURED=false— no anchor, no marker, no engagement. This retires both the bogus-0/0-anchor and the phantom0 − BASEgrowth shapes. - Marker lifecycle: read keyed on
LIVE_REARM_KEY(not matrix WINDOW — supersede-exempt conflict rounds land under the live key), bot-login-only, first-wins viasort_by(.at)|.[0], anchors older than the latestautofix-base-updatedinvalidated, writer↔scanner round-trip pinned (incl. negative nets),key=spelling cannot match thewin=window censuses. - Brake semantics: growth only sets
CRITICAL_ONLY→ reuses the round brake's deferral machinery; Critical / changes-requested / maintainer / failed-checks / conflict channels untouched. Cause preamble names the right cause in both languages across all three engagement shapes (no+-artifact, replayed), and the batch-budget sentence is scoped to round-threshold engagements only — matching what the OVER_BUDGET census actually enforces. - No wedge:
/retry/ takeover re-engage opens a fresh window → fresh anchor → growth 0; fail-open skips the brake rather than dying red; under-count at worst restores the pre-PR status quo. No miscount direction can silence Critical-channel feedback. - Report-post retry (3 attempts, final logs "giving up", then
exit 1) protects the round's persisted state; replayed against a stubbedghfor success-once and full-outage paths. - R6-1 decline (
.gitattributes binaryas a self-service off-switch) accepted as a maintainer judgment call: this is accountability/cost tooling, not a security boundary; the failure direction is under-count → brake doesn't engage → status quo; push-holders have overt equivalents (label removal); follow-up tracked for #8996.
Nits (non-blocking):
- R7-2 stands at this commit: no replay defines
BRANCH=origin/main, so deleting that disjunct ships green — a one-line replay addition would pin it. - R4-5 residual: the pinned rationale for the 7-digit cap ("bash integer literals wrap at 64 bits") is factually wrong (wrap is near ~19 digits). Behavior is safe (fallback + warning), but the comment should say "conservative cap", not a false mechanism.
- R5-2 residual: the handoff comment post still lacks the bounded retry the report post gained (P3 follow-up).
:(glob)integration-tests/**is root-anchored while the other test pathspecs match at any depth — intentional per the comment, just worth knowing when tuning.
CI at 2d71a0f: Test (ubuntu-latest, Node 22.x) green (this is the suite running the new workflow test), 8 success / 90 skipped (path-filtered), route cancelled (infra), web-shell E2E Smoke still in progress — same family as the known web-shell flake, and this PR touches no web-shell code; not gating.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR #8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
…QwenLM#9104) QwenLM#8981's growth brake trims non-Critical feedback once a window's diff grows past budget, but when the growth is Critical-driven (a complex feature whose every fix opens the next fail-open gap the reviewer then flags — e.g. PR QwenLM#8777, 8 rounds, 13k additions) Critical-only cannot help: the Criticals ARE the growth, so the diff keeps climbing and the agent keeps patching. Two additions on the autofix side: - Feed the growth trajectory to the agent. feedback.md now opens with a "Diff growth this window" section (net src/test vs budget + how many prior rounds were over budget) whenever growth is measured, telling the agent to prefer minimal/subtractive fixes and to read a rising trajectory as a signal to escalate for a split, not add another guard. - Detect divergence and hand off. A new per-round autofix-growth-now marker records each round's growth + over-budget flag; prepare reads the window's history and, once the brake has been over budget for >= GROWTH_DIVERGENCE_ROUNDS prior rounds (default 2, tunable) AND the diff has not shrunk from its worst, injects a "Needs a maintainer's decision — this PR is not converging" block. It is framed as a defer-to-human item, so the address run stops BLOCKED with a handoff (split / accept core + track the tail / redesign) instead of patching again. A diff that is over budget but shrinking, or a one-off overshoot, stays in ordinary Critical-only. SKILL.md documents both blocks. Contract tests pin the knob, run the extracted divergence detector against fixture history (climbing → diverged, shrinking → not, sub-threshold → not, wrong-window → not), and assert the growth-now marker is written on both report paths.
|
Released in v0.21.12. |
…ensuses (QwenLM#9156) * feat(autofix): deny-by-default footprint gate, positional window censuses, review-loop backlog Follow-up to QwenLM#8981/QwenLM#8996, closing the structural causes behind their review-round non-convergence: - Deny-by-default footprint: every file a round touches maps to an AREA (declared workspace, else top-level directory, else the root file itself); areas outside the PR's own footprint are surfaced in a gate-authored advisory, or rejected retryably once the repo variable QWEN_AUTOFIX_FOOTPRINT_ENFORCE is staged to 'reject'. The enumerated class gate keeps rejecting regardless — this inverts the default for everything it cannot enumerate (a denylist is not a boundary). - The three window censuses (PRIOR_TIMEOUTS, WIN_HEADS, PRIOR_HEADS) attribute comments positionally over their own scan-parsed eval markers instead of whole-body win= substrings: a neutralized marker quoted in a handoff excerpt, or any future marker embedding win=, can no longer double-attribute a comment (decoy fixture included; the census fixture's non-numeric round= placeholder is corrected). - BITE_ENFORCE's reply arm inherits the thread root's CHANGES_REQUESTED membership, not just its body tag. - Backlog tests: the bite restore-failure crash contract (verdict-less exit with the rejection document, driven by a ref-deleting runner), merge-base-anchored footprint compares under an advanced main (afterPr fixture hook), and the shrink+bite advisory append order. - SKILL: cap each round's implemented batch (~8 findings, Critical first, defer the rest via comment-replies) — nine review rounds of evidence that oversized fix batches breed fix-of-fix defects — and document the footprint gate. * fix(autofix): close the R1 footprint-gate findings - Advisory lifecycle: one reset at gate start, every writer appends — the footprint advisory no longer dies to the shrink section's rm or its truncating write. - Footprint membership is REF-ANCHORED: areas derive from the pre-round root manifest's workspaces globs (longest ancestor wins, nested workspaces correct), so a round cannot redefine its own boundary and the on-disk resolver is out of this path entirely; non-workspace paths under packages/ keep two segments so sibling projects stay distinct areas; emitted areas are newline-sanitized against phantom footprint grants. - The enforcement knob rides step-level env at both verify gates — $GITHUB_ENV writes from earlier steps cannot downgrade 'reject'. - TESTSIDE's critical() mirrors cr_attached (root and self), keeping enforcement and demotion on one comment set. - Census ownership is LAST-WINS over scan-parsed markers (a stray quoted-or-appended marker cannot double-attribute), the replay decoy is now genuinely discriminating (old whole-body → 0, new → 1), and the growth-gate comment stops citing retired whole-body matchers. Queued per the batch cap: per-line advisory bullets and the third sink charset, discriminating fixtures at the two remaining census sites, the reply-arm bite fixture, freight and merge-base footprint fixtures, and digest-pinning the staged resolver for its remaining consumers. * fix(autofix): close the R2 footprint-gate findings - list_areas reads and translates the ref's workspaces globs ONCE per invocation and matches ancestors in-bash (was ~21ms git+jq+sed per file×ancestor call), emits printf %q keys — line-safe AND injective, so distinct areas can never collapse into one comparison key — and both render arms print one bullet per area from those keys. - Producer failures are a STATE: a failed round- or PR-side diff (orphan history, transient git error) skips the footprint check loudly instead of shrinking one side into a verdict. - The workflow-level FOOTPRINT_ENFORCE env is gone (the step-level pins are the only consumers and outrank it — dead config removed); the two step wirings are count-pinned. - Fixtures: nested-workspace membership discriminates against the packages/ two-segment fallback (sibling nested workspaces stay distinct areas), and the advisory-lifecycle discriminator proves an earlier section's advisory survives the shrink section. Queued: consolidating the six eval-marker regex variants behind one grammar constant (touches six jq programs; its own change).
3 participants
What this PR does
Adds a net-diff growth brake to the autofix review loop. The first round of each counting window records the PR's net size (insertions − deletions vs the merge base) as a baseline marker in its report comment, split into test lines (
*.test.*/*.spec.*files,__snapshots__/,__tests__/,test-utils/,integration-tests/) and source lines, with mechanical churn (lockfiles, the regenerated settings schema) excluded from both sides. When either dimension's growth beyond the baseline exceeds its budget (vars.QWEN_AUTOFIX_GROWTH_BUDGET_{SRC,TEST}_LINES, default 400 each), Critical-only mode engages early, reusing the round brake's entire deferral machinery — Critical findings, Request changes reviews, in-budget maintainer feedback, failed checks, and conflict resolution keep flowing; only the suggestion channel stops. The deferred-feedback preamble names the actual cause (round threshold, growth budget, or both, bilingual), and@qwen-code /retryor re-engaging takeover re-anchors the baseline with the fresh window.Why it's needed
Managed PRs bloat while still under the round threshold. Measured on the current takeover pool: #8853 grew 399 → 1490 lines in a day and a half (86% of the growth was test lines; one "harden per review feedback" round alone added +609), #8276 grew ~2700 net lines under management (78% tests), and 12 of the 27 currently managed PRs exceed 4000 additions. The mechanism is a feedback loop — every push triggers a full re-review, the reviewer produces findings on code the previous round just added, and the next round implements them — which
CRITICAL_ONLY_AFTER_ROUND=5cannot bound because a single round can add hundreds of lines, and every window re-arm reopens five suggestion-capable rounds. Two budgets rather than one because the bloat concentrates in tests: a single budget is effectively spent by test growth and cannot be tightened on tests without also strangling source fixes.Reviewer Test Plan
How to verify
Run the behavioral suite, which replays every new block against real fixtures rather than text-pinning it:
npm run test:scripts(or the single file:npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow). The relevant cases: the Critical-only mode block executes under bash across round/growth combinations (boundary exclusive at budget, either dimension trips, both causes coexist, negative growth never engages); the measurement block runs against a real fixture repo (source rewrite, appended tests, spec/snapshot/__tests__/test-utils/integration-tests files, a binary file, a root lockfile, a nested lockfile, a lockfile under a test directory, the generated schema path) and yields the expectedtotal/test/src = 22/18/4split; the baseline jq parse replays over fixture comments (bot-only, window-keyed, first-wins, negative nets, spoofed-marker rejection, empty window); the budget-sanitize fallback and the bilingual cause construction are executed, not pinned. Marker write sites, env plumbing, and output writes are contract-pinned.Evidence (Before & After)
N/A — CI workflow logic; behavior is exercised by the replayed suite above. Before: a takeover PR accumulates suggestion-driven growth for five change-producing rounds per window with no size bound (#8853: 3.7× in ~30 h). After: growth past the per-window budget defers non-Critical suggestions with a preamble naming the measured growth and budgets.
Tested on
Environment (optional)
Unit tests only (
npm run test:scripts); the workflow itself runs on the repo's Actions runners.Risk & Scope
@qwen-code /retry(fresh window, baseline re-anchored at current size), and everything Critical-only preserves still flows. Budgets are repo variables — tunable without a code change, with malformed values (including zero-padded octal-shaped ones) falling back to the default at the read site.<!-- autofix-growth-base … -->rides the existing report comment (its own marker,autofix-redcheck-style; the positionalautofix-evalparsers are untouched). Rounds report their measurement in the job log (📏 net diff …) every run.Linked Issues
None — follow-up to the takeover-bloat investigation on the current managed pool (#8853 and #8276 used as measurement samples only).
中文说明
本 PR 做了什么
为 autofix 评审循环增加净 diff 增长刹车。每个计数窗口的首轮把 PR 的净体积(相对 merge base 的插入 − 删除)作为基线标记写入其报告评论,拆分为测试行(
*.test.*/*.spec.*文件、__snapshots__/、__tests__/、test-utils/、integration-tests/)与源码行,机械性变更(lockfile、再生成的 settings schema)在两侧都不计入。任一维度相对基线的增长超过预算(vars.QWEN_AUTOFIX_GROWTH_BUDGET_{SRC,TEST}_LINES,各默认 400)时提前进入 Critical-only 模式,完整复用轮次刹车的延后机制——Critical findings、Request changes、预算内维护者反馈、失败 checks、冲突解决照常流动,只停建议通道。deferred 前言双语点名实际触发原因(轮次阈值、增长预算或两者),@qwen-code /retry或重挂 takeover 会随新窗口重新锚定基线。为什么需要
托管 PR 在轮次阈值之内就已严重膨胀。对当前 takeover 池的实测:#8853 一天半内从 399 行涨到 1490 行(增长的 86% 是测试行;单轮 "harden per review feedback" 就 +609),#8276 托管期间净增约 2700 行(78% 为测试),27 个在管 PR 中 12 个超过 4000 行 additions。机制是正反馈环——每次 push 触发全量重审,reviewer 对上一轮刚加的代码再产 findings,下一轮继续实现——
CRITICAL_ONLY_AFTER_ROUND=5拦不住它,因为单轮就能加几百行,且每次窗口重开都再送五个可实现建议的轮次。用两个预算而非一个,是因为膨胀集中在测试:单一预算实际会被测试增长耗尽,想对测试收紧就必然连源码修复一起勒死。评审验证方案
如何验证
运行行为测试套件(所有新增块都在真实 fixture 上回放执行而非仅文本钉住):
npm run test:scripts(或单文件:npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow)。相关用例:Critical-only 模式块在 bash 下按轮次/增长组合真实执行(预算边界排他、任一维度可触发、两因可并存、负增长不触发);测量块在真实 fixture 仓库上运行(源码重写、追加测试、spec/snapshot/__tests__/test-utils/integration-tests 文件、二进制文件、根级与嵌套 lockfile、测试目录下的 lockfile、生成 schema 路径),得到预期total/test/src = 22/18/4拆分;基线 jq 解析在 fixture 评论上回放(仅 bot、按窗口、first-wins、负值、伪造标记拒绝、空窗口);预算清洗回退与双语原因组句为真实执行而非钉文本。标记写点、env 传递与输出写入以契约钉住。证据(Before & After)
N/A —— CI workflow 逻辑;行为由上述回放套件覆盖。Before:takeover PR 每窗口五个产生改动的轮次内建议驱动增长无体积上限(#8853 约 30 小时 3.7 倍)。After:超过窗口预算后非 Critical 建议被延后,前言写明实测增长与预算。
测试平台
环境(可选)
仅单元测试(
npm run test:scripts);workflow 本体运行于仓库 Actions runner。风险与范围
@qwen-code /retry(新窗口、按当前体积重锚基线),且 Critical-only 保留的通道全部照常。预算为仓库变量,免代码调参;非法值(含零填充的八进制形态)在读取处回退默认。<!-- autofix-growth-base … -->附于现有报告评论(独立标记,autofix-redcheck模式;位置敏感的autofix-eval解析器不动)。每轮在 job 日志打印测量(📏 net diff …)。关联 Issue
无——源于对当前托管池的 takeover 膨胀调查(#8853 与 #8276 仅作为测量样本引用)。