feat(review): add a --topology minimal single-pass A/B arm - #9919
Conversation
The first step of #9783 is an A/B baseline: run the full --effort high pipeline and a minimal prompt over the same PR set, then compare recall and precision per model. This adds the minimal arm (the B side) so that experiment can run. The minimal topology is a single careful senior-engineer pass over the diff in the orchestrator's own context — at most fifteen findings, each requiring a concrete failure scenario, with no fan-out, verification, reverse audit, build/test, or posting. It is deliberately not the low-effort angle rotation. Routing lives in parse-args' verdict (topology / topologySource), orthogonal to effort — minimal is a different shape of review, not a depth of one. It is terminal-only, so the parser forces comment.effective and fix.effective off. SKILL.md gains a Step 3M and a Step 3 dispatch override; DESIGN.md prices the arm (0 subagent calls) in the LLM call budget. No model auto-routing yet — that waits on this A/B's data.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-run after six autofix commits — gate re-checked on Template: complete ✓ Problem: unchanged from the first pass — observed, not theoretical. Issue #9783 documents the motivation (a side-by-side study against a competitor reviewer showing the scaffolding's marginal value shrinking, even turning negative, as the model strengthens), and its step 1 — the A/B baseline — cannot start without the minimal arm this PR adds. Direction: aligned, and the scope restraint is still the right call. Only the selectable arm — no model-family auto-routing until the A/B data exists. The competitor's own CHANGELOG shows them oscillating between exactly these two shapes, which is the signal that both arms earn their place. Size: core paths touched (the review skill under Approach: the verdict stands — the scope is exactly the three things a selectable A/B arm needs: the parser verdict with the write gates forced off in code, the Step 3M pass plus dispatch overrides, and the DESIGN.md price entry. Since the last pass the autofix rounds added what the iterative reviews asked for — Risk: Moving on to code review. 🔍 中文说明autofix 六轮提交后的重跑——已在新 head 模板：完整 ✓ 问题：与上一轮结论一致——已观测、非理论性。Issue #9783 记录了动机（与竞品评审器的对照研究表明，随着模型变强，脚手架的边际价值下降甚至转负），其第 1 步（A/B 基线）必须先有本 PR 添加的极简臂才能开始。 方向：对齐，范围克制依然正确。只做可选臂——A/B 数据出来之前不做模型族自动路由。竞品自己的 CHANGELOG 显示他们恰好在两种形态之间摇摆过，这正是两种臂各有存在价值的信号。 规模：触及核心路径（ 方案：结论不变——范围恰好是一个可选 A/B 臂所需的三样：代码里强制关闭写门控的解析器裁决、Step 3M 通路加分发覆写、DESIGN.md 的预算条目。上一轮之后，autofix 补上了迭代评审要求的内容——minimal 下门控 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (re-run at
|
| Check | Conclusion |
|---|---|
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Not verified: that a live review <pr> --topology minimal run actually walks Step 3M end-to-end — the suite pins the parser verdict through the real handler wiring and the write gate at exit-code level, but prompt adherence is not exercisable in CI. The author's reported macOS runs are their claim, not evidence this pass relies on. Sandboxed verification would settle this: @qwen-code /tmux — drive a real review <pr> --topology minimal and observe the terminal-only report (≤15 findings, each with a failure scenario, labeled unverified, nothing posted).
中文说明
代码审查（重跑于 11cf00cb，autofix 六轮提交之后）
独立基线与上一轮一致；本轮重点是 autofix 新增的内容，以及第 5 轮评审的问题是否真正闭环。结论：已闭环。
- [第 5 轮 Critical——已关闭] Step 3M 现在自带与 Step 3C 平行的
report_findings覆写：level: "low"、条目来自组合好的 findings 列表（该臂不存在 artifact）、仅对保留的低置信候选传confidence: "low"。我没有采信 diff 的自述，而是对照代码验证了前提：FindingsDisplay.tsx只在level === 'low'时渲染未验证标记，因此该臂复用 low 档的渲染路径，无需扩展枚举。新增的SKILL.test.ts用例直接点名了该失效模式。 - [R5-1——已关闭] 拒绝建议改为结构化分类：
reviewWriteAuthorization返回cls: 'topology' | 'comment-not-requested' | 'unbound'判别字段，submit.ts按它分支，不再匹配拒绝文案。已核对全部 7 条拒绝路径都携带cls、authorization()包装函数原样转发结果，且有回归测试钉住了子串匹配曾导致的误导——记录参数中内嵌的反引号--topology minimal不再能把非 topology 拒绝误导进该分支。 - [R5-2——已关闭] topology 拒绝现在只在记录通过全部绑定轴之后才触发，因此"去掉它重跑"是真实有效的建议——且 submit 侧的分支点名了重跑仍需要的 comment 来源（
--comment或review.comment设置）。两个门控调用方形都有测试钉住：submit 的，以及 publish-assets 的环境变量解析 host 形态（后者在 host 不匹配的记录上会先报 host 绑定、而非 topology）。 - 上一轮的两个瑕疵也已消除：minimal 拒绝点名 topology 而不是"未指定 PR"；
--resume在 minimal 下被门控（全新的单遍既不续跑也不消耗中断状态），警告文案如实说明。 --user-authorized快速通道刻意不查询 topology——有测试记录了这一分层设计，posting.md 的跳过规则是兜底层，负责捕获 Step 6 的拒绝被漏掉的运行。两处文件自洽。
其余非阻塞项，系第 5 轮评审按收敛姿态明确延期（已记录、本轮未要求）：PR 目标上的 --fix 警告在 minimal 下仍建议使用已被禁用的 --comment，另有 5 条 SKILL.md/posting.md 的措辞问题。提请维护者留意；均不阻塞本 PR。
另核查了高风险路径扫描命中的 geminiChat.ts：系从 main 合并带入的 3 行 prettier 格式产物，与当前 main 逐字相同——零语义内容。
未发现正确性阻塞问题。
测试证据——PR 自身的 CI（无人值守运行；此处未执行任何 PR 代码）
主单测套件仍在被审提交上运行；安全检查已通过；macOS / Windows / 集成测试本次报告为 skipped——如实转述，不做评判。不轮询等待；CI 结束后 finalize 任务会更新上表。
未验证项：一次真实的 review <pr> --topology minimal 运行是否真的走完 Step 3M——单测已把解析器裁决（经由真实 handler 接线）和写门控（到 exit-code 级别）钉住，但提示词的遵守情况无法在 CI 中演练。作者报告的 macOS 本地运行是其自述，不作为本审查的证据。沙箱验证可以补上这一环：@qwen-code /tmux——真实驱动一次 review <pr> --topology minimal，观察仅终端输出的报告（≤15 条、每条带失败场景、标注未验证、不发帖）。
— Qwen Code · qwen3.8-max
Reviewed at 11cf00cb4de226896633a2b279c1b70708ece74c · re-run with @qwen-code /triage
|
Confidence: 4/5 — every finding the iterative reviews raised on this branch is closed with regression tests; what remains is the review's own explicitly-deferred prompt-wording list, which it chose not to block on. Honest reflection: the six autofix commits since the first pass turned the round-5 review's Critical and two suggestions into exactly the fixes it prescribed — the structural refusal classifier it proposed, the sole-blocker ordering, and the Step 3C-parallel The 4, not the 5, is the standing residual: no live run has walked Step 3M (named in Stage 2's lane line), and the deferred wording items stay open by the review's own convergence call. Neither blocks. Verdict: approve. CI is still running on the reviewed commit, so the approval is deferred until it lands green on that SHA. 中文说明置信度：4/5 —— 迭代评审在这条分支上提出的所有问题均已关闭且带回归测试；剩下的只是评审自己明确决定不阻塞的提示词措辞延期清单。 如实复盘：上一轮之后的 6 个 autofix 提交，把第 5 轮评审的 Critical 和两条建议变成了其所处方的修复——结构化拒绝分类器、唯一阻塞者优先的排序、与 Step 3C 平行的 给 4 不给 5，是两项常设残留：没有一次真实运行走完过 Step 3M（已在 Stage 2 的沙箱入口点名）；措辞类延期项按评审自己的收敛决定保持开放。两者都不阻塞。 结论：批准。CI 仍在被审提交上运行，待其在同一 SHA 上全绿后，批准自动落上。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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 /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
| ); | ||
| } | ||
|
|
||
| const resumeEffective = resumeRequested && isPr; |
There was a problem hiding this comment.
[Critical] The minimal-topology gate is applied to comment.effective and fix.effective in the parser — this diff's own principle is "keep the guarantee in code rather than in whichever prose the orchestrator reads" — but --resume is left ungated: under --topology minimal, resume.effective stays resumeRequested && isPr, with no warning, and the new SKILL.md minimal bullet defines no semantics for the combination.
Running /review 1234 --resume --topology minimal while an interrupted full-pipeline review of PR 1234 exists on disk emits resume: {requested: true, effective: true} with no warning. The new minimal bullet says diff capture runs "exactly as below" (whose resume branch appends --resume to fetch-pr) while also saying to skip the rest of Step 1 and jump to Step 3M — the two instructions collide. On one reading, fetch-pr --resume reuses the interrupted run's worktree/plan, consumes one of the two bounded resume leases (RESUME_MAX = 2), then Step 3M skips every recovery step and Step 9 cleanup deletes the worktree — the interrupted run's resumable state is consumed and destroyed by a pass that never continues it, silently, and the next genuine --resume hits worktree-gone. On the other reading --resume is silently dropped — the inert-flag case every sibling gates with an explicit warning. Which one happens is model-dependent improvisation the parser exists to prevent.
Witness (probe against the PR's code):
parseReviewArgs('1234 --resume --topology minimal')
→ resume: { requested: true, effective: true }, warnings: []
parseReviewArgs('1234 --comment --fix --topology minimal')
→ comment.effective: false, fix.effective: false, both WITH terminal-only warnings
Gate it where its siblings are gated — const resumeEffective = resumeRequested && isPr && !isMinimal; — plus a terminal-only warning mirroring the comment/fix ones, or explicitly arbitrate the combination in SKILL.md's minimal bullet and cover it with a test.
中文说明
[Critical] minimal 拓扑的门控在解析器里应用到了 comment.effective 和 fix.effective——本 diff 自己的原则就是「把保证放在代码里，而不是放在 orchestrator 读到的某段文案里」——但 --resume 没有被门控：在 --topology minimal 下，resume.effective 仍然是 resumeRequested && isPr，既没有门控也没有警告，SKILL.md 新增的 minimal 条目也没有为这个组合定义任何语义。
当 PR 1234 存在一个被中断的全量管线评审时运行 /review 1234 --resume --topology minimal，裁决会输出 resume: {requested: true, effective: true} 且没有任何警告。新的 minimal 条目一方面说 diff 捕获「完全按下文执行」（其中的 resume 分支会把 --resume 追加到 fetch-pr），另一方面又说跳过 Step 1 的其余部分直接进 Step 3M——两条指令互相冲突。按其中一种解读，fetch-pr --resume 会复用被中断运行的 worktree/plan，消耗两次有界 resume 租约（RESUME_MAX = 2）之一，然后 Step 3M 跳过所有恢复步骤、Step 9 cleanup 删掉 worktree——被中断运行的可恢复状态被一个根本不继续它的通路静默地消耗并销毁，下一次真正的 --resume 会撞上 worktree-gone。按另一种解读，--resume 被静默丢弃——而这正是每个兄弟 flag 都会用显式警告门控的「flag 无效」情形。到底发生哪一种，取决于模型的临场发挥，而解析器的存在正是为了消除这种临场发挥。
证据（对 PR 代码的探针）：
parseReviewArgs('1234 --resume --topology minimal')
→ resume: { requested: true, effective: true }, warnings: []
parseReviewArgs('1234 --comment --fix --topology minimal')
→ comment.effective: false, fix.effective: false，且都带 terminal-only 警告
请在兄弟门控所在处一并门控——const resumeEffective = resumeRequested && isPr && !isMinimal;——并仿照 comment/fix 增加一条 terminal-only 警告；或者在 SKILL.md 的 minimal 条目中显式裁定该组合，并补一个测试覆盖。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Then skip Steps 4 and 5 entirely and go to Step 6 with these adjustments: | ||
|
|
||
| - Use Step 6's structure, but label the review **"Minimal pass (topology: minimal) — findings are unverified"** (translated per output language) in the Summary, and skip verification stats (there was no verification). | ||
| - Emit **no verdict** — no Approve / Request changes / Comment, and skip the open-Criticals re-check. Chunks that are uncoverable by `maxLineChars` are still listed under "Not reviewed". |
There was a problem hiding this comment.
[Critical] Step 3M introduces a run shape that cannot satisfy Step 9's completion-line contract: the final line must be byte-for-byte one of four dispositions, and none fits a minimal run. The posted forms require a Step 7 submission (the parser forces comment.effective false for this topology), <verdict>, not posted requires a verdict (Step 3M forbids emitting one), Aone partial does not apply, and quick pass, not posted is annotated "low effort only" while a minimal PR review resolves effort: "high" (probe-verified).
So every completed /review <target> --topology minimal run either borrows the low-only quick-pass line — mislabeling the arm as a low-effort run in the machine record that batch drivers and log scrapers key on, which corrupts the per-model A/B comparison data this arm exists to produce — or invents a disposition outside the enumerated four, reproducing the exact ad-hoc-phrasing failure the fixed contract was created to end. Either way one of Step 9's two MUSTs is violated on every run of the new arm.
Witness: not run — the disposition is emitted by the orchestrator per prose contract; no code unit in the repo emits or validates the Review complete: line, so no executable exists to settle it. The effort-resolution half was run: probe observed parseReviewArgs('6711 --topology minimal') → {"effort":"high","effortSource":"default"}.
Add a minimal disposition to Step 9's list (e.g. minimal pass, not posted (<N> unverified findings)) and reference it from Step 3M's adjustments — or explicitly bless the quick-pass form for Step 3M and widen its "low effort only" annotation to name the topology.
中文说明
[Critical] Step 3M 引入了一种无法满足 Step 9 完成行契约的运行形态：最后一行必须逐字节匹配四种 disposition 之一，而没有一种适合 minimal 运行。posted 形态需要 Step 7 提交（解析器已对该拓扑强制 comment.effective 为 false），<verdict>, not posted 需要裁决（Step 3M 明确禁止输出裁决），Aone 的 partial 不适用，而 quick pass, not posted 标注为「仅 low effort」——但 minimal 的 PR 评审解析出的 effort 是 "high"（已用探针验证）。
因此每一次完成的 /review <target> --topology minimal 运行，要么借用 low 专属的 quick-pass 行——在批处理驱动和日志抓取器所依赖的机器记录里把这个臂误标为 low-effort 运行，从而污染本臂存在的全部意义（按模型对比的 A/B 数据）——要么发明一个枚举之外的 disposition，复现固定契约当初要消灭的「临时措辞」失败。无论哪一种，新臂的每次运行都会违反 Step 9 两个 MUST 之一。
证据：未执行——disposition 由 orchestrator 按文案契约输出，仓库中没有任何代码单元发出或校验 Review complete: 行，因此没有可执行的判定对象。其中 effort 解析的一半已执行：探针观察到 parseReviewArgs('6711 --topology minimal') → {"effort":"high","effortSource":"default"}。
请在 Step 9 的列表中为 minimal 增加一个 disposition（例如 minimal pass, not posted (<N> unverified findings)），并在 Step 3M 的调整里引用它；或者显式允许 Step 3M 复用 quick-pass 形态，并把其「仅 low effort」的注解扩展为同时点名该拓扑。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| } else if (commentRequested && isPr && isMinimal) { | ||
| // Only when minimal is THE reason a would-be-effective comment is |
There was a problem hiding this comment.
[Suggestion] This branch fires on commentRequested (flag OR the review.comment setting via defaults.comment === true), but the warning text always names the --comment flag — misattributing the request when it came from settings. The sibling forced-by-comment effort warning ~20 lines below explicitly distinguishes flag vs setting origin; this branch dropped that distinction.
An operator with review.comment: true in settings who runs /review 6711 --topology minimal without typing the flag gets "Warning: --comment is ignored because --topology minimal is terminal-only…" for a flag they never passed — and the verdict simultaneously reports comment.requested: false (that field keys on the flag only), so the output contradicts itself. Probe-verified: parseReviewArgs('6711 --topology minimal', { comment: true }) returns exactly that warning with comment: {requested: false, effective: false}.
Mirror the sibling pattern:
warnings.push(
commentRequestedByFlag
? 'Warning: `--comment` is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.'
: 'Warning: the `review.comment` setting is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.',
);中文说明
[Suggestion] 该分支基于 commentRequested（flag 或设置里的 review.comment，经由 defaults.comment === true）触发，但警告文案始终点名 --comment flag——当请求实际来自设置时归因错误。下方约 20 行的兄弟警告（comment 强制 effort）明确区分了 flag 与设置两种来源；这个分支丢掉了这一区分。
一位在设置里开了 review.comment: true 的操作者，在没有输入该 flag 的情况下运行 /review 6711 --topology minimal，会看到「Warning: --comment is ignored because --topology minimal is terminal-only……」——指向一个他从未输入过的 flag；而且裁决同时报告 comment.requested: false（该字段只以 flag 为准），输出自相矛盾。已用探针验证：parseReviewArgs('6711 --topology minimal', { comment: true }) 正是带着 comment: {requested: false, effective: false} 返回了这条警告。
请仿照兄弟分支的写法：
warnings.push(
commentRequestedByFlag
? 'Warning: `--comment` is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.'
: 'Warning: the `review.comment` setting is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.',
);— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning (the previous round exhausted its time budget) and was scoped to the two Critical findings only. The finding is acknowledged as real: the minimal-topology comment warning fires on commentRequested (flag OR setting) but always names the --comment flag, contradicting comment.requested: false when the request came from review.comment: true. The fix is the suggested flag-vs-setting split mirroring the sibling forced-effort warning; it is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行（上一轮耗尽了时间预算），范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：minimal 拓扑的 comment 警告基于 commentRequested（flag 或设置）触发，但文案始终点名 --comment flag，当请求实际来自 review.comment: true 时与 comment.requested: false 自相矛盾。修复方案即建议中的仿照兄弟强制 effort 警告、区分 flag 与设置来源的写法；已排入下一轮。
|
|
||
| const commentRequested = commentRequestedByFlag || defaults.comment === true; | ||
| const commentEffective = commentRequested && isPr; | ||
| const commentEffective = commentRequested && isPr && !isMinimal; |
There was a problem hiding this comment.
[Suggestion] This line adds a third cause for comment.effective === false (minimal topology), but the write-authorization slow path in lib/authorization.ts (~356-373) composes its refusal why from only the two pre-existing causes — so a minimal run receives a factually wrong reason, while the gate's own comment says the refusal must "name the REAL blocker".
After /review 6711 --topology minimal --comment (args are recorded verbatim before the skill runs), any later publish attempt in that session re-parses the recording and correctly refuses — but the message says the arguments "do not name a pull request, so they cannot authorise posting to one" while they plainly name PR 6711. The real blocker (--topology minimal is terminal-only) is never named, so the operator is told a fix that cannot work and never told the one that does. Probe-verified by driving reviewWriteAuthorization with the recorded args: refusal why = the review arguments ("6711 --topology minimal --comment") do not name a pull request, so they cannot authorise posting to one; a control (src/foo.ts --comment) returns the same wording truthfully there.
Branch on the new cause in the refusal construction, ahead of the existing arms, e.g. if (verdict.topology === 'minimal') → "the review arguments selected --topology minimal, which is terminal-only and cannot authorise posting".
中文说明
[Suggestion] 这一行为 comment.effective === false 增加了第三个成因（minimal 拓扑），但 lib/authorization.ts（约 356-373 行）的写授权慢路径只根据既有的两个成因拼装拒绝理由——于是 minimal 运行会收到一个与事实不符的理由，而该门控自己的注释要求拒绝必须「点名真正的阻塞原因」。
在 /review 6711 --topology minimal --comment 之后（参数在技能运行前已被逐字记录），该会话中任何后续的发布尝试都会重新解析这份记录并正确拒绝——但消息说这些参数「没有指定 pull request，因此不能授权发布」，而参数里明明写着 PR 6711。真正的阻塞原因（--topology minimal 仅终端输出）从未被点名，操作者被告知一个不可能生效的修复方向，却永远得不到真正有效的那个。已用探针验证：以记录参数驱动 reviewWriteAuthorization，拒绝理由为 the review arguments ("6711 --topology minimal --comment") do not name a pull request, so they cannot authorise posting to one；对照组（src/foo.ts --comment）在同样文案下返回的理由是真实的。
请在拒绝理由的构造中、在既有分支之前，为这个新成因加一个分支，例如 if (verdict.topology === 'minimal') → "the review arguments selected --topology minimal, which is terminal-only and cannot authorise posting"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real: the write-authorization refusal why in lib/authorization.ts composes only the two pre-existing causes of comment.effective === false, so a minimal run is refused with the factually wrong "do not name a pull request" reason while the real blocker (--topology minimal is terminal-only) goes unnamed. The fix is the suggested topology === 'minimal' branch ahead of the existing arms; it is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：lib/authorization.ts 的写授权拒绝理由只根据 comment.effective === false 的既有两个成因拼装，于是 minimal 运行会收到与事实不符的「没有指定 pull request」理由，而真正的阻塞原因（--topology minimal 仅终端输出）从未被点名。修复方案即建议中的在既有分支之前增加 topology === 'minimal' 分支；已排入下一轮。
| continue; | ||
| } | ||
|
|
||
| if (token === '--topology' || token.startsWith('--topology=')) { |
There was a problem hiding this comment.
[Suggestion] This block is a near-verbatim third copy of the value-token grammar already implemented for --effort and --severity-floor in this file (~34 lines: equals form, PR-shaped rescue, quoted-empty consumption with the i++, flag-final/flag-followed missing, kept-push), and the new deferred-warning switch is likewise a third copy. The file's own comments record repeated bug-fixes to this grammar that had to be replicated across every block — the "(round-8 review finding)" eq-form rescue was replicated into this new copy — and the module header declares it "the single source of truth".
The cost is concrete: the next grammar fix or invalid-value rule applied to --effort/--severity-floor but missed in --topology makes the identical typo dispose differently depending on which flag carries it (different target rescue, different warning) — exactly the inconsistency this module exists to eliminate.
Extract the shared grammar into one helper used by all three flags (flag name, asX validator, issues list, kept, setter — returning the new token cursor), and unify the three deferred-warning switches into one composeIssueWarnings(flagLabel, issues, resolution, warnings); per-flag differences stay at the call sites.
中文说明
[Suggestion] 这个块是值 token 语法的第三份近乎逐字的拷贝——本文件中 --effort 和 --severity-floor 已各有一份（约 34 行：等号形态、PR 形状救援、带 i++ 的引号空值吞噬、flag 结尾/flag 跟随的缺值、kept-push），新增的延迟警告 switch 同样是第三份拷贝。本文件自己的注释记录了这段语法反复修 bug、且每次修复都必须在所有块里复制一遍的历史——「(round-8 review finding)」的等号形态救援就被复制进了这个新块——而模块头部宣称自己是「唯一事实来源」。
代价是具体的：下一次对 --effort/--severity-floor 应用、却漏掉 --topology 的语法修复或无效值规则，会让同一个拼写错误因落在不同 flag 上而有不同处置（不同的目标救援、不同的警告）——这正是本模块存在要消灭的不一致。
请把共享语法抽成一个三个 flag 共用的助手函数（flag 名、asX 校验器、issues 列表、kept、setter——返回新的 token 游标），并把三个延迟警告 switch 统一为一个 composeIssueWarnings(flagLabel, issues, resolution, warnings)；各 flag 的差异留在调用点。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only, and this one is a refactor (~34-line grammar extraction across three flags plus unifying three deferred-warning switches). The finding is acknowledged as real: --topology is a near-verbatim third copy of the value-token grammar, and the module's own comments record grammar bug-fixes that had to be replicated across every copy. The extraction into one shared helper + composeIssueWarnings is queued for the next round, where it can get its own verification pass.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现，而本条是一次重构（跨三个 flag 提取约 34 行的值 token 语法，并统一三个延迟警告 switch）。该发现被承认为真实：--topology 是值 token 语法的第三份近乎逐字的拷贝，模块自身的注释也记录了每次语法修复都必须在所有拷贝中复制的历史。抽成一个共享助手函数 + composeIssueWarnings 已排入下一轮，届时可以有自己独立的验证过程。
| continue; | ||
| } | ||
| const next = i + 1 < tokens.length ? tokens[i + 1] : undefined; | ||
| const nextTopology = next !== undefined ? asTopology(next) : null; |
There was a problem hiding this comment.
[Suggestion] The new arm's missing-value error paths — flag-final, flag-followed-by-flag, quoted-empty consumption (the i++ here), and the empty --topology= equals-form — have zero tests, while both sibling flags test every one of these shapes (~739-776 in the test file). The file's own annotation on the sibling quoted-empty test records a "Mutation-shown gap" that survived until that test existed. (The discarded warning kind is already covered by the existing '--topology minial 6711' test — verification corrected the original claim.)
Measured in a scratch tree: dropping the i++ in the topology quoted-empty consumption, or deleting the flag-final missing push, each survive the entire suite (still 133/133 green); the flip check distinguishes the arms (mutant: extraTokens: ["minimal"] for '6711 --topology minimal'; PR code: clean parse). The identical i++ deletion in the --effort branch fails the existing suite, proving the gap is topology-specific.
Extend the existing missing-value tests to --topology: parseReviewArgs('6711 --topology --comment') (flag-followed; --comment must still register as requested), parseReviewArgs('6711 --topology') (flag-final), parseReviewArgs('--topology ""') (consumed as missing, target stays local), each asserting a --topology requires a value warning.
中文说明
[Suggestion] 新臂的缺值错误路径——flag 位于末尾、flag 后紧跟另一个 flag、引号空值吞噬（即此处的 i++）、空的 --topology= 等号形态——一个测试都没有，而两个兄弟 flag 对这些形态都有测试（测试文件约 739-776 行）。兄弟引号空值测试上的注解本身就记录过一个「突变测试揭示的缺口」，直到那个测试存在才被堵住。（discarded 警告种类已被既有的 '--topology minial 6711' 测试覆盖——验证阶段修正了最初的表述。）
在 scratch tree 中实测：删掉 topology 引号空值吞噬里的 i++，或删掉 flag 结尾的 missing push，整个测试套件仍然全绿（133/133）；翻转检查可以区分两臂（突变体对 '6711 --topology minimal' 输出 extraTokens: ["minimal"]；PR 代码解析干净）。在 --effort 分支做同样的 i++ 删除会让既有套件失败，证明这个缺口是 topology 专属的。
请把既有的缺值测试扩展到 --topology：parseReviewArgs('6711 --topology --comment')（flag 后跟 flag；--comment 仍须登记为 requested）、parseReviewArgs('6711 --topology')（flag 结尾）、parseReviewArgs('--topology ""')（按缺值吞噬，目标保持 local），各自断言出现 --topology requires a value 警告。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real (with its corrected note that the discarded kind is already covered): the --topology missing-value paths — flag-final, flag-followed-by-flag, quoted-empty consumption, empty equals-form — have zero tests while both sibling flags test all of them, and the measured mutants survive the suite. Extending the existing missing-value tests to --topology is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实（包括其修正说明：discarded 种类已有覆盖）：--topology 的缺值路径——flag 位于末尾、flag 后紧跟 flag、引号空值吞噬、空的等号形态——一个测试都没有，而两个兄弟 flag 对这些形态都有测试，且实测突变体在整个套件中存活。把既有的缺值测试扩展到 --topology 已排入下一轮。
| // guarantee in code rather than in whichever prose the orchestrator reads. | ||
| const isMinimal = topology === 'minimal'; | ||
|
|
||
| const commentRequested = commentRequestedByFlag || defaults.comment === true; |
There was a problem hiding this comment.
[Suggestion] The new gate below is deliberately written over commentRequested (flag OR the review.comment setting), but every added test requests --comment only via the flag — no test crosses a settings-provided comment: true with --topology minimal, so the settings path through the !isMinimal gate is untested.
Measured: a harm-shaped mutant that lets a settings-driven comment bypass the minimal gate — (commentRequestedByFlag && isPr && !isMinimal) || (defaults.comment === true && isPr) — survives the entire suite (133/133), and the probe observes the harm: parseReviewArgs('6711 --topology minimal', { comment: true }) returns comment: {requested: false, effective: true} under it. Because lib/authorization.ts re-parses the recorded args through this same function and gates on verdict.comment.effective, the same mutation opens the posting gate — defeating the exact guarantee the code comment says is kept "in code rather than in whichever prose the orchestrator reads". The missing test is the only guard that would catch that regression.
Add parseReviewArgs('6711 --topology minimal', { comment: true }) asserting comment: { requested: false, effective: false } (note requested is flag-only) plus the terminal-only warning, mirroring the existing settings-comment tests.
中文说明
[Suggestion] 下面新增的门控刻意基于 commentRequested（flag 或设置里的 review.comment）书写，但所有新增测试都只通过 flag 请求 --comment——没有任何测试把设置提供的 comment: true 与 --topology minimal 组合起来，因此设置路径经过 !isMinimal 门控的这条通路没有测试覆盖。
实测：一个让设置驱动的 comment 绕过 minimal 门控的危害形突变体——(commentRequestedByFlag && isPr && !isMinimal) || (defaults.comment === true && isPr)——整个套件全部通过（133/133），且探针观察到危害：在该突变体下 parseReviewArgs('6711 --topology minimal', { comment: true }) 返回 comment: {requested: false, effective: true}。由于 lib/authorization.ts 会通过同一个函数重新解析记录的参数、并以 verdict.comment.effective 为门，同一个突变还会打开发布门控——恰好击穿代码注释所说的「放在代码里而非放在 orchestrator 读到的文案里」的保证。缺失的这个测试是唯一能抓住该回归的防线。
请补一个 parseReviewArgs('6711 --topology minimal', { comment: true })，断言 comment: { requested: false, effective: false }（注意 requested 只以 flag 为准）并断言 terminal-only 警告，仿照既有的设置 comment 测试。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real: no test crosses a settings-provided comment: true with --topology minimal, and the measured mutant that lets a settings-driven comment bypass the minimal gate survives the suite and would open the posting gate in lib/authorization.ts. Adding parseReviewArgs('6711 --topology minimal', { comment: true }) asserting comment: {requested: false, effective: false} plus the warning is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：没有任何测试把设置提供的 comment: true 与 --topology minimal 组合起来，且实测中让设置驱动的 comment 绕过 minimal 门控的突变体在整个套件中存活，并会打开 lib/authorization.ts 的发布门控。补一个断言 comment: {requested: false, effective: false} 与警告的 parseReviewArgs('6711 --topology minimal', { comment: true }) 已排入下一轮。
|
|
||
| ## LLM call budget | ||
|
|
||
| **`--topology minimal` — 0 subagent calls.** The minimal arm (issue #9783, Step 3M) is a single careful pass over the diff in the orchestrator's own context — no fan-out, no verification, no reverse audit, no build/test. It costs one model turn, the same shape as the low tier's inline pass, and it is priced here for completeness, not as a recommended default: it exists so the full pipeline and this minimal prompt can be run over the same PR set and compared per model. This section tracks per-topology _cost_; the A/B the minimal arm enables extends it with per-model _quality_, which is what decides whether any cell of a future model-family × effort routing table routes away from the full pipeline. Until that data exists, every review runs the topology below. |
There was a problem hiding this comment.
[Suggestion] The minimal arm shares all three properties the prose reserves for low — labeled unverified, emits no verdict, never writes the incremental cache — but two enumerations still say the set has one member, and this diff does not update either:
docs/users/features/code-review.md:44— "Onlylowis unverified." and "The low pass is labeled unverified, emits no verdict, and never writes the incremental review cache", on the page that documents the other flags but omits--topologyentirely.DESIGN.md:682in the very file this diff edits — the guardrails scope sentence "means low alone", eleven lines above the paragraph appended here.
A user reading the docs trusts --topology minimal to be verified-grade, or discovers the flag from the new argument-hint and finds no documentation for it; and a maintainer re-scoping the guardrails for "the" unverified pass from the design record would enumerate low only and silently leave the minimal arm out, because the authoritative design doc says the set has exactly one member. Both texts verified at the reviewed commit.
Correct both enumerations in this PR: add a short --topology minimal section to docs/users/features/code-review.md (or at least change "Only low is unverified" to include the minimal arm), and in the guardrails paragraph replace "means low alone" with "means low and the --topology minimal arm (Step 3M)".
中文说明
[Suggestion] minimal 臂具备文案中保留给 low 的全部三个属性——标注为未验证、不输出裁决、从不写增量缓存——但仍有两处枚举说这个集合只有一个成员，而本 diff 没有更新任何一处：
docs/users/features/code-review.md:44—— "Onlylowis unverified." 与 "The low pass is labeled unverified, emits no verdict, and never writes the incremental review cache"，所在的页面为其他 flag 都有文档，却完全没有提及--topology。- 就在本 diff 编辑的这个文件里，
DESIGN.md:682—— guardrails 范围句 "means low alone"，位于此处新增段落上方十一行。
用户读到文档会以为 --topology minimal 是验证过的等级，或者从新的 argument-hint 发现这个 flag 却找不到任何文档；而日后按设计记录为「那个」未验证通路重划 guardrails 的维护者只会枚举 low，把这个 minimal 臂静默地漏掉——因为权威设计文档说这个集合恰好只有一个成员。两处文本均已在被评审的 commit 上核实。
请在本 PR 中一并修正两处枚举：为 docs/users/features/code-review.md 增加一小节 --topology minimal（至少把 "Only low is unverified" 改为包含 minimal 臂），并在 guardrails 段落中把 "means low alone" 替换为 "means low and the --topology minimal arm (Step 3M)"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real: the minimal arm shares all three properties the prose reserves for low, but docs/users/features/code-review.md ("Only low is unverified") and DESIGN.md's guardrails scope sentence ("means low alone") still enumerate a one-member set, and the docs page omits --topology entirely. Correcting both enumerations and adding a short --topology minimal docs section is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：minimal 臂具备文案保留给 low 的全部三个属性，但 docs/users/features/code-review.md（「Only low is unverified」）与 DESIGN.md 的 guardrails 范围句（「means low alone」）仍说这个集合只有一个成员，且文档页完全没有提及 --topology。修正两处枚举并为 --topology minimal 增加一小节文档已排入下一轮。
| - Emit **no verdict** — no Approve / Request changes / Comment, and skip the open-Criticals re-check. Chunks that are uncoverable by `maxLineChars` are still listed under "Not reviewed". | ||
| - Step 7 never runs and cannot: the parser forced `comment.effective` to false for this topology. If the user asks to post the findings, decline and point at `/review <target> --effort high` (unverified findings must not be posted publicly). | ||
| - Step 6B never runs either: the parser forced `fix.effective` to false. If the user asks to apply the findings, point at `/review --fix`, which re-runs at medium with verified findings. | ||
| - In Step 8, save the report (marked `topology: minimal`) but do **not** write the incremental cache — this pass must never make a later full review report "No new changes since last review". Step 9 cleanup runs as usual. |
There was a problem hiding this comment.
[Suggestion] This bullet carves out only the incremental-cache write for Step 8, but Step 8's structured-artifact creation (references/persistence.md) is keyed to effort — "create and register the structured review artifact for medium and high effort" — and a minimal PR review resolves effort: "high" (topology overrides the dispatch, not the verdict's value; probe-verified). save-artifact requires inputs a minimal run never produces: --findings (written by Step 4 verification) and --composed (compose-review's output — minimal emits no verdict and never composes). Nothing argues minimal out of the rule the way low is argued out ("low has no canonical composed verdict and must not invent one" — minimal has none either).
Measured: driving the compiled saveReviewArtifact with a canonical findings fixture and no composed file at effort high refuses with Could not read the composed verdict file … ENOENT. So a minimal PR run following the literal rule spends its tail disclosing a fail-closed refusal and skipping record_artifact on error noise — or the orchestrator improvises the carve-out the prose never states, up to inventing a verdict the persistence rules explicitly forbid.
State the disposition here, e.g. "save no structured artifact — like low, minimal has no canonical composed verdict and must not invent one".
中文说明
[Suggestion] 这一条只为 Step 8 剔除了增量缓存的写入，但 Step 8 的结构化产物创建（references/persistence.md）是以 effort 为键的——「为 medium 和 high effort 创建并注册结构化评审产物」——而 minimal 的 PR 评审解析出的 effort 是 "high"（topology 覆盖的是分发，不是裁决里的值；已用探针验证）。save-artifact 需要的输入是 minimal 运行永远不会产生的：--findings（由 Step 4 验证写出）和 --composed（compose-review 的输出——minimal 不输出裁决、从不 compose）。没有任何文案像论证 low 那样把 minimal 排除在这条规则之外（「low 没有权威的 composed 裁决，不得虚构一个」——minimal 同样没有）。
实测：在 effort high 下以一个权威的 findings 夹具、不带 composed 文件驱动编译后的 saveReviewArtifact，得到拒绝 Could not read the composed verdict file … ENOENT。因此一个按字面规则执行的 minimal PR 运行，其收尾会花时间披露一次失败关闭的拒绝、并在错误噪音中跳过 record_artifact——或者 orchestrator 临场发挥出一段文案从未写过的例外处理，甚至虚构一个持久化规则明令禁止的裁决。
请在此处写明处置方式，例如「不保存结构化产物——与 low 一样，minimal 没有权威的 composed 裁决，不得虚构」。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real: Step 8's structured-artifact creation is keyed to effort ("medium and high"), a minimal PR review resolves effort: "high", and save-artifact requires --findings/--composed inputs a minimal run never produces — so the prose owes an explicit carve-out the way low has one. Stating the disposition in Step 3M's bullet ("save no structured artifact — like low, minimal has no canonical composed verdict and must not invent one") is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：Step 8 的结构化产物创建以 effort 为键（「medium 和 high」），minimal 的 PR 评审解析出 effort: "high"，而 save-artifact 需要的 --findings/--composed 输入是 minimal 运行永远不会产生的——因此文案需要像论证 low 那样给出显式豁免。在 Step 3M 的条目中写明处置方式（「不保存结构化产物——与 low 一样，minimal 没有权威的 composed 裁决，不得虚构」）已排入下一轮。
| - Step 7 never runs and cannot: the parser forced `comment.effective` to false for this topology. If the user asks to post the findings, decline and point at `/review <target> --effort high` (unverified findings must not be posted publicly). | ||
| - Step 6B never runs either: the parser forced `fix.effective` to false. If the user asks to apply the findings, point at `/review --fix`, which re-runs at medium with verified findings. |
There was a problem hiding this comment.
[Suggestion] Step 3M's adjustment list drops the follow-up-tip bullet that Step 3C has, while Step 6's post-verdict tip emission (whose preamble says "high and medium effort"), Step 6B's fix these issues intake, and Step 7's posting gate are keyed on effort and exclude only low's no-verdict shape — so a minimal run is told two contradictory things about the exact follow-ups it must decline.
A minimal LOCAL run resolves effort medium with findings present, so the tip rule for "Local review with unfixed findings" fires and emits "Tip: type fix these issues…", directing the user into a flow whose acceptance condition reads fix.effective — false for minimal — so the same run both offers and then refuses; the tip's preamble never names the topology. Symmetrically, a minimal PR run resolves effort high and emits "Tip: type post comments…"; Step 7's decline arm names only "low or medium" effort, so under minimal only this bullet opposes proceeding — the orchestrator declines here, proceeds otherwise, and when it proceeds the code write gate refuses the eventual submit with the misattributed refusal message (see the finding on line 823 of parse-args.ts). Nothing wrongful posts — the code gate holds — but the directives in context contradict each other on a contract this skill has measured drifting.
Give Step 3M the follow-up bullet Step 3C has (a tip offering only /review <target> --effort medium|high re-runs, explicitly declining interactive fix/post), and name the topology in the post-verdict tip preamble ("high and medium effort, topology: auto") and in Step 7's decline arm ("at low or medium effort, or under --topology minimal").
中文说明
[Suggestion] Step 3M 的调整清单丢掉了 Step 3C 拥有的那条「后续提示」条目，而 Step 6 的裁决后提示发射（其前言写着「high 和 medium effort」）、Step 6B 的 fix these issues 受理、以及 Step 7 的发布门控都以 effort 为键、只排除 low 的无裁决形态——于是 minimal 运行在它本应拒绝的后续操作上，同时收到两种互相矛盾的指令。
一个 minimal 的本地评审解析出 effort medium 且有 findings，于是「本地评审有未修复发现」的提示规则触发，输出「Tip: type fix these issues……」，把用户引向一个受理条件读取 fix.effective 的流程——该值对 minimal 为 false——于是同一次运行先提供、后拒绝；提示的前言从未点名 topology。对称地，minimal 的 PR 评审解析出 effort high，输出「Tip: type post comments……」；Step 7 的拒绝分支只点名「low 或 medium」effort，因此在 minimal 下只有本条反对继续——orchestrator 读到本条就拒绝，读不到就继续，而一旦继续，代码写门控会以那条归因错误的拒绝消息（见 parse-args.ts 823 行的发现）拒绝最终的 submit。没有任何错误的发布发生——代码门控守住了——但上下文中的指令在这个本技能实测漂移过的契约上互相矛盾。
请给 Step 3M 补上 Step 3C 拥有的后续提示条目（提示只提供 /review <target> --effort medium|high 重跑，并显式拒绝交互式 fix/post），并在裁决后提示的前言（「high 和 medium effort，topology: auto」）与 Step 7 的拒绝分支（「在 low 或 medium effort 下，或在 --topology minimal 下」）里点名该拓扑。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round, not declined — this round ran under a budget warning and was scoped to the two Critical findings only. The finding is acknowledged as real: Step 3M dropped Step 3C's follow-up-tip bullet while Step 6's tip emission, Step 6B's intake, and Step 7's posting gate key on effort and exclude only low's shape — so a minimal run both offers and then refuses the same follow-up. Giving Step 3M the follow-up bullet and naming the topology in the tip preamble and Step 7's decline arm is queued for the next round.
中文说明
推迟到下一轮，并非拒绝——本轮在预算警告下运行，范围被限定为仅处理两条 Critical 发现。该发现被承认为真实：Step 3M 丢掉了 Step 3C 的后续提示条目，而 Step 6 的提示发射、Step 6B 的受理与 Step 7 的发布门控都以 effort 为键、只排除 low 的形态——于是 minimal 运行在同一个后续操作上先提供、后拒绝。给 Step 3M 补上后续提示条目，并在提示前言与 Step 7 的拒绝分支中点名该拓扑，已排入下一轮。
|
🤖 AutoFix ran out of time before finishing (timeout (2700000ms)) (attempt 1/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (2700000ms)）（第 1/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32728002254 🧠 Handled by Qwen Code · model/模型 |
…opology # Conflicts: # packages/core/src/skills/bundled/review/SKILL.md
…ion disposition (#9919) Address the two Critical findings from round 1 of automated review: - `--resume` was left ungated under `--topology minimal` while its siblings `--comment` and `--fix` were forced ineffective. An effective resume would make `fetch-pr --resume` consume an interrupted pipeline run's lease and worktree for a pass that never continues it. Gate `resume.effective` on `!isMinimal` with a terminal-only warning mirroring the sibling flags, and name the new cause in the Step 1 verdict bullets. - Step 3M introduced a run shape no Step 9 completion-line disposition fits: minimal emits no verdict and resolves effort high, so neither the `<verdict>, not posted` nor the low-only `quick pass` form is contract-conformant. Add `minimal pass, not posted (<N> unverified findings)` to Step 9's enumerated list and reference it from Step 3M's adjustments.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9919 (review feedback, round 2)This round ran under a budget warning (the previous round exhausted its time budget before finishing), so per the workflow rule it addresses the smallest blocking subset — the two Critical findings — and defers the nine Suggestions to the next round via Also per the invocation ( Conflict resolutionOne conflict, in Note: the repository's own pre-commit hook (lint-staged → prettier) reformatted one line of Feedback dispositions
rc:3843520432 — Critical:
|
| id | 严重级 | 决定 |
|---|---|---|
| rc:3843520432 | Critical | 已在代码中解决 |
| rc:3843520443 | Critical | 已在代码中解决 |
| rc:3843520448 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520461 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520471 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520482 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520489 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520497 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520504 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520510 | Suggestion | 因预算推迟到下一轮 |
| rc:3843520512 | Suggestion | 因预算推迟到下一轮 |
rc:3843520432 — Critical：--topology minimal 下 --resume 未门控
先复现（来源无关规则）：对修复前代码的探针观察到 parseReviewArgs('1234 --resume --topology minimal') → resume: {requested: true, effective: true} 且零警告，与发现的描述完全一致，而 --comment/--fix 都带警告地被门控。
修复（最小化，在兄弟门控所在处）： resumeEffective = resumeRequested && isPr && !isMinimal，并仿照 comment/fix 增加一条仅终端的警告（「--resume is ignored because --topology minimal runs a fresh single pass — it neither continues nor consumes an interrupted run.」）。SKILL.md Step 1 中记录这些门控的两条裁决条目现在也点名第三个字段（resume.effective）以及新的 requested && !effective 成因。由于 resume.effective 为 false，Step 1 的 resume 分支不会把 --resume 追加到 fetch-pr，被中断运行的租约与 worktree 既不会被消耗也不会被销毁。新增的单元测试钉住了该门控与警告。
rc:3843520443 — Critical：Step 3M 的运行形态无法满足 Step 9 的完成行契约
先复现： 该声明中 effort 的一半已用探针验证——parseReviewArgs('6711 --topology minimal') → effort: "high", effortSource: "default"，因此 low 专属的 quick pass disposition 不可用，而其余三种枚举 disposition 分别需要 Step 7 提交、裁决或 Aone partial——minimal 运行一个都不会产生（已在被评审 commit 上对照 Step 9 列表原文确认）。
修复（采用发现提出的第一方案）： 在 Step 9 的枚举列表中新增第五种 disposition——minimal pass, not posted (<N> unverified findings)——标注仅限 --topology minimal，并附一句说明为何裁决形态与 quick-pass 形态都不适用；同时在 Step 3M 的调整清单中新增一条，指示完成行使用该 disposition。这让机器记录对本臂要产出的按模型 A/B 对比保持诚实。
推迟的 Suggestion
这九条 Suggestion 都是真实的且在范围内，但本轮受预算约束只能处理最小阻塞子集。每条推迟及其理由都记录在 comment-replies.json 中（发布在各自的线程上，线程保持打开），不会有任何一条被静默丢弃。
验证
本轮实际执行的命令，按顺序：
npm run build— 通过（exit 0；突变探针恢复后再次运行，exit 0）。npm run typecheck— 通过（exit 0）。npm run lint— 通过（exit 0，全仓库）。npx eslint packages/cli/src/commands/review/parse-args.ts packages/cli/src/commands/review/parse-args.test.ts— 通过（exit 0）。npx vitest run src/commands/review/parse-args.test.ts（packages/cli）— 134/134 通过（133 条既有 + 1 条新增）。- 突变探针（见证检查）： 临时移除
resumeEffective中的&& !isMinimal；新测试minimal gates --resume…失败（1 failed | 133 skipped），证明它见证了该守卫；恢复守卫后重跑整个文件——134/134 再次全部通过。 - 修复前复现探针（tsx）：
parseReviewArgs('1234 --resume --topology minimal')→{requested: true, effective: true}，warnings[]（R1-1 复现）；修复后 →{requested: true, effective: false}并带仅终端警告；parseReviewArgs('1234 --resume')与parseReviewArgs('--resume')行为不变（R1-2 的 effort 一半复现：6711 --topology minimal解析出effort: "high", effortSource: "default"）。 - 未改动任何设置源 → 不适用
npm run generate:settings-schema。被触碰的行为有直接的单元测试（并非只经由打包后的 CLI 验证）→ 无需运行集成测试。
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/skills/bundled/review/SKILL.md:68 — [probe] the comment verdict bullet's cause enumeration is stale — requested && !effective no longer implies a non-PR target (deferred by the code-age rule: unchanged since the previo…packages/cli/src/commands/review/parse-args.test.ts:928 — [probe] hollow 'minimal does not force effort the way --comment does' test — executed mutation survives 134/134 green (deferred by the code-age rule: unchanged since the previous rou…packages/cli/src/commands/review/parse-args.ts:54 — [review] ReviewTopology / --topology collides with the subsystem's established diff-shape 'topology' term of art (deferred by the code-age rule: unchanged since the previous round's head…
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| - Emit **no verdict** — no Approve / Request changes / Comment, and skip the open-Criticals re-check. Chunks that are uncoverable by `maxLineChars` are still listed under "Not reviewed". | ||
| - Step 7 never runs and cannot: the parser forced `comment.effective` to false for this topology. If the user asks to post the findings, decline and point at `/review <target> --effort high` (unverified findings must not be posted publicly). | ||
| - Step 6B never runs either: the parser forced `fix.effective` to false. If the user asks to apply the findings, point at `/review --fix`, which re-runs at medium with verified findings. | ||
| - In Step 8, save the report (marked `topology: minimal`) but do **not** write the incremental cache — this pass must never make a later full review report "No new changes since last review". Step 9 cleanup runs as usual. |
There was a problem hiding this comment.
[Critical] R1-10: (round 1 filed this as a Suggestion; round 2's probe-backed trace shows the mechanism fires on every run of the arm) Step 3M routes minimal runs into Step 8 with only the incremental-cache carve-out, but Step 8's structured-artifact obligation is keyed on effort ("create and register the structured review artifact for medium and high effort"), and a minimal run resolves effort independently — high for a PR target, medium for a local one. save-artifact requires a valid composed-verdict JSON (it throws on a missing file) and rejects only low; there is no topology awareness in save-artifact.ts, ReviewArtifactV1, or persistence.md. Minimal emits no verdict (Step 3M forbids one), so compose-review never runs and no composed JSON exists.
Every /review <pr> --topology minimal run therefore reaches save-artifact with a missing composed input: a fail-closed persistence-failure disclosure ends every run of the arm, or the orchestrator improvises (running compose-review against the no-verdict rule, or synthesizing inputs) and archives a single-pass unverified run as an effort:high verdict-bearing artifact — mislabeling the run in the durable store and poisoning the per-model A/B comparison data this arm exists to produce.
Witness (probe against this commit):
parseReviewArgs('6711 --topology minimal') → { effort: 'high', topology: 'minimal' }
parseReviewArgs('src/foo.ts --topology minimal') → { effort: 'medium' }
— both tiers the artifact rule keys on; save-artifact.ts's only effort rejection is if (args.effort === 'low') throw, and grep for topology|minimal in persistence.md returns zero matches.
Suggested fix: carve the artifact out explicitly in Step 3M's Step 8 bullet, keyed on topology since effort cannot carry it — "save the report (marked topology: minimal) but do not create or register the structured artifact and do not write the incremental cache — the artifact persists a composed verdict, and this pass emits none" — and add the matching topology line to persistence.md's report-content list.
中文说明
[Critical] R1-10：（第 1 轮将此条记为 Suggestion；第 2 轮带探针的追踪表明该机制在极简臂的每次运行上都会触发）Step 3M 把极简运行带进 Step 8 时只挖掉了增量缓存这一项，但 Step 8 的结构化产物义务是按 effort 键控的（「为 medium 和 high effort 创建并注册结构化评审产物」），而极简运行的 effort 是独立解析的——PR 目标为 high，本地目标为 medium。save-artifact 要求一个有效的 composed 裁决 JSON（文件缺失时直接抛错），且只拒绝 low；save-artifact.ts、ReviewArtifactV1 和 persistence.md 都没有任何 topology 感知。极简臂不产生裁决（Step 3M 明确禁止），所以 compose-review 永远不会运行，也就没有 composed JSON。
因此每一次 /review <pr> --topology minimal 运行都会在 composed 输入缺失的情况下走到 save-artifact：要么以一次失败关闭（fail-closed）的持久化失败披露结束该臂的每次运行，要么 orchestrator 临场发挥（违反「禁止裁决」规则去跑 compose-review，或凭空合成输入），把一个单遍未验证的运行归档为带裁决的 effort:high 产物——在持久化存储中给这次运行错误贴标签，并污染本臂存在的全部意义（按模型对比的 A/B 数据）。
证据（对本 commit 的探针）：parseReviewArgs('6711 --topology minimal') → { effort: 'high', topology: 'minimal' }；parseReviewArgs('src/foo.ts --topology minimal') → { effort: 'medium' }——正是产物规则所键控的两个档位；save-artifact.ts 唯一的 effort 拒绝是 if (args.effort === 'low') throw；在 persistence.md 中 grep topology|minimal 为零匹配。
建议修复：在 Step 3M 的 Step 8 条目中显式挖掉产物义务，并以 topology 为键（effort 承载不了）——「保存报告（标记 topology: minimal），但不要创建或注册结构化产物，也不要写增量缓存——产物持久化的是一个组合裁决，而本通路不产生裁决」——并在 persistence.md 的报告内容列表中补上对应的 topology 一行。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // suppressed: on a non-PR target the comment does not apply anyway, and | ||
| // that case keeps its usual handling above. | ||
| warnings.push( | ||
| 'Warning: `--comment` is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.', |
There was a problem hiding this comment.
[Suggestion] R1-3: This warning branch fires on commentRequested (commentRequestedByFlag || defaults.comment === true), but its text always names the --comment flag. An operator with review.comment: true in settings who runs /review 6711 --topology minimal with no flag is told "--comment is ignored" — a flag they never typed — and goes hunting for a stray flag or doubts the parser instead of recognizing their setting was suppressed. The same function already distinguishes the two origins in its forced-by-comment warning via commentRequestedByFlag ("--comment requires…" vs "review.comment is enabled in settings…"), and lib/authorization.ts explicitly warns against sending a setting-driven operator to type a flag they never needed. Probe-confirmed: parseReviewArgs('6711 --topology minimal', { comment: true }) returns the flag-naming warning.
| 'Warning: `--comment` is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.', | |
| warnings.push( | |
| commentRequestedByFlag | |
| ? 'Warning: `--comment` is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.' | |
| : 'Warning: the `review.comment` setting is ignored because `--topology minimal` is terminal-only — the minimal arm posts nothing.', | |
| ); |
中文说明
[Suggestion] R1-3：该警告分支基于 commentRequested（commentRequestedByFlag || defaults.comment === true）触发，但文案始终点名 --comment 标志。一位在 settings 中设置了 review.comment: true 的操作者，在不带任何标志运行 /review 6711 --topology minimal 时，会被告知「--comment 被忽略」——一个他从未输入过的标志——于是去排查是否多打了标志，或怀疑解析器出错，而不是意识到是自己的设置被抑制。同一函数在 forced-by-comment 警告中已经通过 commentRequestedByFlag 区分了两种来源（「--comment requires…」与「review.comment is enabled in settings…」），且 lib/authorization.ts 明确告诫不要让设置驱动的操作者去输入一个他根本不需要的标志。已用探针确认：parseReviewArgs('6711 --topology minimal', { comment: true }) 返回点名标志的警告。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
|
||
| const commentRequested = commentRequestedByFlag || defaults.comment === true; | ||
| const commentEffective = commentRequested && isPr; | ||
| const commentEffective = commentRequested && isPr && !isMinimal; |
There was a problem hiding this comment.
[Suggestion] R1-4: !isMinimal adds a third cause for comment.effective === false, but the downstream consumer — the write gate's slow path in lib/authorization.ts (~lines 356-372, untouched by this diff) — composes its refusal why from only the two pre-existing causes. Probe-confirmed: parseReviewArgs('6711 --topology minimal --comment') yields a pr-number target with comment.requested: true / comment.effective: false; the slow path then takes the commentRequested branch and returns "the review arguments (…) do not name a pull request, so they cannot authorise posting to one" — factually wrong, and that block's own comment requires the refusal to "name the REAL blocker". The refusal itself stays fail-closed correct; only the diagnosis misdirects — the operator is sent to fix a target-shape problem that does not exist, and re-running with identical args refuses again for the unnamed reason. Fix lives in authorization.ts: branch on the re-parsed verdict's topology === 'minimal' before the target-shape message, e.g. "the recorded review ran with --topology minimal, which is terminal-only and cannot authorise posting — re-run without it".
中文说明
[Suggestion] R1-4：!isMinimal 为 comment.effective === false 增加了第三个成因，但下游消费方——lib/authorization.ts 中写授权的慢路径（约 356-372 行，本 diff 未触碰）——仍然只按既有的两个成因来组织拒绝理由（why）。已用探针确认：parseReviewArgs('6711 --topology minimal --comment') 得到 pr-number 目标，且 comment.requested: true / comment.effective: false；慢路径随后走入 commentRequested 分支，返回「评审参数（…）没有指定 pull request，因此不能授权发布」——与事实不符，而该代码块自身的注释要求拒绝必须「点名真正的阻塞原因」。拒绝本身仍是失败关闭（fail-closed）且正确的；只有诊断误导了方向——操作者被引导去修复一个并不存在的目标形态问题，用相同参数重跑仍会因这个未点名的原因再次被拒。修复位置在 authorization.ts：在目标形态文案之前，先按重新解析出的裁决中的 topology === 'minimal' 分支，例如「记录的评审运行使用了 --topology minimal，它是仅终端输出的，不能授权发布——请去掉它重跑」。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| continue; | ||
| } | ||
|
|
||
| if (token === '--topology' || token.startsWith('--topology=')) { |
There was a problem hiding this comment.
[Suggestion] R1-5: This block is a third near-verbatim copy of the file's value-token grammar — the ~35-line first-pass parse block (beside --effort at line 488 and --severity-floor at line 535), a third lowercase-and-set-check validator (asTopology), a third four-case EffortIssue deferred-warning switch, and the rescue-loop issue-list mapping now grown into a positional nested ternary. No shared helper exists (grepped). The file's own comments document four prior grammar-fix rounds (round-8 equals-form parity, round-9 distinct-target dedupe, round-10 URL-preferred rescue, round-12 mixed-shape restatement), each of which had to land consistently in every flag's copy. The next such fix — or a fourth value-taking flag — landing in two of the three copies makes rescue/warning behaviour silently depend on which flag the user typed, the exact syntax-dependent divergence the round-8 comment forbids ("which codebase gets reviewed cannot depend on which syntax happened to be typed"). Cost today: ~90 duplicated lines plus a lockstep-maintenance tax on every future grammar fix. Extract one helper (flag name, an as* validator, the flag's issue list, a setter; returns tokens consumed) called by all three flags.
中文说明
[Suggestion] R1-5：本块是该文件中值-令牌语法的第三份近乎逐字拷贝——约 35 行的第一遍解析块（与 488 行的 --effort、535 行的 --severity-floor 并列）、第三个「小写化+集合校验」验证器（asTopology）、第三个四分支 EffortIssue 延迟警告 switch，以及救援循环中如今长成位置式嵌套三元运算符的 issue 列表映射。不存在可复用的共享助手（已 grep 确认）。本文件自身的注释记录了此前四轮语法修复（round-8 等号形态对齐、round-9 相同目标去重、round-10 URL 优先救援、round-12 混合形态重述），每一轮都必须在每个标志的拷贝中一致落地。下一次同类修复——或第四个取值标志——只要落在三份拷贝中的两份上，就会让救援/警告行为悄悄取决于用户输入的是哪个标志，正是 round-8 注释所禁止的语法依赖型分歧（「评审哪个代码库不能取决于碰巧输入了哪种语法」）。当前成本：约 90 行重复代码，外加未来每次语法修复的同步维护税。建议抽取一个助手函数（标志名、一个 as* 验证器、该标志的 issue 列表、一个设置器；返回消耗的令牌数），三个标志共用。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round. The extraction is the right fix, but it is this batch's largest change — ~90 lines of churn across three hardened copies of the value-token grammar that carry four documented grammar-fix rounds — and this round already carries the Critical (R1-10) plus seven other findings, at the ~8-finding batch bound. De-risking for it landed this round: the three --topology grammar branches that had no witnesses (equals-form PR-shaped rescue, quoted-empty consumption, flag-final/flag-followed missing value) are now pinned by sibling-mirroring probes in the --topology describe, so the helper refactor lands against full branch coverage and any divergence fails loudly.
中文说明
延至下一轮处理。抽取助手函数是正确的修复方向，但它是本批中最大的改动——约 90 行的改动横跨值-令牌语法的三份已加固拷贝（各有四份有据可查的语法修复记录）——而本轮已承载 Critical（R1-10）加另外七条发现，达到每轮约 8 条的批次上限。本轮已为其提前降低风险：原先没有证据的三个 --topology 语法分支（等号形态的 PR 形状救援、引号空值消耗、标志结尾/标志跟随的缺值分支）现在已由 --topology describe 中镜像兄弟标志的探针钉住，因此助手函数重构将在完整的分支覆盖下落地，任何分歧都会大声失败。
| const issues = | ||
| k.invalidValueOf === '--effort' | ||
| ? effortIssues | ||
| : k.invalidValueOf === '--severity-floor' | ||
| ? floorIssues | ||
| : topologyIssues; |
There was a problem hiding this comment.
[Suggestion] R1-6: Adding the third flag grew the flat two-way ternary into a nested ternary whose fall-through silently maps invalidValueOf === undefined (plain kept tokens) to topologyIssues. Confirmed inert today — every read of issues sits inside the if (k.invalidValueOf !== undefined) guard below — but a reader must verify that to trust the mapping, and a refactor hoisting issues out of the guard would silently feed the wrong list; each future value-taking flag deepens the nest. The flat equivalent is a lookup keyed by flag name, resolved inside the guard:
const issueListFor: Record<string, EffortIssue[]> = {
'--effort': effortIssues,
'--severity-floor': floorIssues,
'--topology': topologyIssues,
};
// inside `if (k.invalidValueOf !== undefined)`:
const issues = issueListFor[k.invalidValueOf];中文说明
[Suggestion] R1-6：第三个标志的加入把原本的扁平二元三元运算膨胀成了嵌套三元，其 fall-through 会把 invalidValueOf === undefined（普通保留令牌）悄悄映射到 topologyIssues。今天确认为惰性无害——issues 的每一处读取都位于下方 if (k.invalidValueOf !== undefined) 守卫之内——但读者必须自行验证这一点才能信任该映射；一旦有重构把 issues 提出守卫之外，就会悄悄传入错误的列表；未来每增加一个取值标志都会让嵌套更深。等价的扁平写法是以标志名为键的查表，在守卫内解析。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| } else if (value !== '' && isPrShapedToken(value)) { | ||
| kept.push({ token: value, invalidValueOf: '--topology' }); |
There was a problem hiding this comment.
[Suggestion] R1-7: Three branches of this copy of the value-token grammar have no topology-specific tests, while both sibling flags carry named regression probes: the equals-form PR-shaped rescue (sibling probe at test line 682), the quoted-empty consumption (line 739), and the flag-final/flag-followed missing-value branch (lines 208/218/769). The topology rescue test exercises only the spaced form --topology 6711. Probe-verified that all three branches behave correctly today — this is a guard gap, not a live bug — but three concrete mutations survive with the suite green: delete the equals-form rescue branch and /review --topology=6711 warns invalid and reviews the local tree instead of rescuing PR 6711 (the Round-8 regression the sibling probes pin); delete the quoted-empty branch and /review --topology "" yields an empty-string file target instead of a requires-a-value warning; delete the missing-value branch and /review 6711 --topology eats the following flag/undefined into the kept pool so --comment never registers. Extend the --topology describe with the three sibling probes (--topology=6711 → pr-number target; --topology "" bare and after a target → requires-a-value warning, never an empty-string file target; 6711 --topology and 6711 --topology --comment → warning with --comment still requested).
中文说明
[Suggestion] R1-7：这份值-令牌语法拷贝中有三个分支没有 topology 专属测试，而两个兄弟标志都有点名的回归探针：等号形态的 PR 形状救援（兄弟探针在测试 682 行）、引号空值消耗（739 行）、以及标志结尾/标志跟随的缺值分支（208/218/769 行）。topology 的救援测试只覆盖了空格形态 --topology 6711。已用探针确认三个分支目前行为全部正确——这是防护缺口，不是现网缺陷——但三个具体突变可以在测试套件全绿的情况下存活：删掉等号形态救援分支，/review --topology=6711 会报无效值并转而评审本地目录，而不是救援出 PR 6711（正是兄弟探针钉住的 Round-8 回归）；删掉引号空值分支，/review --topology "" 会得到一个空字符串文件目标而不是「需要一个值」的警告；删掉缺值分支，/review 6711 --topology 会把后续标志/undefined 吞进保留池，导致 --comment 永远无法注册。请在 --topology 的 describe 中补上三个兄弟探针（--topology=6711 → pr-number 目标；--topology "" 单独出现及跟在目标后 → 需要值的警告，绝不产生空字符串文件目标；6711 --topology 与 6711 --topology --comment → 警告且 --comment 仍为 requested）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // than in whichever prose the orchestrator reads. | ||
| const isMinimal = topology === 'minimal'; | ||
|
|
||
| const commentRequested = commentRequestedByFlag || defaults.comment === true; |
There was a problem hiding this comment.
[Suggestion] R1-8: The gate below is deliberately written over commentRequested (flag OR the review.comment setting), and probe confirms it suppresses a settings-driven comment today — but every added test requests --comment only via the flag: the settings-defaults block (test lines 937+) has comment: true cases at lines 963/972/996/1040, none combined with topology, and the topology block plus the wiring test use the flag only. A future edit narrowing this line to commentRequestedByFlag (or refactoring commentRequested) would let a settings-driven comment request bypass the minimal gate — the terminal-only arm would then be able to post — while every existing test stays green. Add one test: defaults { comment: true } + '6711 --topology minimal' → comment.requested true, comment.effective false, with the minimal-suppression warning.
中文说明
[Suggestion] R1-8：下面的门控刻意写在 commentRequested（标志 或 review.comment 设置）之上，探针确认它今天确实能抑制设置驱动的请求——但所有新增测试都只通过标志请求 --comment：settings 默认值块（测试 937 行起）在 963/972/996/1040 行有 comment: true 用例，没有一个与 topology 组合；topology 块与接线测试也只用标志。未来若有编辑把本行收窄为 commentRequestedByFlag（或重构 commentRequested），设置驱动的评论请求就会绕过 minimal 门控——仅终端输出的臂将能够发帖——而所有现有测试依然全绿。请补一个测试：默认值 { comment: true } + '6711 --topology minimal' → comment.requested 为 true、comment.effective 为 false，并带 minimal 抑制警告。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
|
||
| ## LLM call budget | ||
|
|
||
| **`--topology minimal` — 0 subagent calls.** The minimal arm (issue #9783, Step 3M) is a single careful pass over the diff in the orchestrator's own context — no fan-out, no verification, no reverse audit, no build/test. It costs one model turn, the same shape as the low tier's inline pass, and it is priced here for completeness, not as a recommended default: it exists so the full pipeline and this minimal prompt can be run over the same PR set and compared per model. This section tracks per-topology _cost_; the A/B the minimal arm enables extends it with per-model _quality_, which is what decides whether any cell of a future model-family × effort routing table routes away from the full pipeline. Until that data exists, every review runs the topology below. |
There was a problem hiding this comment.
[Suggestion] R1-9: The minimal arm shares all three properties this prose reserves for low — labeled unverified, emits no verdict, never writes the incremental cache — but the enumeration ~11 lines above ("…which since medium became a verified fan-out means low alone") still says the set has one member, and this diff (which adds the pricing paragraph above) did not update it. A reader trusting that enumeration miscounts the set of unverified/no-verdict/no-cache arms — e.g. writing future prose or a check keyed on "the one unverified arm" — in exactly the doc that prices the second such arm. Suggested fix: update the enumeration to name both arms, e.g. "means low and --topology minimal alone".
中文说明
[Suggestion] R1-9：极简臂具备这段文案为 low 保留的全部三个属性——标注为未验证、不产生裁决、从不写增量缓存——但上方约 11 行处的枚举（「…自 medium 成为带验证的扇出之后，就只剩 low alone」）仍然声称该集合只有一个成员，而本 diff（在上方新增了定价段落）没有更新它。信任该枚举的读者会少数「未验证/无裁决/无缓存」臂的数目——例如在正是为第二个此类臂定价的文档里，写下以「唯一未验证臂」为键的后续文案或检查。建议修复：把枚举更新为同时点名两个臂，例如「就只剩 low 与 --topology minimal」。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
|
||
| Then skip Steps 4 and 5 entirely and go to Step 6 with these adjustments: | ||
|
|
||
| - Use Step 6's structure, but label the review **"Minimal pass (topology: minimal) — findings are unverified"** (translated per output language) in the Summary, and skip verification stats (there was no verification). |
There was a problem hiding this comment.
[Suggestion] R1-11: This adjustment list exempts the verdict, verification stats, Step 7, Step 6B, the cache, and the completion line — but not Step 6's follow-up tips. Step 6's tip emission keys its PR tips on comment.effective being false (which the parser now forces for every minimal run) under a "high and medium effort" preamble a minimal PR run satisfies (effort resolves high), and the decline carve-outs in Step 6/Step 7 name only low/medium effort, never minimal. The parallel low arm got an explicit carve-out; minimal got none. Failure scenario: a minimal PR review with findings emits "Tip: type post comments to publish findings as PR inline comments." — inviting exactly the action this arm declines; when the user types the trigger phrase, Step 6's "proceed directly to Step 7" handler routes toward posting. Worse, verified against the code: lib/authorization.ts's --user-authorized fast path returns ok WITHOUT consulting comment.effective or topology, and posting.md instructs passing it precisely when the user asks in-session to publish — so on that flow the single prose bullet declining the post is the entire protection; nothing in code stands between the tip and a post of unverified findings. Suggested fix: add a tip carve-out for minimal mirroring low's ("Offer no follow-up tip from Step 6's list; the only follow-up this arm offers is the pointer to /review <target> --effort high"), and/or add "or under --topology minimal" to the Step 6/Step 7 decline lists.
中文说明
[Suggestion] R1-11：这份调整清单豁免了裁决、验证统计、Step 7、Step 6B、缓存和完成行——却没有豁免 Step 6 的后续提示（follow-up tip）。Step 6 的提示发射以 comment.effective 为 false 作为 PR 提示的触发条件（解析器现在对每个极简运行都强制为 false），其「high 和 medium effort」前言又恰被极简 PR 运行满足（effort 解析为 high），而 Step 6/Step 7 中的拒绝豁免只点名 low/medium effort，从不点名 minimal。平行的 low 臂有显式豁免；minimal 没有。失败场景：一次带发现的极简 PR 评审会输出「Tip: type post comments to publish findings as PR inline comments.」——恰好邀请本臂所拒绝的动作；当用户输入该触发短语时，Step 6 的「直接进 Step 7」处理器会把运行导向发帖。更糟的是，经代码核实：lib/authorization.ts 的 --user-authorized 快速路径会直接返回 ok，完全不查询 comment.effective 或 topology，而 posting.md 恰好要求在用户于会话中请求发布时传入它——因此在那条流程上，拒绝发帖的单条文案就是全部防线；在这条提示与「发布未验证发现」之间没有任何代码屏障。建议修复：仿照 low 为 minimal 增加提示豁免（「不提供 Step 6 列表中的任何后续提示；本臂唯一提供的后续指引是 /review <target> --effort high」），和/或在 Step 6/Step 7 的拒绝列表中加上「或处于 --topology minimal 之下」。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9919 (
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/parse-args.ts:888 — [review] PR-target --fix warning pre-empts the minimal branch and advises --comment , itself disabled under minimalpackages/core/src/skills/bundled/review/SKILL.md:3 — [review] no section for --topology minimal in docs/users/features/code-review.md while four sibling selectors have onepackages/core/src/skills/bundled/review/SKILL.md:607 — [review] Step 6's format intro restates Step 3C's adjustments for low but has no minimal clause, while a minimal PR run resolves effort highpackages/core/src/skills/bundled/review/DESIGN.md:693 — [review] the minimal arm's 'one model turn' pricing does not reproduce — turns scale with the chunk planpackages/cli/src/commands/review/parse-args.ts:845 — [review] --severity-floor is the one posting-governing knob neither gated nor warned under minimalpackages/core/src/skills/bundled/review/SKILL.md:1071 — [review] Step 9's normative completion-line paragraph still prescribes a verdict-carrying not posted form that matches a finished minimal runpackages/core/src/skills/bundled/review/SKILL.md:605 — [review] Step 3M routes minimal into Step 6 without excluding the script-lint and Test Plan gates, whose sole consumer (compose-review) never runspackages/core/src/skills/bundled/review/SKILL.md:68 — [review] the comment bullet's cause enumeration is stale — minimal is a second cause of requested && !effective on a PR targetpackages/core/src/skills/bundled/review/SKILL.md:1071 — [review] minimal x explicit/configured low/medium effort fires both effort-keyed clauses — two exclusive completion-line dispositions match one runpackages/cli/src/commands/review/parse-args.ts:43 — [review] topology overloads the subsystem's established diff-shape 'topology' (3A/3B) with no disambiguationpackages/cli/src/commands/review/parse-args.ts:569 — [review] R1-5 third near-verbatim copy of the value-flag grammar — still stands, author deferred the extraction
Convergence: round 3 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 9 (0 new). Findings keep coming back to the same files: packages/core/src/skills/bundled/review/SKILL.md (findings in round 1; 2 more now); packages/core/src/skills/bundled/review/DESIGN.md (findings in round 1; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 11 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 9 条（其中 0 条首次提出）。发现反复回到同一批文件：packages/core/src/skills/bundled/review/SKILL.md（第 1 轮已出过发现，本轮又有 2 条）；packages/core/src/skills/bundled/review/DESIGN.md（第 1 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| - **Three levels (chosen):** **low** = 3-6 directed angles (per `plan.budget.inlineAngles`) plus a gap sweep, all in the orchestrator's own context over the chunk plan — hunk-visible bugs only, ≤10 unverified findings. **medium** = the high pipeline minus its most expensive passes: the parallel finder fan-out over a reduced dimension set (no adversarial personas, no Agent 8), build & test, and a single verification pass — verified findings, Approve capped at Comment, no reverse audit. **high** = the full pipeline, unchanged. | ||
|
|
||
| **Guardrails, because an unverified pass is recall-limited by construction.** These guardrails defend against findings that no verifier ever checked, which since medium became a verified fan-out means **low alone**; medium shares only the cache and posting rules (its Approve cap is Step 6's own rule, not one of these). | ||
| **Guardrails, because an unverified pass is recall-limited by construction.** These guardrails defend against findings that no verifier ever checked, which since medium became a verified fan-out means **low and `--topology minimal` alone**; medium shares only the cache and posting rules (its Approve cap is Step 6's own rule, not one of these). |
There was a problem hiding this comment.
[Suggestion] This commit's line scopes the guardrail bullets over --topology minimal, but the second bullet it now covers — "Never posts to the PR: --comment forces high, and a 'post comments' follow-up after a quick pass is declined" — states a mechanism that is false for the minimal arm. Under --topology minimal the parser forces comment.effective to false with a terminal-only warning, and the forced-by-comment block keys on commentEffective, so nothing is ever promoted to high — pinned by this PR's own test "minimal does not force effort the way --comment does". A maintainer auditing why a minimal run cannot post would conclude --comment upgrades the run into the high pipeline — the exact opposite of the shipped gate — and a later change "aligning" code with this doc could force-promote minimal+--comment to high, silently converting the A/B arm into a full pipeline run and invalidating the experiment it exists to run. Split the bullet's mechanism per arm, e.g.: "Never posts to the PR: --comment forces high at low effort, and the parser forces comment.effective to false on the minimal arm (terminal-only); a 'post comments' follow-up is declined in both."
中文说明
本提交把护栏条目扩展到了 --topology minimal，但它现在涵盖的第二条——「从不发帖到 PR：--comment 会强制提升为 high，且 quick pass 之后的 'post comments' 追问会被拒绝」——所陈述的机制对极简臂是错误的。在 --topology minimal 下，解析器会把 comment.effective 强制置为 false 并给出「仅终端」警告，而 forced-by-comment 逻辑以 commentEffective 为条件，因此极简臂永远不会被提升为 high——本 PR 自己的测试 "minimal does not force effort the way --comment does" 钉住了这一点。一位想弄清极简运行为何不能发帖的维护者读到这里，会得出 --comment 会把运行升级为 high 管线的结论——与实际上线的门控恰好相反；后续若有改动按此文档「对齐」代码，就可能把 minimal+--comment 强制提升为 high，悄悄把 A/B 臂变成一次全量管线运行，从而废掉本臂存在的全部意义。建议把该条目的机制按臂拆分，例如：「从不发帖到 PR：low effort 下 --comment 强制提升为 high；极简臂下解析器强制 comment.effective 为 false（仅终端）；两种情况下 'post comments' 追问都会被拒绝。」
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (verdict.topology === 'minimal') { | ||
| return { | ||
| ok: false, | ||
| why: |
There was a problem hiding this comment.
[Suggestion] This new minimal refusal sits inside !verdict.comment.effective ahead of both the target-shape refusal and the PR-number binding check, so it fires for any minimal record even when the topology is not the binding blocker — and its remedy ("re-run the review without it") cannot lift the refusal in two subcases. Probe-verified on this commit: (1) record src/foo.ts --topology minimal (a supported combination the new tests pin) then submit — the refusal names the topology; the operator re-runs without it and is refused again with the target-shape wording, because a file target can never authorise posting; (2) record 123 --topology minimal --comment with submit targeting #456 — the topology wording fires first; only after re-running without the flag does the real blocker appear ("authorise pull request #123, but this submission targets #456"). Probe output: record 123 --topology minimal --comment + submit #456 → ok:false with the topology wording; the remedy's resulting state (123 --comment) + submit #456 → ok:false with the binding wording; control (456 --comment, submit #456) → ok:true. The gate still fails closed in every arm — only the wording misdirects, in the same class this commit's own comment says the refusal exists to eliminate ("name the REAL blocker"). Only take the topology wording when the topology is the sole blocker: when the recorded target is a PR, compare its number/repo against req.pr/req.repo first and lead with the binding refusal (or name both); let non-PR records fall through to the target-shape wording.
中文说明
新增的 minimal 拒绝分支位于 !verdict.comment.effective 内部，且排在目标形态拒绝与 PR 编号绑定检查之前，因此任何 minimal 记录都会先命中它——即使 topology 并不是真正的阻塞——而它给出的补救（「去掉该参数重跑评审」）在两种子情形下无法解除拒绝。已在本 commit 上用探针验证：(1) 记录 src/foo.ts --topology minimal（新测试钉住的合法组合）后提交——拒绝信息点名 topology；操作者去掉参数重跑，会再次被拒（这次是目标形态文案），因为文件目标永远无法授权发帖；(2) 记录 123 --topology minimal --comment 而 submit 目标是 #456——先命中 topology 文案；只有去掉参数重跑后才会看到真正的阻塞（「授权的是 PR #123，但本次提交目标是 #456」）。探针输出：记录 123 --topology minimal --comment + 提交 #456 → ok:false（topology 文案）；按补救去掉参数后的状态（123 --comment）+ 提交 #456 → ok:false（绑定文案）；对照（456 --comment 提交 #456）→ ok:true。门控在所有分支上仍然是失败即关闭的——只有文案误导了方向，而这正是本 commit 自己的注释声称要消除的那一类问题（「点名真正的阻塞」）。建议只在 topology 是唯一阻塞时才使用 topology 文案：当记录的目标是 PR 时，先把其编号/仓库与 req.pr/req.repo 比对，以绑定拒绝为先（或两者都点名）；非 PR 记录则落到目标形态文案。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
|
||
| - Never run a `gh` command that writes to the pull request — nor an `a1` command that writes to the MR — `qwen review submit` is the only write path in this skill, and it refuses when the run is not authorised. The one carve-out is Step 4's render-adjudication post to the user-designated `QWEN_REVIEW_SCRATCH_REPO` — that repo, that check, nothing else. | ||
| - Posting is a PR-only, high-only action: on a non-PR target there is nothing to post to, and at **low or medium** effort a "post comments" follow-up is declined with a pointer at `--effort high` (low's findings are unverified; medium's verdict is capped at Comment — `--comment` forces high). | ||
| - Posting is a PR-only, high-only action: on a non-PR target there is nothing to post to, and at **low or medium** effort — or under `--topology minimal` — a "post comments" follow-up is declined with a pointer at `--effort high` (low's findings are unverified; medium's verdict is capped at Comment — `--comment` forces high; minimal's findings are unverified and the arm posts nothing). |
There was a problem hiding this comment.
[Suggestion] This commit added --topology minimal to the posting-skip enumeration here, but the Step 7 authority file references/posting.md — which Step 7 says to load and "follow" the moment posting goes live — still keys on effort alone: its skip rule ("Also skip this step … if the review target is not a PR, or if the review ran at low or medium effort") and its load header ("the user asked this session to post the comments — on a PR target at high effort") never mention the arm, and the arm posts nothing at any effort. Concrete path: /review 123 --topology minimal resolves effort high (minimal deliberately never changes effort — pinned by this PR's own test) while all findings stay unverified; the user types "post comments"; if the orchestrator misses the single-clause decline in the trigger-phrase handler — the model-adherence failure mode these layered defenses exist for — it enters Step 7, posting.md's load condition is affirmatively met (user asked, PR target, high effort), its skip rule does not catch a high-effort minimal run, and submit --user-authorized takes the topology-blind fast path (probe: ok:true for a minimal recording, topology never consulted) — unverified findings posted to the public PR. Before this PR the shape could not exist: every high-effort run was verified, and every unverified run (low) was caught by the skip rule. Add the arm to posting.md's skip rule and mirror it in the load header: "…or if the review ran at low or medium effort, or under --topology minimal at any effort (its findings are unverified and the arm posts nothing — decline and point at --effort high)".
中文说明
本提交在这里的发帖跳过枚举中加入了 --topology minimal，但 Step 7 的权威文件 references/posting.md——Step 7 要求一旦发帖生效就加载并「遵循」它——仍然只以 effort 为键：其跳过规则（「如果评审目标不是 PR，或评审运行在 low 或 medium effort，则跳过本步骤」）与加载头部（「用户在本次会话中要求发帖——目标为 PR 且 effort 为 high」）都未提及该臂，而该臂在任何 effort 下都不发帖。具体路径：/review 123 --topology minimal 解析出 effort 为 high（minimal 刻意不改变 effort——本 PR 自己的测试钉住了这一点），同时所有发现都未经验证；用户输入 "post comments"；如果 orchestrator 漏看了触发短语处理器里那一条拒绝子句——正是这种分层防线所要应对的模型遵循失败——它就会进入 Step 7，posting.md 的加载条件被正向满足（用户要求、PR 目标、high effort），其跳过规则拦不住 high effort 的 minimal 运行，而 submit --user-authorized 走的是对 topology 无感知的快速路径（探针：minimal 记录返回 ok:true，全程不查询 topology）——未验证的发现就被发到了公开 PR 上。在本 PR 之前这种形态不可能存在：所有 high effort 运行都是已验证的，而所有未验证运行（low）都会被跳过规则拦住。建议在 posting.md 的跳过规则中补上该臂，并同步到加载头部：「……或评审运行在 low/medium effort，或在任何 effort 下使用了 --topology minimal（其发现未经验证且该臂不发帖——拒绝并指向 --effort high）」。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| @@ -1003,14 +1030,14 @@ Append a follow-up tip after the verdict (high and medium effort — only a **lo | |||
|
|
|||
| If the user responds with "fix these issues" (local review only), use the `edit` tool to fix each remaining finding interactively based on the suggested fixes from the review — do NOT re-run Steps 1-6. This is the same work Step 6B does; when the review has a findings artifact, record the outcomes into it the same way (`review findings --outcomes`) rather than leaving the list and the tree disagreeing about what was applied. | |||
There was a problem hiding this comment.
[Suggestion] The very hunk that added "Under --topology minimal, decline per Step 3M instead" to the adjacent "post comments" trigger-phrase handler left this "fix these issues" handler unconditional, so Step 6's interactive-fix instruction now contradicts Step 3M's no-edit guarantee for the minimal arm — the same shape this hunk closed for posting, one line away. Concrete path: a user runs /review src/foo.ts --topology minimal; the parser prints "--fix is ignored because --topology minimal is terminal-only — the minimal arm edits nothing", and Step 3M withholds all tips (apply requests point at /review --fix). The user then types fix these issues — a documented trigger phrase that needs no tip to invoke. This handler says "use the edit tool to fix each remaining finding interactively … do NOT re-run Steps 1-6" with no minimal carve-out, so up to fifteen findings that had no verification, no build/test, and no reverse audit are applied to the user's working tree — violating the "edits nothing" guarantee printed minutes earlier, and the same mistake class the fix-floor comment names ("editing the user's files on the strength of an unverified finding is the same mistake as posting one"). Step 3C's low tier differs by design: it explicitly offers the fix these issues tip, so the handler firing there is intended; Step 3M deliberately withholds it. Mirror the clause added one line down: "Under --topology minimal, decline per Step 3M instead — the findings are unverified; point at /review --fix, which re-runs at medium with verified findings."
中文说明
正是这个 hunk 在相邻的 "post comments" 触发短语处理器上加了「在 --topology minimal 下按 Step 3M 拒绝」，却让这个 "fix these issues" 处理器保持无条件执行，于是 Step 6 的交互式修复指令现在与 Step 3M 对极简臂的「不做任何编辑」保证相矛盾——和这个 hunk 为发帖关闭的形态一模一样，只差一行。具体路径：用户运行 /review src/foo.ts --topology minimal；解析器输出「--fix 被忽略，因为 --topology minimal 仅终端输出——极简臂不编辑任何东西」，且 Step 3M 不提供任何提示（应用修复的请求指向 /review --fix）。随后用户输入 fix these issues——这是一个文档化的触发短语，不需要提示就能触发。该处理器写着「用 edit 工具交互式修复每条剩余发现……不要重跑 Steps 1-6」，且没有 minimal 豁免，于是最多十五条未经任何验证、未跑构建/测试、未经反向审计的发现会被应用到用户的工作树上——违反了刚刚打印的「不编辑任何东西」保证，也正是 fix 门槛注释所点名的同一类错误（「凭未验证的发现编辑用户文件，与发帖是同一类错误」）。Step 3C 的 low 档在设计上不同：它明确提供 fix these issues 提示，因此该处理器在那里触发是预期行为；Step 3M 是刻意不提供。建议仿照下一行新增的子句补上：「在 --topology minimal 下按 Step 3M 拒绝——发现未经验证；指向 /review --fix，它会以 medium 重跑并产生已验证的发现。」
— qwen3.8-max via Qwen Code /review (v0.22.0)
…xtend the arm's no-write guards (#9919) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9919 (round 4)Four inline findings this round (all Findings and dispositions[rc:3847933691] [rc:3847933686] [rc:3847933701] [rc:3847933705] Notes
VerificationCommands actually run this round (repository root unless noted):
中文说明Autofix 本轮总结 — PR #9919（第 4 轮）本轮共 4 条行内发现（均为 发现与处置[rc:3847933691] [rc:3847933686] [rc:3847933701] [rc:3847933705] 说明
验证本轮实际执行的命令（除注明外在仓库根目录）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 3a": none — no check was cut short..
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/skills/bundled/review/SKILL.md:68 — [review] the comment verdict bullet's cause enumeration is stale — minimal is a second cause of requested && !effective on a PR targetpackages/core/src/skills/bundled/review/SKILL.md:609 — [review] Step 3M's tip rationale asserts the 'post comments' handler 'routes that ask toward Step 7' while the handler declines under --topology minimalpackages/core/src/skills/bundled/review/DESIGN.md:693 — [review] the minimal arm's 'one model turn' pricing does not reproduce — turns scale with the chunk plan
Convergence: round 4 posted 5 inline comment(s), 5 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/cli/src/commands/review/lib/authorization.ts (findings in round 3; 3 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 3a"：none — no check was cut short.。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 5 条行内评论，其中 5 条是首次提出；上一轮发布了 4 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/lib/authorization.ts（第 3 轮已出过发现，本轮又有 3 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const wrongRepo = authFor( | ||
| 'https://github.com/x/y/pull/123 --topology minimal --comment', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The sole-blocker ordering test pins three of the four binding axes the production comment in authorization.ts names ("a non-PR target, or another PR's number, repo, or host, still holds") — target shape, PR number, repo — but never pins the host axis, even though the minimal fall-through now crosses the host-binding check before the trailing topology refusal.
The gap is empirically demonstrated: a mutant relocating the host check after the trailing topology refusal survives all 118 tests in this file, while a minimal wrong-host record flips from the host-mismatch refusal to the topology wording — sending the operator to re-run without --topology minimal into the same still-unnamed host refusal. That is the exact misdirection this round's reorder exists to prevent.
| const wrongRepo = authFor( | |
| 'https://github.com/x/y/pull/123 --topology minimal --comment', | |
| ); | |
| const wrongHost = authFor( | |
| 'https://ghe.corp.example/o/r/pull/123 --topology minimal --comment', | |
| ); | |
| expect(wrongHost.ok).toBe(false); | |
| expect(wrongHost.why).toContain('authorise ghe.corp.example'); | |
| expect(wrongHost.why).toContain('targets github.com'); | |
| expect(wrongHost.why).not.toContain('`--topology minimal`'); | |
| const wrongRepo = authFor( | |
| 'https://github.com/x/y/pull/123 --topology minimal --comment', | |
| ); |
中文说明
唯一阻塞排序测试钉住了 authorization.ts 生产注释所列四个绑定轴（「非 PR 目标，或另一个 PR 的编号、仓库或 host 仍未绑定」）中的三个——目标形态、PR 编号、仓库——但从未钉住 host 轴，尽管 minimal 回落现在会先穿过 host 绑定检查，才到达末尾的 topology 拒绝。
该缺口已被实证：把 host 检查移到末尾 topology 拒绝之后的变异，在本文件全部 118 个测试保持通过的情况下存活；而一个错误 host 的 minimal 记录会从 host 不匹配拒绝翻转为 topology 文案——把操作者引向「去掉 --topology minimal 重跑」，然后撞上同一个仍未被点名的 host 拒绝。这正是本轮重排所要消除的误导。
（建议修复：补上钉住 host 轴的第四个用例，见上方代码块。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Minimal, and bound to this write on every axis above — the parser | ||
| // forced effective false, so the topology is now the sole blocker, and | ||
| // its remedy lifts the refusal. |
There was a problem hiding this comment.
[Suggestion] This comment states two invariants unconditionally — "the topology is now the sole blocker, and its remedy lifts the refusal" — that are false for the no-comment-source sub-case this same block serves. The final if (!verdict.comment.effective) fires for any minimal record bound on every axis, including one with no --comment flag and no review.comment setting — the neither case the new test explicitly routes through this block. For that record topology is a blocker but not the sole one, and the remedy does not lift the refusal: executed at HEAD, re-running record 123 answers "--comment was not in the review arguments". The new test's own comment is careful — topology is "STILL the blocker to name" there — exactly where this code comment overclaims.
A maintainer of this refusal-wording-critical gate trusts this comment six months later: they "simplify" the neither test branch away as contradicting the documented invariant, or diagnose an operator's "did exactly what the message said, refused again" report as a gate bug instead of the expected second blocker.
| // Minimal, and bound to this write on every axis above — the parser | |
| // forced effective false, so the topology is now the sole blocker, and | |
| // its remedy lifts the refusal. | |
| // Minimal, and bound to this write on every axis above. When a comment | |
| // source was recorded, the parser forced effective false, so the | |
| // topology is the sole blocker and its remedy lifts the refusal; when | |
| // none was, the topology is still the blocker to name — even a typed | |
| // --comment would not lift the refusal while minimal stands. |
中文说明
该注释无条件地断言了两个不变量——「topology 现在是唯一阻塞，且其补救能解除拒绝」——但对同一代码块所服务的「无 comment 来源」子情形并不成立。末尾的 if (!verdict.comment.effective) 对任何在各轴上都绑定的 minimal 记录都会触发，包括既没有 --comment 标志、也没有 review.comment 设置的记录——即新测试显式经过此块的 neither 用例。对该记录而言，topology 是一个阻塞而非唯一阻塞，且其补救无法解除拒绝：已在 HEAD 上执行验证，重跑记录 123 得到 "--comment was not in the review arguments"。新测试自己的注释很谨慎——那里写的是 topology 「仍是要点名的阻塞」——而这正是本代码注释过度断言之处。
六个月后，维护这个「拒绝措辞攸关」门控的维护者会信任这段注释：他们要么把 neither 测试分支当作与文档化不变量矛盾而「顺手简化」掉，要么把操作者「我完全照做了提示却再次被拒」的报告误诊为门控 bug，而不是预期中的第二个阻塞。
（建议修复：把注释限定为实际不变量，见上方代码块。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| If **none** of the three holds, `submit` refuses and nothing is written. You MUST NOT reach around it — no `gh api .../pulls/.../reviews`, no other comment/review write, at all in this run — regardless of the verdict, the number of Criticals, or any "Tip: post comments" text you are about to print. A Request-changes verdict with unposted Criticals is the correct, complete outcome of a review without an effective comment authorisation: the findings live in the terminal (Step 6) and the saved report (Step 8), and the follow-up tip invites the user to post if they want. Do not rationalize a post because the findings "seem important" — the user decides when feedback becomes public. This gate has been violated in dogfooding (measured; DESIGN.md — The self-filed COMMENT review (PR #6771)); the check is arithmetic, not judgment: no flag, no standing setting, and no explicit request ⇒ no write. | ||
|
|
||
| Also skip this step (independently of the gate above) if the review target is not a PR, or if the review ran at low or medium effort. **Low**'s findings are unverified and must never be posted. **Medium**'s findings ARE verified (Step 4 ran), but posting is a high-only action — `--comment` forces high, and medium's verdict is capped at Comment — so a medium review reports to the user and does not post to the PR. Decline a "post comments" follow-up after either, and point at `--effort high`. | ||
| Also skip this step (independently of the gate above) if the review target is not a PR, or if the review ran at low or medium effort, or if it ran under `--topology minimal` at any effort. **Low**'s findings are unverified and must never be posted. **Medium**'s findings ARE verified (Step 4 ran), but posting is a high-only action — `--comment` forces high, and medium's verdict is capped at Comment — so a medium review reports to the user and does not post to the PR. **Minimal**'s findings are unverified and the arm posts nothing at any effort — it resolves effort high by default on a PR target while the `--user-authorized` fast path never consults the topology, so this rule is the layer that catches a run whose Step 6 decline was missed. Decline a "post comments" follow-up after any of the three, and point at `--effort high`. |
There was a problem hiding this comment.
[Suggestion] This sentence asserts a load-bearing fact about the write gate — "the --user-authorized fast path never consults the topology" — while this same diff makes topology a write-gate concept on the slow path. Yet no test pins the fast-path × minimal interaction in either direction: all 38 userAuthorized: true call sites in submit.test.ts carry no minimal record. The documented slow/fast contrast is observable on the identical record — slow path ok:false ("…ran with --topology minimal…") vs fast path ok:true ("the user asked for this review to be published") — but exists only as prose today.
The cost is concrete: adding a topology refusal to the fast path, or a refactor that widens it, ships with a green suite and silently falsifies the layering this sentence documents — and mechanising the arm's no-post guarantee would go equally unnoticed. Either drift changes what the designated catch layer does without any test failing.
Pin the documented contract (in submit.test.ts):
it('the fast path honours the user ask even under minimal (documented layering)', () => {
const auth = authFor('123 --topology minimal --comment', {
userAuthorized: true,
});
expect(auth.ok).toBe(true);
});— or, if the arm's no-post guarantee is meant to be mechanical rather than prose, give the fast path the same topology refusal and let the test pin that instead.
中文说明
本句断言了写门控的一个承重事实——「--user-authorized 快速路径从不查询 topology」——而同一 diff 又使 topology 成为慢路径上的写门控概念。然而没有任何测试在任一方向上钉住「快速路径 × minimal」的交互：submit.test.ts 中全部 38 处 userAuthorized: true 调用点都不携带 minimal 记录。文档所述的慢/快路径对比在同一条记录上可观察——慢路径 ok:false（"…ran with --topology minimal…"）对快速路径 ok:true（"the user asked for this review to be published"）——但目前只存在于文案中。
代价是具体的：给快速路径加上 topology 拒绝、或一次将其放宽的重构，都会在整个测试套件保持绿色的情况下合入，并悄悄证伪本句所记录的层次设计——反过来，把该臂的「不发帖」保证机械化也同样无人察觉。任一方向的漂移都会改变指定兜底层的实际行为，而没有任何测试失败。
请在 submit.test.ts 中钉住该文档化契约（见上方代码块）；或者，如果该臂的不发帖保证应当是机械的而非文案的，就给快速路径同样的 topology 拒绝，并让测试钉住那个行为。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| '`--topology minimal`, which is terminal-only and cannot authorise ' + | ||
| 'posting — re-run the review without it', |
There was a problem hiding this comment.
[Suggestion] This new topology refusal is a third refusal class, but its only shape-sensitive consumer — the advice dispatch at submit.ts:746 (auth.why.includes('--comment was')) — still knows only two classes. The refusal string contains no such substring, so the dispatch selects the second arm, whose preamble "Nothing recorded authorises binding this target" is false for this class — the record DID name the target and bound every axis (that is why it reached the topology block at all) — and whose remedy "a review invoked naming it" re-refuses while the topology is kept: the futile-retry loop the dispatch's own comment block exists to prevent.
Aggravating: that arm's other remedy — "--user-authorized after the user has asked" — mechanically succeeds against the same record (fast path ok: true observed at HEAD), so the advice appended to this refusal actively points at the bypass of that very refusal.
Fix in submit.ts — a third dispatch arm keyed on the topology marker, checked before the existing branch, whose advice restates the refusal's own remedy (re-run without --topology minimal) and does not offer --user-authorized.
中文说明
这个新的 topology 拒绝是第三个拒绝类别，但它唯一的「按措辞分流」的消费者——submit.ts:746 的建议分发（auth.why.includes('--comment was')）——仍然只认识两个类别。该拒绝字符串不含该子串，因此分发落入第二个分支：其前言「没有任何记录授权绑定此目标」对本类别是错误的——记录确实指名了目标并在各轴上完成绑定（正因如此才走到 topology 块）——其补救「用指名该目标的评审重跑」在保留 topology 时会再次被拒：正是分发自己的注释块所要防止的「徒劳重试」循环。
更糟的是：该分支的另一个补救——「用户已要求后用 --user-authorized」——对同一条记录在机制上会成功（已在 HEAD 观察到快速路径返回 ok: true），因此附加在这条拒绝之后的建议，恰恰指向了绕过该拒绝本身的通路。
请在 submit.ts 中修复：新增以 topology 标记为键的第三个分发分支，置于既有分支之前检查，其建议复述拒绝自身的补救（去掉 --topology minimal 重跑），且不提供 --user-authorized。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!verdict.comment.effective) { | ||
| // Minimal, and bound to this write on every axis above — the parser |
There was a problem hiding this comment.
[Suggestion] The reorder makes the gate shape-sensitive — A/B probe verified: at the round-3 anchor cba3a2966c the same minimal record answered the topology refusal under every caller shape; at this commit it flips to the host/repo-binding refusals on the second caller's shape — yet no test exercises a minimal record through publish-assets' call shape (repo optional, host env-resolved including GH_HOST, absentHostFollowsRecording omitted so an absent host reads as a github.com claim): topology appears zero times in publish-assets.test.ts, whose only refusal-wording gate cases are non-minimal.
The gate's own header says "one function, two callers, one lesson-ledger", but this round's lesson is test-pinned for one caller only: a future refusal-ordering refactor — the exact edit class of this round; round 3's ordering was itself wrong and had to be reordered — regresses minimal handling on the publish-assets shape (wrong blocker named, or the evidence-image advice wrapper misapplied) while every suite stays green.
Mirror the new submit tests in publish-assets.test.ts, matching the existing run(...) / writeFileSync(argsFile, …) shape:
it('a fully-bound minimal record names the topology (evidence-images advice)', () => {
writeFileSync(argsFile, '8346 --topology minimal --comment\n');
run({ files: [pngFile('a.png')] });
expect(process.exitCode).toBe(3);
const why = (stderrSpy.mock.calls.map((c) => c[0]) as string[]).join(' ');
expect(why).toContain('`--topology minimal`');
});plus a wrong-host record without --reviewed-repo, expecting the host-binding refusal and not the topology wording.
中文说明
重排使门控变得「形状敏感」——A/B 探针已验证：在第 3 轮锚点 cba3a2966c 上，同一条 minimal 记录在所有调用者形状下都回答 topology 拒绝；而在本提交上，在第二调用者的形状下翻转为 host/仓库绑定拒绝——然而没有任何测试让 minimal 记录走一遍 publish-assets 的调用形状（repo 可选、host 经环境解析含 GH_HOST、省略 absentHostFollowsRecording 因而缺省 host 读作 github.com 主张）：publish-assets.test.ts 中 topology 出现零次，其仅有的拒绝措辞门控用例均为非 minimal。
门控自己的头部注释写着「一个函数，两个调用者，一本教训台账」，但本轮的教训只为其中一个调用者钉了测试：未来一次拒绝排序重构——正是本轮的编辑类别；第 3 轮的排序本身就是错的、不得不重排——会在 publish-assets 形状上使 minimal 处理回归（点名错误的阻塞，或误用证据图片建议包装），而所有套件保持绿色。
请在 publish-assets.test.ts 中仿照新增的 submit 测试，按既有 run(...) / writeFileSync(argsFile, …) 形态补上（见上方代码块），另加一个不带 --reviewed-repo 的错误 host 记录用例，断言得到 host 绑定拒绝而非 topology 文案。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…opology # Conflicts: # packages/core/src/skills/bundled/review/SKILL.md
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9919 (round 4 feedback)Conflict resolution
All other files merged cleanly. Feedback points and dispositionsAll five points are automated-reviewer Suggestions. All five were reproduced against the code before implementation and are resolved in code this round:
Recorded by the reviewer under its convergence posture ("recorded, not requested in this round"): the three deferred documentation findings on Failed check diagnosis —
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/parse-args.ts:888 — [probe] PR-target --fix warning pre-empts the minimal branch and advises --comment , itself disabled under minimalpackages/core/src/skills/bundled/review/SKILL.md:611 — [review] Step 3M's tip rationale claims the trigger-phrase handler routes 'post comments' toward Step 7 while the same PR adds a minimal decline to exactly that handlerpackages/core/src/skills/bundled/review/SKILL.md:613 — [probe] Step 3M's /review --fix pointer is wrong for PR targets — --fix is ignored and the medium effort floor never firespackages/core/src/skills/bundled/review/SKILL.md:1077 — [review] the new minimal pass disposition bullet contradicts the unchanged 'posted is a fact' paragraph below the disposition listpackages/core/src/skills/bundled/review/references/posting.md:7 — [review] the header says posting.md never loads under minimal while the same diff's skip rule claims the catch-layer role that requires it to load in exactly that runpackages/core/src/skills/bundled/review/references/posting.md:35 — [probe] the minimal decline's remedy --effort high is the wrong axis — effort never lifts the topology bar, contradicting the gate's own refusal remedy
[Critical] Step 3M inherits Step 6's report_findings instruction without the override its sibling Step 3C has: Step 6 mandates level = the run's resolved effort and entries copied from the findings artifact, while Step 3M forbids creating the artifact. REPORT_FINDINGS_LEVELS ('low' | 'medium' | 'high', report-findings.ts:65) has no value expressing minimal/unverified, and FindingsDisplay.tsx renders the only unverified marker solely for level === 'low' — so a minimal pass (effort resolving high on a PR target) either renders its unverified findings indistinguishably from a verified high-effort review in the TUI/Web Shell/ACP, or skips the call for lack of an artifact, defeating the labeled-unverified property this diff's parser force-offs, posting declines, and DESIGN.md's guardrail edit reserve for this arm. Fix: add a Step 3C-parallel bullet to Step 3M's adjustments fixing the level (or extend the enum plus FindingsDisplay) and stating entries come from the composed finding list since no artifact exists. (Placed in the body because its anchor line SKILL.md:609 already carries an earlier, different thread — comment 3846222717; the claim itself is new this round.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
[Critical] Step 3M inherits Step 6's report_findings instruction without the override its sibling Step 3C has: Step 6 mandates level = the run's resolved effort and entries copied from the findings artifact, while Step 3M forbids creating the artifact. REPORT_FINDINGS_LEVELS ('low' | 'medium' | 'high', report-findings.ts:65) has no value expressing minimal/unverified, and FindingsDisplay.tsx renders the only unverified marker solely for level === 'low' — so a minimal pass (effort resolving high on a PR target) either renders its unverified findings indistinguishably from a verified high-effort review in the TUI/Web Shell/ACP, or skips the call for lack of an artifact, defeating the labeled-unverified property this diff's parser force-offs, posting declines, and DESIGN.md's guardrail edit reserve for this arm. Fix: add a Step 3C-parallel bullet to Step 3M's adjustments fixing the level (or extend the enum plus FindingsDisplay) and stating entries come from the composed finding list since no artifact exists. (Placed in the body because its anchor line SKILL.md:609 already carries an earlier, different thread — comment 3846222717; the claim itself is new this round.)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| `needs a review invoked naming it, or --user-authorized after the ` + | ||
| `user has asked, in a message they typed, for this review to be ` + | ||
| `published.`; | ||
| const advice = auth.why.includes('`--topology minimal`') |
There was a problem hiding this comment.
[Suggestion] This new advice arm classifies refusals by substring-matching auth.why for `--topology minimal`, but the "missing --comment" and "do not name a pull request" refusals embed the operator's verbatim recorded args via JSON.stringify(raw.trim()). writeSkillArgs records the invocation byte-for-byte and tokenizeArgs strips only single/double quotes, so a markdown-backticked mention of the phrase in the invocation never parses as the flag yet still reaches why — steering a non-topology refusal into this arm. Probe at this commit: recording 6771 `--topology minimal` parses to topology: "auto" with no comment source; submit then refuses with "--comment was not in the review arguments" while the advice printed is this topology arm — claiming the run "ran under --topology minimal" when it did not, and prescribing a re-run without a flag that was never in effect while the real blocker stays unnamed: the futile-retry loop the comment block above this ternary exists to prevent (observed exit 3, gh never called — the write stays fail-closed; only the advice class is wrong). Reverting the ternary to the pre-PR two-arm shape flips the same record to the comment arm, so the misroute is introduced by this match. Fix by classifying on structure rather than on text that embeds operator input — e.g. have reviewWriteAuthorization return a cls: 'topology' | 'comment-not-requested' | 'unbound' discriminator and branch on it; a tighter marker string does not close the class, since any fixed fragment can itself be embedded in the quoted record.
中文说明
新增的这条建议分支通过子串匹配 auth.why 中的 `--topology minimal` 来分类拒绝，但「缺少 --comment」与「未指名 pull request」两类拒绝会通过 JSON.stringify(raw.trim()) 内嵌操作者逐字记录的参数。writeSkillArgs 逐字节记录调用，tokenizeArgs 只剥离单/双引号，因此调用中带 markdown 反引号的该短语永远不会被解析为 flag，却仍能进入 why——把非 topology 拒绝误导进这条分支。已在本 commit 上用探针验证：记录 6771 `--topology minimal` 解析为 topology: "auto" 且无 comment 来源；submit 随后以 "--comment was not in the review arguments" 拒绝，而打印的建议却是这条 topology 分支——声称该运行 "ran under --topology minimal"（事实并非如此），并要求去掉一个从未生效的 flag 重跑，真正的阻塞始终未被点名：正是本三元上方注释块声称要防止的徒劳重试循环（观察到 exit 3、gh 从未被调用——写入仍是失败即关闭的，只有建议分类错了）。把三元还原为 PR 前的双分支形态，同一条记录会翻转到 comment 分支，说明误导正是由这个匹配引入的。请改用结构化判别而非匹配内嵌操作者输入的文本——例如让 reviewWriteAuthorization 返回 cls: 'topology' | 'comment-not-requested' | 'unbound' 判别字段并按其分支；收紧标记字符串无法关闭这一类问题，因为任何固定片段都可能被嵌进被引用的记录里。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const advice = auth.why.includes('`--topology minimal`') | ||
| ? `This is the correct outcome of a review run under ` + | ||
| `\`--topology minimal\` — the arm posts nothing at any effort. ` + | ||
| `Report the findings in the terminal and stop. Re-run the review ` + | ||
| `without \`--topology minimal\` to make posting available.` |
There was a problem hiding this comment.
[Suggestion] This arm promises "Re-run the review without --topology minimal to make posting available", but that sufficiency holds only when a comment source was recorded. The canonical /review 123 --topology minimal invocation records none, so the trailing if (!verdict.comment.effective) arm fires for it regardless of comment.requested (this diff's own neither test pins the refusal firing with `--topology minimal` in why) — and the advised bare re-run /review 123 re-refuses with "--comment was not in the review arguments". The refusal's own remedy this arm claims to restate ("re-run the review without it") makes no sufficiency promise; the added "to make posting available" is false for this class, and the arm names none of the remedies that would actually authorise the post (--comment, the review.comment setting, or a user-typed ask). The advice test only exercises the comment-requested shape (6771 --topology minimal --comment), where the promise happens to hold, leaving this class untested — one wasted re-run cycle under the very "futile retry loop" standard this hunk's comment invokes.
| const advice = auth.why.includes('`--topology minimal`') | |
| ? `This is the correct outcome of a review run under ` + | |
| `\`--topology minimal\` — the arm posts nothing at any effort. ` + | |
| `Report the findings in the terminal and stop. Re-run the review ` + | |
| `without \`--topology minimal\` to make posting available.` | |
| const advice = auth.why.includes('`--topology minimal`') | |
| ? `This is the correct outcome of a review run under ` + | |
| `\`--topology minimal\` — the arm posts nothing at any effort. ` + | |
| `Report the findings in the terminal and stop. Re-run the review ` + | |
| `without \`--topology minimal\` — with posting requested (\`--comment\` or the \`review.comment\` setting) — to make posting available.` |
中文说明
这条分支承诺「去掉 --topology minimal 重跑即可发帖」，但该充分性只在记录了 comment 来源时成立。典型调用 /review 123 --topology minimal 不记录任何 comment 来源，末尾的 if (!verdict.comment.effective) 分支对其无条件触发（diff 自己的 neither 测试钉住了该拒绝会携带 `--topology minimal`）——照建议裸重跑 /review 123 会再次以 "--comment was not in the review arguments" 被拒。拒绝自身的补救（"re-run the review without it"）并无充分性承诺；新增的 "to make posting available" 对该子情形为假，且该分支没有点名任何真正能授权发帖的补救（--comment、review.comment 设置或用户当场要求）。建议测试只覆盖了带 comment 的形态（6771 --topology minimal --comment，承诺恰好成立），该子情形未被测试——在本 hunk 注释自己引用的「徒劳重试」标准下白白消耗一次重跑。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…imal findings (#9919) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9919Actionable feedback this round: one Critical (review body rv:5015633539) and two automated-reviewer Suggestions (rc:3850155076, rc:3850155079). All three were verified against the exact reviewed commit, reproduced with failing tests before any fix, and resolved in one commit ( Findings and decisions
Witnesses and mutation probesEvery guard this round's commit adds has its own witness; each was probed by removal before committing:
Changed files: VerificationCommands actually run and their results:
中文说明轮次总结 — PR #9919本轮可执行反馈：一条 Critical（评审正文 rv:5015633539）和两条自动评审器的 Suggestion（rc:3850155076、rc:3850155079）。三条均已在被评审的确切 commit 上核实、在修复前以失败测试复现，并在一个 commit（ 发现与决定
见证与变异探针本轮 commit 新增的每个守卫都有自己的见证测试；提交前均通过「移除探针」验证：
变更文件： 验证实际执行的命令及结果：
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /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: 70 passed · 0 failed · 70 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：70 通过 · 0 失败 · 70 总计 抖动门：✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9919 — feat(review): add a --topology minimal single-pass A/B armVerdict: 中文摘要
Central claim + A/BCentral claim: Environment per cell: head = CI-built
The flip cells are the load-bearing proof: without this PR, a recorded minimal run that names its PR and requests posting authorises the public write at the gate (base Mutation matrix (
No survivors; every kill landed in the suite that covers the mutated file. M2/M2b together attribute the two round-2 test families precisely: the minimal-advice tests pin the arm's existence, the backtick test pins the structural predicate. FindingsS1 (Suggestion, non-blocking) — Measured minimal fix (one line, preserves intent)In Not covered
MethodologyEnvironment: CI merge-ref checkout at Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated 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. ✅
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approved. Verified at head 11cf00c that all three earlier-round Criticals are closed: (1) the write gate in authorization.ts now owns a shape-aware topology refusal that falls through to the binding checks when minimal is not the sole blocker, with a dedicated advice arm; (2) Step 9 carries the dedicated 'minimal pass, not posted ( unverified findings)' disposition that Step 3M references; (3) Step 3M's Step-8 routing now also forbids the structured artifact (no composed verdict to persist), with report_findings pinned at level low. Bot re-approved after the final commit; remaining open threads are Suggestion-level.
|
Released in v0.22.2. |
4 participants
What this PR does
Adds an explicitly selectable minimal review topology to the
/reviewskill. When chosen, the review runs as a single careful senior-engineer pass over the diff in the orchestrator's own context instead of the multi-agent pipeline: it reports at most fifteen findings, each required to carry a concrete failure scenario, and skips the subagent fan-out, verification, reverse audit, and build/test machinery, reporting to the terminal without posting or editing. The selection is parsed deterministically and carried in the argument verdict alongside the target and effort; the skill prompt gains the new pass and a dispatch override, and the design doc prices the new arm in its LLM-call-budget section.Why it's needed
Issue #9783 notes, from a side-by-side study of a competitor's reviewer, that the marginal value of the multi-agent scaffolding shrinks — and can turn negative — as the underlying model gets stronger. The proposal's first step is an A/B baseline: run the same PR set through the full
--effort highpipeline and through a minimal prompt, then compare recall and precision per model. That decision datum does not exist today, and the experiment cannot start without a minimal arm to compare against. This PR adds exactly that arm. It is deliberately only the selectable arm: no model-based auto-routing is introduced, because the routing decision is gated on the A/B result this arm makes possible.Reviewer Test Plan
How to verify
The default path is unchanged — confirm that a review without the new flag behaves exactly as before, then confirm the flag selects the minimal arm and that the minimal arm is terminal-only.
topologyresolves toauto(the standing pipeline); with--topology minimal,topologyresolves tominimal,topologySourceisexplicit, and bothcomment.effectiveandfix.effectiveare forced tofalseeven if--comment/--fixare also passed (the minimal arm posts and edits nothing):printf '6711\n' | node dist/cli.js review parse-args --stdin→"topology": "auto","topologySource": "default"printf '6711 --topology minimal --comment\n' | node dist/cli.js review parse-args --stdin→"topology": "minimal","topologySource": "explicit","comment": { "requested": true, "effective": false }and a warning that--commentis ignored because the arm is terminal-only.printf -- '--topology 6711\n' | node dist/cli.js review parse-args --stdinrescues6711as the PR target and warns, rather than falling back to the local diff.cd packages/cli && npx vitest run src/commands/review/parse-args.test.ts(133 tests, includes 13 new topology tests plus an end-to-end stdin→yargs→handler wiring test) andcd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts(39 tests; the prompt-pin suite confirms no pinned prose was broken by the new Step 3M).qwen review <pr> --topology minimalshould read the diff once, produce ≤15 findings each with a failure scenario, print a terminal report labelled as the minimal pass, and post nothing.Evidence (Before & After)
N/A — non-user-visible change (argument parsing + reviewer prompt); no TUI surface.
Tested on
Environment (optional)
Unit tests plus the local CLI (
node dist/cli.js review parse-args --stdin) on macOS.Risk & Scope
autopath is unchanged./review <pr> --topology minimalcomparison run is included — that A/B experiment is the follow-up this arm enables. Model-family auto-routing is intentionally out of scope until the A/B data exists.--topologyis a new opt-in flag; omitting it leaves every existing invocation byte-identical in behavior.Linked Issues
Part of #9783 — implements the minimal-prompt arm that step 1 (the A/B baseline) compares against. The A/B run itself and the conditional model×effort routing table (steps 2–3) follow once the baseline data exists, so this does not auto-close the issue.
中文说明
这个 PR 做了什么
为
/review技能新增了一个可显式选择的 极简（minimal） 评审拓扑。选中后，评审不再走多 agent 管线，而是在 orchestrator 自己的上下文里对 diff 做一次认真的资深工程师式通读：最多报告 15 条 findings，每条都必须带一个具体的失败场景（failure scenario），并跳过 subagent 扇出、验证、反向审计和 build/test 机制，只在终端输出、不发帖也不改代码。该选择由参数解析确定性地判定，并与 target、effort 一起放在裁决（verdict）里；技能提示词新增了这条通路和一个分发覆写，设计文档也在「LLM 调用预算」一节为这个新臂定了价。为什么需要
Issue #9783 指出，通过与某竞品评审器的对照研究可以发现：随着底层模型能力变强，多 agent 脚手架的边际价值会下降，甚至转负。该提案的第一步是跑一次 A/B 基线：同一批 PR，分别用
--effort high全量管线和极简提示词跑，再按模型对比 recall/precision。这个决策依据目前并不存在，而没有一个可对比的极简臂，实验就无法开始。本 PR 正是补上这个臂。它刻意只做「可选臂」这一件事：不引入基于模型的自动路由，因为路由决策要以本臂所支撑的 A/B 结果为前提。评审者测试计划
如何验证
默认路径不变——先确认不带新 flag 时行为与之前完全一致，再确认该 flag 能选中极简臂、且极简臂是「仅终端」的。
topology解析为auto（现有管线）；带--topology minimal时topology解析为minimal、topologySource为explicit，并且即使同时传了--comment/--fix，comment.effective和fix.effective都会被强制为false（极简臂不发帖也不改代码）：printf '6711\n' | node dist/cli.js review parse-args --stdin→"topology": "auto"、"topologySource": "default"printf '6711 --topology minimal --comment\n' | node dist/cli.js review parse-args --stdin→"topology": "minimal"、"topologySource": "explicit"、"comment": { "requested": true, "effective": false }，并附带一条「--comment被忽略，因为该臂仅终端输出」的警告。printf -- '--topology 6711\n' | node dist/cli.js review parse-args --stdin会把6711救援为 PR 目标并给出警告，而不是退回本地 diff。cd packages/cli && npx vitest run src/commands/review/parse-args.test.ts（133 个，含 13 个新 topology 测试和一个端到端 stdin→yargs→handler 接线测试）以及cd packages/core && npx vitest run src/skills/bundled/review/SKILL.test.ts（39 个；提示词 pin 套件确认新增 Step 3M 没有破坏任何被 pin 的文案）。qwen review <pr> --topology minimal应当通读一遍 diff，产出 ≤15 条、每条带失败场景的 findings，打印标注为极简通路的终端报告，且不发帖。证据（前后对比）
N/A——非用户可见改动（参数解析 + 评审提示词），无 TUI 界面。
测试环境
运行环境（可选）
单元测试 + 本地 CLI（macOS 上的
node dist/cli.js review parse-args --stdin）。风险与范围
auto路径保持不变。/review <pr> --topology minimal对比运行——那次 A/B 实验正是本臂所支撑的后续工作。基于模型族的自动路由刻意不在本 PR 范围内，要等 A/B 数据出来。--topology是新的可选 flag；不传时所有现有调用的行为逐字节不变。关联 Issue
属于 #9783 的一部分——实现了第 1 步（A/B 基线）所要对比的极简提示词臂。A/B 运行本身以及有条件的「模型 × effort」路由表（第 2、3 步）会在基线数据出来后跟进，因此本 PR 不会自动关闭该 issue。