feat(autofix): run one bounded self-review before an address-review commit (A/B) - #11516
Conversation
…ommit (A/B) Measured on the takeover fleet (40 PRs, 79 acted rounds, 1369 inline findings blamed at their review heads): after a round pushes, 73% of the next review's new Criticals and 93% of its Suggestions sit on that round's own delta, while Critical density is the same on bot and human deltas (~2 per review, no decay across rounds). So a fresh adversarial pass over the delta before the push has the right scope, and ONE bounded pass, never a loop, is the right shape. Armed by the QWEN_AUTOFIX_SELF_REVIEW repo variable (off|ab|on; ab splits on PR parity), resolved once in prepare and never on a github-hosted runner. An armed round gets a 180m agent budget under a per-arm step cap (190/130), skips the same-run repair chain so the 345-minute job bound still holds, and skips the pass below 150 changed lines or 75 minutes left. The skill runs the review inside the session's own sandbox and records self-review.json after its commit; the gate validates the record, binds it to the pushed commit's tree id and publishes one token string that the round report renders as an autofix-self-review marker. Advisory only: nothing rejects. Design record: qwen-autofix.md#af-156.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
The self-review arm added 4,978 bytes to qwen-autofix.yml, past the 4,096-byte ratchet, with 3,113 of them comment prose that af-156 already carries. Compress those comments to pointers; the mechanism is unchanged.
|
Thanks for the PR — the measurement behind this is unusually concrete, which makes it easy to review. Template — complete ✓ (all headings, Risk & Scope filled in, bilingual body). Problem — observed, not theoretical. The claim is a fleet measurement (40 takeover PRs, 79 acted rounds, 1369 inline findings blamed at their review head) rather than a hypothesis: 73% of the next review's new Criticals land on the round's own delta, and a pushed round costs ~870 min median end to end. That is the right kind of evidence for a workflow change. One honest caveat — the measurement is not reproducible from anything in this PR (no script, no artifact, no query), so I am taking the numbers on your word. That is fine for a maintainer-authored, default-off experiment, but the A/B is only worth running if the baseline numbers can be re-derived later. Direction — this is the repo's own autofix machinery, not the shipped product, so the ecosystem-CHANGELOG signal does not apply. The design record follows the established Size — no core paths touched ( Separately, and this one is worth your attention: the first commit tripped the workflow-size ratchet ( Approach — the scope feels right, and the one-pass reasoning is the strongest part: critical density is 0.53 per 100 changed lines on bot deltas against 0.52 on human ones, so a fresh reviewer yields ~2 Criticals per delta regardless of who wrote it. That genuinely rules out "review until clean" inside a bounded budget, and I would have reached the same shape. Three questions before the code review:
Risk — no elevated risk signals: none of the changed files match the revert-correlated path set, and this is same-repo (not a fork), Moving on to code review. 🔍 中文说明感谢贡献——这个 PR 背后的度量非常具体，评审起来很容易。 模板 —— 完整 ✓（所有小标题齐全，Risk & Scope 有实际内容，正文双语）。 问题 —— 是已观测到的，不是理论性的。依据是一次车队级度量（40 个 takeover PR、79 个 acted 轮、1369 条行内发现，每条在其评审 head 上 blame 归因），而不是假设：下一次评审的新 Critical 有 73% 落在该轮自己的 delta 上，一个推送轮的端到端中位成本约 870 分钟。对于工作流改动来说这是正确的证据类型。一个诚实的保留意见——这份度量无法从 PR 里的任何东西复现（没有脚本、没有产物、没有查询语句），所以这些数字我只能采信你的说明。对于一个由维护者提交、默认关闭的实验来说这没问题，但 A/B 只有在基线数字日后能重新推导出来时才值得跑。 方向 —— 这是仓库自身的 autofix 机制，不是发布的产品，所以生态 CHANGELOG 信号不适用。设计记录沿用了工作流同级 规模 —— 未触及核心路径（ 另有一点值得你注意：第一个提交触发了工作流体积棘轮（ 方案 —— 范围合理，而"只做一次"的论证是最有力的部分：bot delta 上每 100 变更行 0.53 个 Critical，人类 delta 是 0.52，所以无论谁写的，新鲜评审员在每个 delta 上都会产出约 2 个 Critical。这确实排除了在有硬预算的轮内"审到干净"，我自己也会得出同样的形态。进入代码审查前有三个问题：
风险 —— 无升级风险信号：改动文件都不匹配与回滚相关的路径集合，并且这是同仓库（非 fork）、 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI read the title and the "Why it's needed" section first and wrote down what I would have done before opening the diff: one pre-commit review pass in the address-review skill, gated by a repo variable, bounded by a wait cap and a small-delta skip, recorded in a machine-readable file that the gate validates and renders as an HTML marker on the round report. That is what this is. The one place I thought I had a simpler shape — keep the existing 120-minute agent budget and let the pass eat into it, which would drop the per-arm step cap, the budget-clamp branch, the repair-chain skip and the whole job-bound re-arithmetic — does not survive contact with your own skip rule: the pass is skipped when fewer than 75 minutes remain, so under a 120-minute budget it could only start in the first 45 minutes and would come back The security handling is the part I looked hardest at, since this is CI machinery that executes PR code next to a write PAT, and it holds up:
I also checked whether the new marker perturbs the scan's bot-comment filters ( Four suggestions, none blocking. 1. The no-action render call site can never produce output. 2. An unrelated comment block is now orphaned above the new function. In 3. The default-path claim in the PR body does not match the code. "With the variable unset, expect behavior identical to today except a 190-minute step backstop that the unchanged 120-minute agent budget never reaches" — with the variable unset the arm resolves to 4. Nothing pins that the prepare step can actually see One thing to keep on your radar rather than fix here: the second commit got under the ratchet by condensing comments, which leaves sequenceDiagram
participant P1 as Prepare step
participant P2 as run-agent.mjs
participant P3 as Autofix skill agent
participant P4 as Nested review run
participant P5 as Verification gate
participant P6 as Finalize verification
participant P7 as Push and report
P1->>P1: resolve arm from repo variable and PR parity
P1->>P2: arm, CLI entry, round deadline
P2->>P3: three Invocation lines
P3->>P4: one bounded delta review at high effort
P4-->>P3: findings
P3->>P3: apply safe acts, re-run checks, commit once
P3->>P5: self-review.json carrying the tree id
P5->>P5: validate grammar, bind tree to the pushed head
P5->>P6: one token string, advisory only
P6->>P7: record selected with the outcome
P7->>P7: render the autofix-self-review marker
Files changed (7 of 7 shown)
TestingThis was an unattended CI run, so per the skill's rules I did not build or execute anything from this PR — all evidence below is the PR's own CI, read through the API. The head moved mid-review. At the first commit
Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 No check on the reviewed head is red. The macOS/Windows unit jobs and the CLI integration job are skipped by the platform-sensitivity classification, which is the expected outcome for a PR that touches only workflow YAML, CI shell, skill prose and a scripts test — there is no platform-dependent product code here. Not verified, and why no sandboxed lane can close it: the central claim — that one bounded delta review before the push reduces what the next review finds — is not verifiable from the diff, and neither isolated lane applies. Also unverified: your local 中文说明代码审查我先只读了标题和"为什么需要"，在看 diff 之前写下了自己的方案：在 address-review skill 里加一次提交前评审，由仓库变量控制，用等待上限和小 delta 跳过加以约束，把结果写进一个机器可读文件，由门校验并渲染成轮报告上的 HTML marker。这个 PR 就是这么做的。唯一一处我以为自己找到了更简形态的地方——保留现有 120 分钟 agent 预算、让这次评审从中扣——撑不过你自己的跳过规则：剩余不足 75 分钟就跳过，所以在 120 分钟预算下它只能在前 45 分钟内启动，多数轮次都会返回 安全处理是我看得最仔细的部分，因为这是在执行 PR 代码、旁边放着写权限 PAT 的 CI 机制，而它站得住：
我还检查了新 marker 是否会干扰扫描的 bot 评论过滤器（ 四条建议，均不阻塞。 1. 无操作形态的那个渲染调用点永远不会有输出。 2. 一段无关的注释现在被孤悬在新函数上方。 在 3. PR 正文关于默认路径的说法与代码不符。"变量未设置时，行为与今天完全一致，只是步骤后备上限变为 190 分钟——未变的 120 分钟 agent 预算永远碰不到它"——变量未设置时臂解析为 4. 没有任何测试钉住 prepare 步骤能否真的看到 有一件事请你留意、但不必在此修：第二个提交是靠压缩注释挤到棘轮之下的，这让 测试这是一次无人值守的 CI 运行，所以按 skill 规则我没有构建或执行本 PR 的任何东西——下面所有证据都是通过 API 读取的本 PR 自己的 CI。 head 在评审过程中变了。在第一个提交
被评审 head 上没有任何红色检查。macOS/Windows 单测作业与 CLI 集成作业是被平台敏感性分类跳过的，对于一个只改工作流 YAML、CI shell、skill 文本和 scripts 测试的 PR 来说这是预期结果——这里没有平台相关的产品代码。我一次性抓取时 **未验证，以及为何没有沙箱通道能补上：**核心主张——推送前一次有界的 delta 评审能减少下一次评审的发现——无法从 diff 验证，而两条隔离通道都不适用。 同样未验证：你本地 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid design and careful security handling; four non-blocking suggestions, and the one real reservation is about the file this lands in rather than the change itself. Going back to the proposal I wrote before reading the diff: this matches it, and it beat the one simplification I thought I had. I assumed the armed round could live inside the existing 120-minute agent budget, which would have removed the per-arm step cap, the budget-clamp branch, the repair-chain skip and the job-bound re-arithmetic — roughly half the workflow diff. It cannot, for a reason your own skip rule supplies: the pass is skipped below 75 minutes remaining, so a 120-minute budget confines it to the first 45 minutes and it would return The strongest thing here is not the mechanism, it is the argument that fixes its shape. Critical density of 0.53 per 100 changed lines on bot deltas against 0.52 on human ones says the reviewer's yield on a fresh delta is the invariant, not the fixer's code quality — so "review until clean" cannot converge inside a round, it just relocates the churn into a step with a hard budget whose overrun feeds the failure breakers. That is a non-obvious observation and it is what makes "one pass, bounded, measured" the correct answer rather than a compromise. I would not have thought to go and measure density on human deltas to settle the loop question. What I could not verify, plainly: no armed round has ever run, so the central behavioural claim rests on your rollout plan rather than on evidence. I am comfortable approving anyway, because the default is On whether I would maintain this in six months: yes, with one irritation. The double grammar-pinning and the tree-id binding look like trying hard until you remember the record is agent-authored on a branch the agent can write, at which point they are the minimum. The irritation is that Am I being worn down by volume? Worth asking out loud, since you have 24 open PRs and this is one of a series reshaping the same review/autofix loop. I evaluated it on its own evidence and I am naming the suggestions rather than waving them through. One coordination note that falls out of the pattern rather than the diff: this PR skips the same-run repair chain on armed rounds, and the open brake/failure-machinery PRs in that series are the ones that decide what a skipped repair costs. The af-156 text already says the A/B must read gate rejections per arm beside the round counts — that instruction is the load-bearing part, and it is worth making sure whoever reads the experiment actually has per-arm rejection counts, which today they would have to reconstruct from the variable, PR parity and runner environment rather than read off the thread (the marker only lands on pushed rounds). The four suggestions from the code review are all worth a pass but none should hold the merge: the output-less second render call site that a Approval is deferred until CI lands green on 中文说明信心：4/5 —— 设计扎实、安全处理谨慎；四条不阻塞的建议，而唯一真正的保留意见是关于这个改动所落进的文件，而不是改动本身。 回到我在看 diff 之前写下的方案：这个 PR 与它一致，而且它战胜了我自以为找到的那个简化。我原以为 armed 轮可以待在现有的 120 分钟 agent 预算里，那样就能去掉按臂区分的步骤上限、预算 clamp 分支、repair 链跳过以及 job 上限的重新算术——大约是工作流 diff 的一半。做不到，而理由正是你自己的跳过规则给出的：剩余不足 75 分钟就跳过，所以 120 分钟预算把它限制在前 45 分钟内，多数轮次都会返回 这里最有力的部分不是机制，而是钉死其形态的那个论证。bot delta 上每 100 变更行 0.53 个 Critical、人类 delta 上 0.52，这说明"评审员在新鲜 delta 上的产出率"才是不变量，而不是修复者的代码质量——所以"审到干净"在一轮之内无法收敛，它只是把 churn 搬进一个有硬预算、超时又会喂给失败熔断器的步骤。这是个不显然的观察，也正是它让"一次、有界、可度量"成为正确答案而非妥协。我想不到要去测人类 delta 上的密度来解决这个循环问题。 我无法验证的部分，直说：还没有任何 armed 轮跑过，所以核心的行为主张依赖的是你的上线计划，而不是证据。我仍然可以放心批准，因为默认是 关于半年后我是否愿意维护它：愿意，但有一处不适。双重语法钉定与 tree id 绑定看起来像是用力过猛，直到你想起这份记录是 agent 撰写、而且写在 agent 自己能写的分支上——那一刻它们就是最低限度。不适之处在于 我是不是被数量磨软了？值得大声问一句，因为你有 24 个开放 PR，而这是重塑同一套 review/autofix 循环的系列之一。我是按它自身的证据评审的，并且是把建议点名而不是挥手放过。有一条协调性备注是从这个系列模式（而非 diff）里掉出来的：本 PR 在 armed 轮跳过同轮 repair 链，而该系列中那些开放的熔断/失败机制 PR 正是决定"跳过一次 repair 代价几何"的地方。af-156 的文本已经说明 A/B 必须在轮数之外按臂读取门拒绝数——那句指示是承重的，而值得确保的是：真正读这个实验的人手上确实有按臂的拒绝数。今天他们必须从变量、PR 奇偶和 runner 环境重新算出来，而无法直接从线程上读到（marker 只落在推送轮上）。 代码审查里的四条建议都值得过一遍，但没有一条该压住合并：那个没有输出、却被 审批已推迟，等 CI 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
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 /review |
|
Qwen Code review request accepted. Review is running in workflow run. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes. |
Independent verification report — measured at head
|
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving at head 675c3355dfd5cbe309b34b46ea31907df1d5e0e9, cashing in the deferral left by our independent verification report (comment 5612573713, 2026-09-10T03:49:51Z). That report concluded mergeable and named the action for the following round explicitly: re-read the newest at-head verdict row and act on its state.
Legs re-verified in one enumeration immediately before this review was submitted:
- Lifecycle —
state=open,merged=false, head675c3355dfd5unmoved since the report was posted. - Approval leg — the newest at-head verdict row is
qwen-code-ci-bot APPROVED(review5162190969, submitted 2026-09-10T03:03:59Z,commit_id== head), with no superseding row from that author. It is a genuine verdict about this head rather than a re-anchored one: it was submitted 33 minutes after the head commit's committer date (02:30:19Z). Its provenance is a completed lane —Qwen Triage Finalizerun34431852750, jobfinalize-triage-ci, 03:03:41Z → 03:04:05Z,conclusion=success— and the triage lifecycle comment confirms it: "CI landed green on675c335and the deferred approval was posted." Triage stage 3 carries the sha-pinned markerapprove-on-green sha=675c3355dfd5…and records Confidence 4/5 with four non-blocking suggestions and no Critical. - CI leg — 50/50 check-runs on the head enumerated, no failing row. Every test lane is green:
Test (ubuntu-latest, Node 22.x),Lint & Static,web-shell E2E Smoke,Integration Tests (no-AK, No Sandbox),Desktop Shell(ubuntu-22.04 and windows-2022). - Our own leg — the report's 60 assertions, 0 failures, found no Critical. Because the changed surface is GitHub Actions workflow, CI gate scripts and autofix skill prompt assembly, the evidence is a locally executable base/head A/B plus a 26-case gate-record matrix rather than a tmux TUI run; the security conclusion is that a branch-writable
self-review.jsoncannot smuggle any token into the published report.
Two review lanes remain in flight on this head and neither has posted a verdict: the automatic review-pr job of run 34429715370 (step 13 Run review, in progress since 02:37:01Z) and the author-requested run 34432180210 (@qwen-code /review, in progress since 03:08:41Z). This approval rests on the settled triage verdict above, not on those two lanes. If either posts a Critical at this head, treat this approval as void and re-derive — a verdict row at an unchanged head can supersede an earlier one from the same author.
One note so the rollup is not misread: reviewDecision reads null here because a bot approval does not count toward the branch-protection decision. That does not contradict the at-head verdict row.
…accumulated (QwenLM#11529) (QwenLM#11537) The ratchet tolerates 4096 bytes of growth per PR without a baseline bump. Two consecutive qwen-autofix.yml PRs — QwenLM#9940 (to 465612, +2522 over the recorded 463090) and QwenLM#11516 (to 469259, +3647 more) — were each inside the allowance against their own base, so neither was forced to touch the recorded number, and the sum (6169) sails past it. Since QwenLM#11516 landed, every strict gate run — a local run without a PR base SHA, the push run of the PR itself whose event.before predates the growth, and any future PR that touches qwen-autofix.yml — fails on growth it did not author, the red-wall shape QwenLM#9904's leniency cannot cover for the file's own editors. Record the measured size (469259, wc -c on main) as the gate's own error message prescribes. The file now sits 741 bytes under the 470000-byte absolute gate, so the next real growth has to move prose into qwen-autofix.md instead of reaching for the ratchet again. Mutation probe: with the entry at 463090 the strict vitest mirror fails ("6169 bytes over its recorded 463090"); at 469259 the workflow-size suite passes 208/208 and check-workflow-size.sh exits 0.
|
Released in v0.23.3. |
What this PR does
Adds an opt-in, A/B-measured in-round self-review to the autofix address-review lane. When a round is armed, it runs ONE bounded adversarial review of its own delta (the same
review runthe local/autofixmode uses, at high effort) after the trusted checks pass and before it commits, applies what that pass finds, and pushes. The round then records what happened in a machine-read file; the verification gate validates that record, binds it to the tree id of the commit being pushed, and the round report carries an<!-- autofix-self-review … -->marker so the experiment can be read straight off the PR thread. Nothing rejects: the gate only measures.The arm is the repository variable
QWEN_AUTOFIX_SELF_REVIEW—off(default),ab(odd PR numbers armed, even ones the control arm) oron— resolved once per round in prepare and never on a GitHub-hosted runner. An armed round gets a 180-minute agent budget under a per-arm step cap (190 armed, 130 unarmed), skips the same-run repair chain, and skips the pass itself when the delta is under 150 changed lines or fewer than 75 minutes remain. The skill reads the arm from three Invocation lines (arm, CLI entry, round deadline) and nothing else.Why it's needed
Measured on all 40
autofix/takeoverPRs on 2026-09-10 (79 acted rounds, 1369 inline findings, each blamed at its review head): after a round pushes, 73% of the next review's new Criticals and 93% of its Suggestions sit on that round's own delta; 54% of those reviews had every new finding on the delta, and 62% would have posted nothing under the critical floor. Each pushed round costs a full cycle of about 870 minutes median (agent 118, review turnaround 289, idle to the next round 348). One fresh pass over the delta before the push therefore has the right scope, and its ceiling is that 54–62% of rounds.The same measurement fixes the shape. Critical density is 0.53 per 100 changed lines on bot deltas against 0.52 on human deltas — about 2 new Criticals per review whoever pushed, with no decay across rounds. The reviewer's yield on a fresh delta is the invariant, not the fixer's code, so "review until clean" cannot converge inside a round; it would only relocate the churn into a step with a hard budget whose overrun feeds the failure breakers. Hence one pass, bounded, measured.
Reviewer Test Plan
How to verify
on/off, a hosted runner never arms), the three Invocation lines the runner prints and the values it refuses, the skill's one-pass rule and its CLI exception, the gate's record validation against real files (missing, malformed, skipped, unbound), the marker render guard, and the per-arm step-cap arithmetic against the job bound. Runnpm run test:scripts -- scripts/tests/qwen-autofix-workflow.test.js.QWEN_AUTOFIX_SELF_REVIEW=onfor one PR you are watching, read the gate advisory and the marker on its round report, then switch toab.Evidence (Before & After)
N/A (workflow, gate script, skill prose, tests).
Tested on
Environment (optional)
Linux,
npm run test:scriptson the workflow test file: 249/250 pass. The one failure (locks the runner file-command backing files against env plants) fails identically onmainon this root-run machine — a chmod lock does not bind root — and is unrelated to this change.Risk & Scope
offchanges nothing. A step-leveltimeout-minutesnow carries an expression (job-level ones already do in this repository).Linked Issues
None. Design record: af-156 in the workflow design doc.
中文说明
这个 PR 做了什么
给 autofix 的 address-review 通道加一个可选、按 A/B 度量的轮内自审。一轮被 arm 时,它在可信检查通过之后、提交之前,对自己的 delta 做一次有界的对抗式评审(与本地
/autofix模式相同的review run,high effort),把这次评审发现的问题修掉再推送。随后该轮把发生了什么写进一个机器可读的文件;验证门校验该记录、把它绑定到即将推送的提交的 tree id,轮报告带上<!-- autofix-self-review … -->marker,实验结果可以直接从 PR 线程读取。不做任何拒绝:门只做度量。臂由仓库变量
QWEN_AUTOFIX_SELF_REVIEW决定——off(默认)、ab(奇数 PR 号 arm、偶数为对照臂)或on——每轮在 prepare 里解析一次,GitHub 托管 runner 上永不 arm。armed 的轮获得 180 分钟 agent 预算、按臂区分的步骤上限(armed 190、非 armed 130),跳过同轮 repair 链;delta 少于 150 变更行或剩余时间不足 75 分钟时跳过这次评审本身。skill 只从 Invocation 块的三行(臂、CLI 入口、本轮 deadline)读取信息,不看别的。为什么需要
2026-09-10 对全部 40 个
autofix/takeoverPR 的测量(79 个 acted 轮、1369 条行内发现,每条在其评审 head 上 blame 归因):一轮推送之后,下一次评审的新 Critical 有 73%、新 Suggestion 有 93% 落在该轮自己的 delta 上;这些评审里 54% 的全部新发现都在 delta 上,critical floor 下 62% 会变成空评审。每个推送轮花费整周期中位约 870 分钟(agent 118、评审周转 289、到下一轮的空闲 348)。因此推送前对 delta 做一次新鲜评审,作用域是对的,收益上限就是这 54–62% 的轮次。同一份测量也钉死了形态。bot delta 上每 100 变更行 0.53 个 Critical,人类 delta 是 0.52——无论谁推送,每次评审约 2 个新 Critical,逐轮不衰减。评审员对新鲜 delta 的产出率才是不变量,不是修复者的代码,所以"审到干净"在一轮之内无法收敛;它只会把 churn 搬进一个有硬预算的步骤,超时又会喂给失败熔断器。因此:一次、有界、可度量。
评审验证计划
如何验证
on/off、托管 runner 永不 arm)、runner 打印的三行 Invocation 及其拒绝的取值、skill 的单次规则与 CLI 例外、门对真实文件的记录校验(缺失、畸形、跳过、未绑定)、marker 渲染守卫、以及按臂的步骤上限与 job 上限的算术。运行npm run test:scripts -- scripts/tests/qwen-autofix-workflow.test.js。QWEN_AUTOFIX_SELF_REVIEW=on,读其轮报告上的门 advisory 与 marker,再切到ab。证据(前后对比)
N/A(工作流、门脚本、skill 文本、测试)。
测试平台
环境(可选)
Linux,对工作流测试文件运行
npm run test:scripts:249/250 通过。唯一失败的locks the runner file-command backing files against env plants在本机(root 运行)的main上同样失败——chmod 锁对 root 不生效——与本改动无关。风险与范围
off不改变任何行为。一个步骤级timeout-minutes现在带表达式(本仓库 job 级早已如此)。关联 Issue
无。设计记录:工作流设计文档 af-156。