feat(review): say when the approach, not the patch, is the open question - #9340
Conversation
Every finding /review emits is anchored to a `file:line` in the current diff. That is what a finding is — and it means a review can report where an approach leaks, but never that a different approach would retire all of the leaks at once. Measured: one change to `extractAndStripMeta` took three attempts across two PRs. QwenLM#9097 (3 rounds, 18 findings) added a timeout to the vm call; QwenLM#9136 (6 rounds, 56 findings) moved the walk inside the vm and ended up spawning a child process per call, growing 228 -> 920 source diff lines. QwenLM#9325 landed it in one commit by not evaluating the literal at all. All 74 findings were individually correct, and every one of them went away with the mechanism. The signal was already there and filed as the wrong kind of thing: `did not converge within the reverse-audit round cap` appeared four times across the two PRs, as a coverage gap — "we did not finish looking" — rather than as a conclusion about the change. Nothing was responsible for reading it as "stop patching". Add one advisory paragraph, and one clause on the terminal verdict line, when a non-Approve round is past the round threshold AND its source diff has grown at least 3x since the review first measured it. This round's round-cap stop rides along as corroborating text when present; it is never a trigger on its own. It is deliberately not a finding. Findings are what the autofix loop consumes, and that loop patching each finding in turn is the pattern being interrupted — a finding here would be fixed rather than read. It addresses the human deciding what happens next, so it is a body paragraph and a verdict-line clause, it adds no cap, and it never moves the event. The baseline is a baseline, not the previous round's size: 228 -> 920 across six rounds is ~1.3x per round, which no per-round delta would notice, but 4.0x cumulatively. `Ledger.src0` records the first measurement and is carried forward unchanged, so a diff that later shrinks cannot rewrite its own baseline. It is the one marker field that survives truncation — the ruling that withholds an anchor from a partial finding list does not extend to a measurement of the diff. Known limits, documented rather than papered over: it cannot see across pull requests, so the three-attempt shape that motivated it would have fired only on a second forgeable persisted counter; and it is retroactively blank, staying silent until a PR has posted two rounds after this ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Re-gate on the maintainer's Template looks good ✓ — all sections present, complete bilingual translation. Problem: observed, not theoretical. The motivating incident is three real attempts at the same change in this repo (#9097, #9136, #9325 — verified by number on the first pass), and the structural gap it names — findings anchored to Direction: aligned. It improves the bundled Size: 344 production / 765 test / 5 schema / 121 docs lines (additions + deletions). Core paths touched remain minor: the Approach: unchanged since the last pass and still deliberate — a baseline carried forward rather than re-measured, growth measured over the full merge-base range on incremental rounds, and the disclosure pinned byte-identical to a no-signal run by its own tests. One standing follow-up from earlier passes (the user-facing settings table row) remains open; it gates nothing, so it is named in the final comment rather than here. Risk: no elevated risk signals — no files match the high-risk revert-correlated paths. Moving on to code review. 🔍 中文说明应维护者的 模板完整 ✓ —— 各节齐全，中文翻译完整。 问题：真实观测，而非理论假设。动机事件是本仓库对同一改动的三次真实尝试（#9097、#9136、#9325，首轮已按编号核实），它指出的结构性盲区——锚定在 方向：对齐。它改进了内置 规模：生产 344 行 / 测试 765 行 / schema 5 行 / 文档 121 行（增+删）。触及的核心路径仍然很小： 方案：与上次审查相比未变，仍是深思熟虑——基线向前携带而非重新测量、增量轮按完整 merge-base 范围测量增长、披露文本由自身测试钉住与无信号运行逐字节一致。早前各轮遗留的一个后续跟进项（面向用户的设置表缺行）仍未解决；因其不构成任何门禁，放在最终评论中点名而非此处。 风险：无升级风险信号——没有文件匹配与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThird full pass. Nine of the sixteen files are byte-identical to the previously reviewed head; the rest changed only by merging an evolved Re-verified against the current base code, not just the diff:
No correctness blockers, no security holes, no convention violations found. The blast radius of the whole mechanism is one advisory paragraph: The one open follow-up is unchanged from earlier passes: The flow, for navigation (unchanged from the last pass, re-checked against the current diff): sequenceDiagram
participant P1 as fetch-pr
participant P2 as compose-review round N
participant P3 as posted review body
participant P4 as pr-context round N+1
participant P5 as prev-ledger side file
participant P6 as compose-review round N+1
P1->>P2: plan with full-range source-diff measurement
P2->>P3: ledger marker with src0 baseline
P4->>P3: walk own reviews, parseLedger keeps src0
P4->>P5: persist recovered ledger, src0 rides along
P6->>P5: read round and src0
P6->>P6: fire when rounds and growth both hold
Files changed (16 of 16 shown)
TestingThis is an unattended run ( The behavioural surface is deterministic text composition over machine-local files, and the suite pins it directly: the byte-identical event/baseEvent/caps case proves the disclosure-only contract (a build with the signal removed fails its own tests), the full-range supply has its own fixture, and every firing and silence arm — no baseline, early round, no growth, below floor, legacy incremental, downgraded Approve — is pinned. 2b settled the central claim, so no sandboxed-lane trigger; separately, maintainer @wenshao deep-verified this exact head locally (17/17 scripted assertions, reported in-thread) and approved at CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查第三次完整审查。16 个文件中 9 个与上次审查的 head 逐字节一致；其余文件的变化全部来自合并演进中的 对照当前基线代码逐项核实，而非只看 diff：
未发现正确性阻断、安全漏洞或规范违规。整个机制的影响半径是一段建议性文字： 唯一未关闭的后续跟进项与早前各轮一致： 测试本次为无人值守运行（ 行为面是机器本地文件上的确定性文本组合，测试套件直接钉住了它：事件/baseEvent/caps 逐字节一致的用例证明了"仅披露"契约（移除信号的构建会在自身测试上失败），全范围供给有专门夹具，每一条触发与沉默分支——无基线、早期轮次、无增长、低于下限、旧版增量计划、被降级的 Approve——均有测试钉住。2b 已证实核心主张，故无沙箱验证触发条件；另外，维护者 @wenshao 已在本地对此 head 做了深度验证（17/17 断言通过，已在帖内报告）并在 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean re-review at the merged-up-to-date head with a fully green CI; the single reservation is the one standing doc-table follow-up, which does not block. Stepping back: the interesting thing about this pass is what DIDN'T happen. Since the last review the branch absorbed ~199 commits of On the substance my view is unchanged from the last two passes, and the motivation got stronger: the earlier reservation that #9325 was described as "landed" while still open is gone — it merged on 2026-08-22, so the incident narrative is now fully on record. What remains is the other reservation: My independent proposal before reading the diff is what this PR implements, and where it goes beyond the sketch it earns it: the truncation ruling (anchor shed, measurement kept), the fail-open recovery direction, and the retroactively-blank property that keeps every in-flight PR silent instead of false. The blast radius of a false positive is one advisory paragraph, and an operator can push the threshold out or silence it. In six months this reads as a small, documented mechanism with an off switch — not something to curse. CI is fully green on the reviewed commit and nothing is pending, so no approve-on-green deferral. Approving now, pinned to the reviewed commit; this is the bot's vote beside the maintainer's existing approval at this head. ✅ 中文说明回顾全局：这一轮值得注意的是没有发生什么。自上次审查以来，分支吸收了约 199 个 实质判断与前两轮一致，且动机更强了：早前"设计文档称 #9325 已落地但它仍开放"的保留意见已消失——#9325 于 2026-08-22 合并，事件叙述现在完全有据可查。剩下的是另一条保留意见： 我在读 diff 之前的独立草案正是本 PR 所实现的；超出草案的部分都物有所值：截断裁决（丢锚、保测量）、恢复 fail open 的方向、以及让所有在途 PR 保持沉默而非误报的"追溯为空"属性。误报的影响半径是一段建议性文字，运维方可调高或静默阈值。六个月后回看，这是一个小而有界、带文档、带开关的机制——不会让人后悔。 CI 在被审 commit 上已全绿且无 pending，因此无需等绿批准。现在批准，钉死在被审 commit 上；这是机器人在维护者已有批准之外的第二票。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-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.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20210, 494, 1471, 1575, 3686, 507 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20210, 494, 1471, 1575, 3686, 507 passed。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // by a build predating the approach signal carries no field at all, and a | ||
| // mid-upgrade load must not fail over one that only affects display. A | ||
| // PRESENT value of the wrong shape is refused like every other field here. | ||
| const approachRaw = verdict['approachSignal'] ?? null; |
There was a problem hiding this comment.
[Suggestion] R1-1: The new approachSignal branches in validateVerdict — the refusal arms for a present-but-malformed value and the absent-means-null upgrade path (verdict['approachSignal'] ?? null) — have zero tests, while the sibling deferredCount block directly below has both shapes ("refuses a present deferredCount of the wrong shape" at save-artifact.test.ts:296, "reads an absent or null deferredCount as zero" at :315). — Failure scenario: delete any one refusal arm and all 27 save-artifact tests stay green (the copy test only proves a VALID object passes through), so a composed.json carrying approachSignal: { round: -1 } or growth: NaN would be persisted into the durable schema-v1 artifact instead of refused; separately, replacing ?? null with the strict access pattern the neighbouring lowSignal block uses would make every pre-feature composed file fail to save, uncaught. — Witness: mutant deleting the refusal loop → 26 passed | 1 skipped (all green); mirrored tests (7 wrong shapes + absent + null) 9/9 pass at HEAD, 4 failed | 5 passed against the mutant. Suggested fix: mirror the deferredCount pattern — an it.each of wrong shapes asserting toThrow(/approachSignal/), plus one case with the key deleted and one with approachSignal: null, asserting the artifact persists with verdict.approachSignal === null.
中文说明
validateVerdict 中新增的 approachSignal 分支——对"存在但形态错误"值的各拒绝分支，以及"缺失即 null"的升级兼容路径（verdict['approachSignal'] ?? null）——没有任何测试；而紧邻其下的同类字段 deferredCount 两种形态都有测试（save-artifact.test.ts:296 拒绝错误形态、:315 缺失/null 读作零）。— 失败场景：删除任意一个拒绝分支，全部 27 个 save-artifact 测试仍然全绿（copy 测试只证明合法对象能透传），于是携带 approachSignal: { round: -1 } 或 growth: NaN 的 composed.json 会被原样持久化进 schema-v1 产物而不是被拒绝；另外，若把 ?? null 换成相邻 lowSignal 块使用的严格访问模式，所有旧版本产物都会在保存时抛错且无测试拦截。— 证据：删除拒绝循环的变异体 → 26 通过 | 1 跳过（全绿）；镜像补测（7 种错误形态 + 缺失 + null）在 HEAD 上 9/9 通过，对变异体 4 失败 | 5 通过。建议修复：仿照 deferredCount 的模式——用 it.each 断言 toThrow(/approachSignal/)，再补"键被删除"与 approachSignal: null 两个用例，断言产物以 verdict.approachSignal === null 持久化。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const approachRaw = verdict['approachSignal'] ?? null; | ||
| if (approachRaw !== null) { | ||
| const signal = object(approachRaw, 'Composed verdict.approachSignal'); | ||
| for (const key of ['round', 'src0', 'srcDiffLines'] as const) { |
There was a problem hiding this comment.
[Suggestion] R1-2: This loop admits 0 for round, src0, and srcDiffLines (it checks < 0), but every producer invariant guarantees strictly positive values — the signal composes only when prevSrc0 > 0, src > LOW_SIGNAL_SRC_DIFF_LINES (100) and round = prevRound + 1 ≥ 1 (compose-review.ts:1950-1978) — and lib/ledger.ts validates the same-named src0 as strictly positive on both read and write. The artifact gate is looser than the semantics of the data it certifies. — Failure scenario: a hand-composed composed.json {"round": 0, "src0": 0, "srcDiffLines": 0, "growth": 4, "nonConverged": false} passes every check and is persisted as an internally impossible record ("grew 4x from a zero baseline, at round 0") that the verdict-line renderer would quote verbatim. — Witness: probe saving that exact object → saveReviewArtifact did not throw; the artifact persisted the all-zero signal verbatim. Suggested fix: reject <= 0 here ("must be a positive integer."), matching lib/ledger.ts's src0 rule; or, if zero is deliberately admissible in the artifact, say so in the comment so the asymmetry stops looking accidental.
中文说明
该循环对 round、src0、srcDiffLines 允许 0（只检查 < 0），但所有生产者不变量都保证严格为正——信号仅在 prevSrc0 > 0、src > LOW_SIGNAL_SRC_DIFF_LINES（100）且 round = prevRound + 1 ≥ 1 时才会生成（compose-review.ts:1950-1978）——而且 lib/ledger.ts 对同名字段 src0 在读写两侧都按严格正整数校验。产物校验门比它所认证数据的语义更宽松。— 失败场景：手工构造的 composed.json {"round": 0, "src0": 0, "srcDiffLines": 0, "growth": 4, "nonConverged": false} 能通过全部检查并被持久化为一条内部不可能的记录（"从 0 基线增长 4 倍、在第 0 轮"），判定行渲染器还会原样引用它。— 证据：以该对象调用保存 → 未抛错，全零信号被原样持久化。建议修复：此处改为拒绝 <= 0（"must be a positive integer."），与 lib/ledger.ts 的 src0 规则一致；若确实有意允许零，请在注释中说明，让这一不对称不再像无意疏漏。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // entry keeps its existing coverage cap untouched, and this flag is | ||
| // never a trigger on its own — only a clause appended when the signal | ||
| // has already fired on rounds and growth. | ||
| roundCapStopped = isRoundCap; |
There was a problem hiding this comment.
[Suggestion] R1-3: The corroborating-clause half of the signal — the roundCapStopped wiring here and both en/zh nonConverged branches in the rendered paragraph — has no test: every firing test uses a plan with no stop file, so nonConverged is always false (the save-artifact fixture's nonConverged: true exercises only the artifact validator, never this rendering). — Failure scenario: delete roundCapStopped = isRoundCap; or the (approachSignal.nonConverged ? ... : '') branches in both language blocks and all 360 tests in the touched files stay green — the clause silently vanishes from exactly the incident shape that motivated this feature (round-cap stop + ballooned diff). — Witness: mutant removing the wiring → 280 passed; the case suggested below fails under the mutant (expected false to be true) and passes at HEAD. Suggested fix (writeRoundCapStop is already imported in the test file):
const planPath = ballooned();
writeRoundCapStop(planPath, 5, 6);
// assert r.approachSignal?.nonConverged === true and
// r.body contains 'the reverse audit also stopped at its round cap without converging'中文说明
信号的佐证子句这一半——此处 roundCapStopped 的接线，以及渲染段落中 en/zh 两处 nonConverged 分支——没有测试：所有触发用例的 plan 都没有 stop 文件，因此 nonConverged 恒为 false（save-artifact fixture 里的 nonConverged: true 只过产物校验器，到不了这里的渲染）。— 失败场景：删除 roundCapStopped = isRoundCap; 或两种语言块中的 (approachSignal.nonConverged ? ... : '') 分支，改动文件的 360 个测试全部保持绿——该子句会从恰恰催生本功能的形态（轮数上限停止 + diff 膨胀）中悄悄消失。— 证据：删除接线的变异体 → 280 通过；下面的用例在变异体下失败（expected false to be true）、在 HEAD 上通过。建议修复（writeRoundCapStop 已被测试文件引入）：ballooned() 后 writeRoundCapStop(planPath, 5, 6)，断言 nonConverged === true 且正文包含相应英文子句。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| src0: | ||
| Number.isInteger(prev.src0) && (prev.src0 as number) > 0 | ||
| ? (prev.src0 as number) | ||
| : 0, |
There was a problem hiding this comment.
[Suggestion] R1-4: The one link of the src0 chain this read depends on — src0 surviving pr-context's persistRecoveredLedger into the side file — is pinned by no test on either side: src0 appears nowhere in any pr-context test, and the new compose-review tests write the side file directly, bypassing that writer. The field flows today only via spread-based writers (verified), so this is a coverage gap, not a live bug. — Failure scenario: a future edit to persistRecoveredLedger that swaps its whole-object spreads for explicit field selection — the pattern its own anonymous-recovery branch already uses — drops src0 from the side file; prevLedgerFor then reads src0: 0 every round, the approach signal never fires again in production, and every existing test stays green — the feature dies silently, failing toward silence. — Witness: grep sweep — src0 matches in pr-context tests: 0; this is the only unpinned joint in the marker→parse→persist→read chain. Suggested fix: in pr-context-persist.test.ts, recover a ledger whose marker carries src0: 228, run persistRecoveredLedger, and assert the written side file still contains "src0": 228 — ideally also across the anonymous-over-existing branch.
中文说明
此处读取所依赖的 src0 链条中，有一环没有任何一侧的测试约束——src0 能否经由 pr-context 的 persistRecoveredLedger 存活进 side file：所有 pr-context 测试中都搜不到 src0，而新增的 compose-review 测试是直接写 side file 的，绕过了那个写入器。该字段目前只靠基于展开（spread）的写入方式传递（已核实），因此这是覆盖缺口，不是现存缺陷。— 失败场景：未来若把 persistRecoveredLedger 的整体展开改为显式字段挑选——它自己的匿名恢复分支已经在用这种模式——src0 就会从 side file 中掉出；prevLedgerFor 此后每轮都读到 src0: 0，approach 信号在生产上永远不再触发，而所有现有测试依旧全绿——功能静默失效。— 证据：grep 扫描——pr-context 测试中 src0 匹配数为 0；这是 marker→parse→persist→read 链条上唯一没有测试钉住的接点。建议修复：在 pr-context-persist.test.ts 中恢复一个 marker 携带 src0: 228 的 ledger，运行 persistRecoveredLedger，断言写出的 side file 仍含 "src0": 228——最好连"匿名覆盖既有文件"分支也覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (event !== 'APPROVE' && prevSrc0 > 0 && input.planPath) { | ||
| const round = prevRound + 1; | ||
| const rounds = | ||
| operatorReviewSettings().approachRounds ?? APPROACH_ROUNDS_DEFAULT; |
There was a problem hiding this comment.
[Suggestion] R1-5: The fires-at-exactly-the-threshold boundary is untested: every firing case uses round 6 against threshold 5, so an off-by-one mutant on round >= rounds stays green. — Failure scenario: mutating >= to > leaves the whole suite green; a PR whose prev ledger records round 4 (this round = 5 = threshold) should fire per the design doc ("at or past the threshold") and the schema's "must reach", but under the mutant the signal silently waits an extra round with no test noticing. — Witness: mutant >= → > → 280 passed (whole suite green); the boundary case below passes at HEAD and fails under the mutant. Suggested fix: one case with prevLedger(planPath, { v: 1, round: 4, findings: [], src0: 228 }) and srcDiffLines: 920, asserting approachSignal is non-null at round 5 === the default threshold.
中文说明
"恰好在阈值处触发"的边界没有测试：所有触发用例都用第 6 轮对阈值 5，因此对 round >= rounds 的差一变异（>= → >）仍然全绿。— 失败场景：把 >= 变异为 > 后整套测试全绿；而 prev ledger 记录第 4 轮的 PR（本轮 = 5 = 阈值）按设计文档（"达到或超过阈值"）与 schema 的 "must reach" 应当触发，但在变异体下信号会无声地多等一轮且无测试察觉。— 证据：变异体 >= → > → 280 通过（整套全绿）；下述边界用例在 HEAD 上通过、在变异体下失败。建议修复：补一个 prevLedger(planPath, { v: 1, round: 4, findings: [], src0: 228 }) 且 srcDiffLines: 920 的用例，断言第 5 轮（=== 默认阈值）时 approachSignal 非空。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| `of the change is still right. Advisory only: this does not affect the verdict, ` + | ||
| `and nothing here is a blocker.`, | ||
| zh: | ||
| `⚠️ 第 ${approachSignal.round} 轮，且自本审查首次测量以来 diff 已增长 ` + |
There was a problem hiding this comment.
[Suggestion] R1-6: The Chinese half of this paragraph has no test — no han: true case fires the signal, although the module's convention tests bilingual renderings of other clauses that way (lines 1098, 3633, 5871 of the test file). — Failure scenario: break or truncate the zh string (e.g. drop the 仅供参考 advisory-only sentence) and all tests stay green; a PR whose description carries Han characters renders the broken paragraph in the posted body and nothing ever saw it. — Witness: mutant deleting the zh advisory sentence → 280 passed; a ballooned({ han: true }) case asserting the body contains ⚠️ 第 6 轮 and 仅供参考 passes at HEAD and fails under the mutant. Suggested fix: add that one case (coveredPlan already threads han into prDescriptionHasHan).
中文说明
本段落的中文半边没有测试——没有任何 han: true 用例触发该信号，尽管本模块对其他子句的双语渲染正是这样测的（测试文件第 1098、3633、5871 行）。— 失败场景：破坏或截断中文字符串（例如删掉"仅供参考"那句），所有测试依旧全绿；而描述含中文的 PR 会在发布正文中渲染出损坏的段落，且从未有任何测试看到。— 证据：删除中文提示句的变异体 → 280 通过；ballooned({ han: true }) 用例断言正文包含 ⚠️ 第 6 轮 与 仅供参考，在 HEAD 上通过、在变异体下失败。建议修复：补上这一个用例（coveredPlan 已把 han 传入 prDescriptionHasHan）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| srcDiffLines: 920, | ||
| ...over, | ||
| }); | ||
| prevLedger(planPath, { v: 1, round: 5, findings: [], src0: 228 }); |
There was a problem hiding this comment.
[Suggestion] R1-7: Neither comparison of the growth arm has a boundary test: every firing case uses 920/228 ≈ 4.04x against APPROACH_GROWTH_FACTOR (3) and src 920 against the 100-line floor, so mutants on growth >= APPROACH_GROWTH_FACTOR (compose-review.ts:1970) and src > LOW_SIGNAL_SRC_DIFF_LINES (:1969) stay green — the growth-arm twin of the round-threshold boundary gap. — Failure scenario: mutating >= to > leaves the suite green; a PR growing from baseline 100 to exactly 300 source lines — exactly the documented "grown by at least 3x" — never receives the advisory, and because the baseline ratchets (src0 carries forward unchanged; a shrink cannot raise it), a PR that later shrinks can never fire at all. Symmetrically, > → >= on the floor fires at exactly 100 lines, contrary to "past the floor". — Witness: mutant >= → > on the factor → suite green while a probe at (src0: 100, srcDiffLines: 300) flips to approachSignal=null (baseline: {growth: 3}); mutant > → >= on the floor → suite green while a probe at srcDiffLines: 100 flips to firing (baseline: null). Suggested fix: a fires-at-exactly-3x case (prevLedger src0: 100, plan srcDiffLines: 300, round 6 → expect signal with growth: 3) and a floor-boundary case (srcDiffLines: 100 → expect null).
中文说明
增长条件的两个比较都没有边界测试：所有触发用例都用 920/228 ≈ 4.04 倍对 APPROACH_GROWTH_FACTOR（3）、src 920 对 100 行下限，因此对 growth >= APPROACH_GROWTH_FACTOR（compose-review.ts:1970）和 src > LOW_SIGNAL_SRC_DIFF_LINES（:1969）的变异仍全绿——这是轮次阈值边界缺口在增长条件上的孪生版。— 失败场景：把 >= 变异为 > 后套件全绿；一个从基线 100 恰好增长到 300 行源码的 PR——正是文档所说的"至少增长 3 倍"——永远收不到提示，而且由于基线只进不退（src0 原样传递，缩小不会抬升它），之后缩小的 PR 永远无法触发。对称地，把下限的 > 变异为 >= 会在恰好 100 行时触发，与"超过下限"的表述相悖。— 证据：对倍数做 >= → > 变异 → 套件全绿，而（src0: 100、srcDiffLines: 300）探针翻转为 approachSignal=null（基线：{growth: 3}）；对下限做 > → >= 变异 → 套件全绿，而 srcDiffLines: 100 探针翻转为触发（基线：null）。建议修复：补一个恰好 3 倍的触发用例（prevLedger src0: 100、plan srcDiffLines: 300、第 6 轮 → 期望信号且 growth: 3）和一个下限边界用例（srcDiffLines: 100 → 期望 null）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(r.approachSignal).toBeNull(); | ||
| }); | ||
|
|
||
| it('honours the operator round threshold, and falls back to the built-in on 0', () => { |
There was a problem hiding this comment.
[Suggestion] R1-8: This test pins neither semantics of its title. The mock replaces only loadSettings (lines 57-83), so the real operatorReviewSettings re-validation runs and {approachRounds: 0} genuinely becomes absent → default 5 (verified end-to-end: at round 2 the signal is null). But the round-6 assertion .not.toBeNull() fires identically under "falls back to 5" and "gate disabled (round >= 0)", and compose-review's own ?? APPROACH_ROUNDS_DEFAULT (compose-review.ts:1953) does NOT fall back on a literal 0 — the 0→absent conversion happens only in lib/review-settings.ts. The title states a contract the unit under test does not implement. — Failure scenario: if the settings-layer re-validation ever regresses, a hand-edited approachRounds: 0 — the natural operator "off switch" — reaches compose-review and the signal fires from the first round carrying a baseline; this test stays green through the whole regression because at round 6 both semantics fire. — Witness: probe approachRounds: 0 at round 2 → approachSignal=null (fallback holds end-to-end today); the advertised contract is unobservable in this test and unimplemented in this unit. Suggested fix: make the semantics observable — compose a below-default round (prevLedger round 1, compose round 2) under approachRounds: 0 and assert null; or give compose-review an explicit positive-integer guard; or drop the 0 arm here and leave it to review-settings.test.ts, which already owns the 0→absent conversion.
中文说明
这个测试对标题所宣称的两种语义都没有钉住。mock 只替换了 loadSettings（第 57-83 行），因此真实的 operatorReviewSettings 再校验仍会运行，{approachRounds: 0} 确实会变成缺失 → 默认 5（已端到端验证：第 2 轮时信号为 null）。但第 6 轮的 .not.toBeNull() 断言在"回落到 5"和"门被禁用（round >= 0）"两种语义下同样触发；而且 compose-review 自己的 ?? APPROACH_ROUNDS_DEFAULT（compose-review.ts:1953）对字面量 0 并不回落——0→缺失的转换只发生在 lib/review-settings.ts。标题宣称的契约在被测单元中并未实现。— 失败场景：若设置层再校验将来回归，手工写入的 approachRounds: 0——运维最自然的"关闭开关"——会到达 compose-review，信号将从第一个有基线的轮次起就触发；而本测试在整个回归过程中保持绿色，因为第 6 轮在两种语义下都触发。— 证据：探针在 approachRounds: 0、第 2 轮时 → approachSignal=null（回落语义目前在端到端成立）；标题宣称的契约在本测试中不可观测、在本单元中未实现。建议修复：让语义可观测——在 approachRounds: 0 下构造一个低于默认阈值的轮次（prevLedger round 1、compose round 2）并断言 null；或给 compose-review 显式的正整数守卫；或删掉此处的 0 分支，交给已经拥有 0→缺失转换的 review-settings.test.ts。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — this round hit the ~8-finding batch bound after the merge from main (three conflicted files resolved) plus the two Criticals' re-verification and eight implemented findings. This one is next in line: the plan is the finding's first option, composing a below-default round (prev ledger round 1, compose round 2) under approachRounds: 0 and asserting null, so the fallback semantics the title claims become observable at a round where the two semantics diverge.
中文说明
延后到下一轮——本轮在合并 main（解决三个冲突文件）、复查两个 Critical 之外已实施八条 finding，达到单轮约 8 条的批次上限。本条排在下一轮首位：计划采用该 finding 的第一个方案，在 approachRounds: 0 下构造低于默认阈值的轮次（prev ledger 第 1 轮、compose 第 2 轮）并断言 null，使标题宣称的回落语义在两种语义分叉的轮次上可观测。
| "type": "number", | ||
| "default": 0 | ||
| }, | ||
| "approachRounds": { |
There was a problem hiding this comment.
[Suggestion] R1-10: The new review.approachRounds setting is added to the CLI schema, the generated vscode editor schema, and the settings dialog, but gets no row in the exhaustive #### review table in docs/users/configuration/settings.md — which lists all five pre-existing review.* settings, including the sibling review.reverseAuditRounds — so the only operator knob this feature introduces is undocumented in the canonical settings reference. — Failure scenario: an operator consulting that reference finds no mention of review.approachRounds; the setting is discoverable only by reading schema descriptions, and a future settings-doc sweep has no way to know the row is owed. Local convention: the sibling reverseAuditRounds got its docs row in its own feature PR (PR 9203). — Witness: not run — documentation-omission claim settled by reading settings.md:110-118, settingsSchema.ts:711-795, and git history at the reviewed commit. Suggested fix: add a review.approachRounds row to the #### review table (type number, default 0 (unset)): advisory paragraph threshold, fires only with matching growth and never on Approve, 0 keeps the built-in 5, non-whole/non-positive values ignored.
中文说明
新增的 review.approachRounds 设置已加入 CLI schema、生成的 vscode 编辑器 schema 和设置对话框，但 docs/users/configuration/settings.md 中详尽的 #### review 表格——列出了全部五个既有 review.* 设置，包括同类的 review.reverseAuditRounds——没有它的行，因此本功能引入的唯一运维开关在权威设置参考中是无文档的。— 失败场景：查阅该参考的运维人员找不到 review.approachRounds；这个设置只能通过读 schema 描述发现，而未来的设置文档清理也无从知道欠了这一行。本地惯例：同类设置 reverseAuditRounds 在它自己的功能 PR（PR 9203）中就补上了文档行。— 证据：未运行——文档缺失的结论通过阅读被审 commit 上的 settings.md:110-118、settingsSchema.ts:711-795 与 git 历史得出。建议修复：在 #### review 表格中补一行 review.approachRounds（类型 number，默认 0（未设置））：提示段落阈值，仅在增长匹配时触发、Approve 上永不触发，0 保留内置的 5，非整数/非正值被忽略。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — this round hit the ~8-finding batch bound after the merge from main plus the two Criticals' re-verification. The row is owed exactly as described and will be added to the #### review table in docs/users/configuration/settings.md next round (type number, default 0 unset, with the threshold/growth/Approve-exemption semantics the finding spells out).
中文说明
延后到下一轮——本轮在合并 main 与复查两个 Critical 之外已达到约 8 条的批次上限。该文档行确实欠着、描述无误，下一轮将按 finding 给出的语义（类型 number、默认 0 未设置，含阈值/增长/Approve 豁免说明）补入 docs/users/configuration/settings.md 的 #### review 表格。
已被后续 commit 取代，当前 head 需重新 review
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.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2676 — [probe] COMMENT rendering path for a fired signal never exercisedpackages/cli/src/commands/review/compose-review.ts:965 — [probe] prevLedgerFor's garbled-src0 guard has no testpackages/vscode-ide-companion/schemas/settings.schema.json:263 (+2 locations) — [review] approachRounds declares number where integer is supporteddocs/design/2026-08-17-review-approach-signal.md:98 — [probe] doc claims force-push disarms the signal; recovery re-arms it
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // by a build predating the approach signal carries no field at all, and a | ||
| // mid-upgrade load must not fail over one that only affects display. A | ||
| // PRESENT value of the wrong shape is refused like every other field here. | ||
| const approachRaw = verdict['approachSignal'] ?? null; |
There was a problem hiding this comment.
[Suggestion] R1-1 (still standing since round 1): The new approachSignal branches in validateVerdict — the refusal arms for a present-but-malformed value and the absent-means-null upgrade path (verdict['approachSignal'] ?? null) — still have zero tests, while the sibling deferredCount block directly below has both shapes ("refuses a present deferredCount of the wrong shape", "reads an absent or null deferredCount as zero"). — Failure scenario: delete any one refusal arm and every save-artifact test stays green (the copy test only proves a VALID object passes through), so a composed.json carrying approachSignal: { round: -1 } or growth: NaN persists into the durable schema-v1 artifact instead of being refused; replacing ?? null with a presence-required pattern makes every pre-feature composed file fail to save, uncaught. — Witness (probe at this commit): mutant deleting the refusal block → 4 probe tests fail while all 26 existing tests stay green; mutant making the field presence-required → only the absent/null probe fails. Suggested fix: mirror the deferredCount pattern — an it.each of wrong shapes asserting toThrow(/approachSignal/), plus one case with the key deleted and one with approachSignal: null, asserting the artifact persists with verdict.approachSignal === null.
中文说明
validateVerdict 中新增的 approachSignal 分支——对"存在但形态错误"值的各拒绝分支，以及"缺失即 null"的升级兼容路径（verdict['approachSignal'] ?? null）——仍然没有任何测试（自第 1 轮起未处理）；而紧邻其下的同类字段 deferredCount 两种形态都有测试。— 失败场景：删除任意一个拒绝分支，全部 save-artifact 测试仍然全绿（copy 测试只证明合法对象能透传），于是携带 approachSignal: { round: -1 } 或 growth: NaN 的 composed.json 会被原样持久化进 schema-v1 产物而不是被拒绝；若把 ?? null 换成要求字段存在的模式，所有旧版本产物都会在保存时抛错且无测试拦截。— 证据（在本 commit 上探测）：删除拒绝块的变异体 → 4 个探针用例失败而既有 26 个测试全绿；"字段必须存在"变异体 → 仅缺失/null 探针失败。建议修复：仿照 deferredCount 的模式——it.each 断言 toThrow(/approachSignal/)，再补"键被删除"与 approachSignal: null 两个用例，断言产物以 verdict.approachSignal === null 持久化。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| for (const key of ['round', 'src0', 'srcDiffLines'] as const) { | ||
| if ( | ||
| typeof signal[key] !== 'number' || | ||
| !Number.isInteger(signal[key]) || | ||
| (signal[key] as number) < 0 | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-2 (still standing since round 1): This loop admits 0 for round, src0, and srcDiffLines (it checks < 0), but every producer invariant guarantees strictly positive values — the signal composes only when prevSrc0 > 0, src > LOW_SIGNAL_SRC_DIFF_LINES (100) and round = prevRound + 1 ≥ 1 — and lib/ledger.ts validates the same-named src0 as strictly positive on both read and write. The artifact gate is looser than the semantics of the data it certifies; unchanged since round 1, still present at this commit. — Failure scenario: a hand-composed composed.json {"round": 0, "src0": 0, "srcDiffLines": 0, "growth": 4, "nonConverged": false} passes every check and persists as an internally impossible record ("grew 4x from a zero baseline, at round 0") that the verdict-line renderer would quote verbatim. — Witness: round-1 probe saving that exact object → saveReviewArtifact did not throw and persisted the all-zero signal verbatim; round-2 re-read confirms the loop still checks < 0. Suggested fix: reject <= 0 here, matching lib/ledger.ts's src0 rule; or, if zero is deliberately admissible in the artifact, say so in the comment so the asymmetry stops looking accidental. (The error message just below sits outside the anchored range and needs the matching update to "must be a positive integer.")
| for (const key of ['round', 'src0', 'srcDiffLines'] as const) { | |
| if ( | |
| typeof signal[key] !== 'number' || | |
| !Number.isInteger(signal[key]) || | |
| (signal[key] as number) < 0 | |
| ) { | |
| for (const key of ['round', 'src0', 'srcDiffLines'] as const) { | |
| if ( | |
| typeof signal[key] !== 'number' || | |
| !Number.isInteger(signal[key]) || | |
| (signal[key] as number) <= 0 | |
| ) { |
中文说明
该循环对 round、src0、srcDiffLines 允许 0（只检查 < 0），但所有生产者不变量都保证严格为正——信号仅在 prevSrc0 > 0、src > LOW_SIGNAL_SRC_DIFF_LINES（100）且 round = prevRound + 1 ≥ 1 时才会生成——而且 lib/ledger.ts 对同名字段 src0 在读写两侧都按严格正整数校验。产物校验门比它所认证数据的语义更宽松；自第 1 轮起未改动，本 commit 上仍然存在。— 失败场景：手工构造的 composed.json {"round": 0, "src0": 0, "srcDiffLines": 0, "growth": 4, "nonConverged": false} 能通过全部检查并被持久化为一条内部不可能的记录（"从 0 基线增长 4 倍、在第 0 轮"），判定行渲染器还会原样引用它。— 证据：第 1 轮以该对象调用保存 → 未抛错且全零信号被原样持久化；第 2 轮复查确认循环仍检查 < 0。建议修复：此处改为拒绝 <= 0，与 lib/ledger.ts 的 src0 规则一致；若确实有意允许零，请在注释中说明，让这一不对称不再像无意疏漏。（紧邻其下的错误消息在锚定范围之外，需一并改为 "must be a positive integer."。）
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // has already fired on rounds and growth. | ||
| roundCapStopped = isRoundCap; |
There was a problem hiding this comment.
[Suggestion] R1-3 (still standing since round 1): The corroborating-clause half of the signal — the roundCapStopped wiring here and both en/zh nonConverged branches in the rendered paragraph — still has no test: every firing test uses a plan with no stop file, so nonConverged is always false (the save-artifact fixture's nonConverged: true exercises only the artifact validator, never this rendering). — Failure scenario: delete roundCapStopped = isRoundCap; or the (approachSignal.nonConverged ? ... : '') branches in both language blocks and every test stays green — the clause silently vanishes from exactly the incident shape that motivated this feature (round-cap stop + ballooned diff). — Witness (probe at this commit): two mutants (wiring deleted; both clause ternaries deleted) → each gives 1 probe failure with all 288 existing tests green. Suggested fix (writeRoundCapStop is already imported in the test file):
const planPath = ballooned();
writeRoundCapStop(planPath, 5, 6);
// assert r.approachSignal?.nonConverged === true and
// r.body contains 'the reverse audit also stopped at its round cap without converging'中文说明
信号的佐证子句这一半——此处 roundCapStopped 的接线，以及渲染段落中 en/zh 两处 nonConverged 分支——仍然没有测试：所有触发用例的 plan 都没有 stop 文件，因此 nonConverged 恒为 false（save-artifact fixture 里的 nonConverged: true 只过产物校验器，到不了这里的渲染）。— 失败场景：删除 roundCapStopped = isRoundCap; 或两种语言块中的 (approachSignal.nonConverged ? ... : '') 分支，所有测试保持全绿——该子句会从恰恰催生本功能的形态（轮数上限停止 + diff 膨胀）中悄悄消失。— 证据（在本 commit 上探测）：两个变异体（删接线；删两处子句三元）→ 各产生 1 个探针失败，既有 288 个测试全绿。建议修复（writeRoundCapStop 已被测试文件引入）：ballooned() 后 writeRoundCapStop(planPath, 5, 6)，断言 nonConverged === true 且正文包含相应英文子句。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| return { | ||
| round: Number.isInteger(prev.round) && prev.round > 0 ? prev.round : 0, | ||
| src0: | ||
| Number.isInteger(prev.src0) && (prev.src0 as number) > 0 | ||
| ? (prev.src0 as number) | ||
| : 0, | ||
| }; |
There was a problem hiding this comment.
[Suggestion] R1-4 (still standing since round 1): The one link of the src0 chain this read depends on — src0 surviving pr-context's persistRecoveredLedger into the side file — is pinned by no test on either side: src0 appears nowhere in any pr-context test (re-verified by grep at this commit: 0 matches), and the compose-review tests write the side file directly, bypassing that writer. The field flows today only via spread-based writers (the chain is intact at this commit), so this is a coverage gap, not a live bug. — Failure scenario: a future edit to persistRecoveredLedger swapping its whole-object spreads for explicit field selection — the pattern its own anonymous-recovery branch already uses — drops src0 from the side file; prevLedgerFor then reads src0: 0 every round, the approach signal never fires again in production, and every existing test stays green — the feature dies silently, failing toward silence. — Witness: grep over pr-context-persist.test.ts at the reviewed commit: 0 matches for src0; this is the only unpinned joint in the marker→parse→persist→read chain. Suggested fix: in pr-context-persist.test.ts, recover a ledger whose marker carries src0: 228, run persistRecoveredLedger, and assert the written side file still contains "src0": 228 — ideally also across the anonymous-over-existing branch.
中文说明
此处读取所依赖的 src0 链条中，有一环仍然没有任何一侧的测试约束——src0 能否经由 pr-context 的 persistRecoveredLedger 存活进 side file：所有 pr-context 测试中都搜不到 src0（在本 commit 上复查：0 处匹配），而 compose-review 测试是直接写 side file 的，绕过了那个写入器。该字段目前只靠基于展开（spread）的写入方式传递（本 commit 上链条完好），因此这是覆盖缺口，不是现存缺陷。— 失败场景：未来若把 persistRecoveredLedger 的整体展开改为显式字段挑选——它自己的匿名恢复分支已经在用这种模式——src0 就会从 side file 中掉出；prevLedgerFor 此后每轮都读到 src0: 0，approach 信号在生产上永远不再触发，而所有现有测试依旧全绿——功能静默失效。— 证据：在被审 commit 上对 pr-context-persist.test.ts grep src0：0 处匹配；这是 marker→parse→persist→read 链条上唯一没有测试钉住的接点。建议修复：在 pr-context-persist.test.ts 中恢复一个 marker 携带 src0: 228 的 ledger，运行 persistRecoveredLedger，断言写出的 side file 仍含 "src0": 228——最好连"匿名覆盖既有文件"分支也覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| zh: | ||
| `⚠️ 第 ${approachSignal.round} 轮，且自本审查首次测量以来 diff 已增长 ` + |
There was a problem hiding this comment.
[Suggestion] R1-6 (still standing since round 1): The Chinese half of this paragraph has no test — no han: true case fires the signal, although the module's convention tests bilingual renderings of other clauses that way. — Failure scenario: break or truncate the zh string (e.g. drop the 仅供参考 advisory-only sentence) and all tests stay green; a PR whose description carries Han characters renders the broken paragraph in the posted body and nothing ever saw it. — Witness (probe at this commit): one-character zh mutant (第 → 回) → Tests 281 passed (281); adding one ballooned({ han: true }) case asserting ⚠️ 第 6 轮 → 1 failed | 281 passed against the mutant, 282 passed pristine. Suggested fix: add that one case — const planPath = ballooned({ han: true }); and assert r.body contains the zh fragments (第 6 轮, 228 → 920, 仅供参考) inside the fold (coveredPlan already threads han into prDescriptionHasHan).
const planPath = ballooned({ han: true });
// assert r.body contains '⚠️ 第 6 轮' and '仅供参考' inside the 中文说明 fold中文说明
本段落的中文半边仍无测试——没有任何 han: true 用例触发该信号，尽管本模块对其他子句的双语渲染正是这样测的。— 失败场景：破坏或截断中文字符串（例如删掉"仅供参考"那句），所有测试依旧全绿；而描述含中文的 PR 会在发布正文中渲染出损坏的段落，且从未有任何测试看到。— 证据（在本 commit 上探测）：中文单字变异（第 → 回）→ 281 个测试全部通过；补一个 ballooned({ han: true }) 用例断言 ⚠️ 第 6 轮 后 → 对变异体 1 失败 | 281 通过，原始代码 282 通过。建议修复：补上这一个用例（coveredPlan 已把 han 传入 prDescriptionHasHan）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if ( | ||
| Number.isFinite(src) && | ||
| src > LOW_SIGNAL_SRC_DIFF_LINES && | ||
| growth >= APPROACH_GROWTH_FACTOR | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-7 (still standing since round 1): Neither comparison of the growth arm has a boundary test: every firing case uses 920/228 ≈ 4.04x against APPROACH_GROWTH_FACTOR (3) and src 920 against the 100-line floor, so mutants on growth >= APPROACH_GROWTH_FACTOR and src > LOW_SIGNAL_SRC_DIFF_LINES stay green. — Failure scenario: mutating the factor comparison >= to > leaves the suite green and a PR growing from baseline 100 to exactly 300 source lines — exactly the documented "grown by at least 3x" — never receives the advisory, and because the baseline ratchets (src0 carries forward unchanged; a shrink cannot raise it), a PR that later shrinks can never fire at all. Symmetrically, > → >= on the floor fires at exactly 100 lines, contrary to "past the floor". — Witness (probe at this commit): factor mutant → only the (src0: 100, srcDiffLines: 300) probe fails; floor mutant → only the src-exactly-100 probe fails; all 281 existing tests green under both mutants. Suggested fix: a fires-at-exactly-3x case (prev ledger src0: 100, plan srcDiffLines: 300, round 6 → expect signal with growth: 3) and a floor-boundary case (srcDiffLines: 100 → expect null).
// fires at exactly 3x: prevLedger src0: 100, plan srcDiffLines: 300 → growth: 3
// floor boundary: plan srcDiffLines: 100 → expect approachSignal null中文说明
增长条件的两个比较仍无边界测试：所有触发用例都用 920/228 ≈ 4.04 倍对 APPROACH_GROWTH_FACTOR（3）、src 920 对 100 行下限，因此对 growth >= APPROACH_GROWTH_FACTOR 与 src > LOW_SIGNAL_SRC_DIFF_LINES 的变异仍全绿。— 失败场景：把倍数比较 >= 变异为 > 后套件全绿；一个从基线 100 恰好增长到 300 行源码的 PR——正是文档所说的"至少增长 3 倍"——永远收不到提示，而且由于基线只进不退（src0 原样传递，缩小不会抬升它），之后缩小的 PR 永远无法触发。对称地，把下限的 > 变异为 >= 会在恰好 100 行时触发，与"超过下限"的表述相悖。— 证据（在本 commit 上探测）：倍数变异体 → 仅（src0: 100、srcDiffLines: 300）探针失败；下限变异体 → 仅 src 恰好 100 的探针失败；两种变异下既有 281 个测试全绿。建议修复：补一个恰好 3 倍的触发用例（prev ledger src0: 100、plan srcDiffLines: 300、第 6 轮 → 期望信号且 growth: 3）和一个下限边界用例（srcDiffLines: 100 → 期望 null）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| reviewSettingsMock.mockReturnValue({ approachRounds: 0 }); | ||
| expect( |
There was a problem hiding this comment.
[Suggestion] R1-8 (still standing since round 1): This test pins neither semantics of its title. The mock replaces only loadSettings, so the real operatorReviewSettings re-validation converts {approachRounds: 0} → undefined → default 5 — the fallback the title claims lives in lib/review-settings.ts, not in the compose-review seam under test, whose ?? APPROACH_ROUNDS_DEFAULT does not fall back on a literal 0. At round 6, .not.toBeNull() fires identically under "falls back to 5" and "gate disabled". — Failure scenario: if the settings-layer re-validation ever regresses, a hand-edited approachRounds: 0 — the natural operator off-switch — reaches compose-review and the signal fires from the first round carrying a baseline; this test stays green through the whole regression because at round 6 both semantics fire. — Witness (probe at this commit): with the settings-layer regression injected, the shipped test stayed green while a round-3 discriminator fired ({"round":3,"src0":228,"srcDiffLines":920,…}); reverted, the same probe read null — with the shipped test passing in both states. Suggested fix: make the semantics observable — compose a below-default round (prev ledger round 1, compose round 2) under approachRounds: 0 and assert null; or give compose-review an explicit positive-integer guard at this seam; or drop the 0 arm here and leave it to review-settings.test.ts, which already owns the 0→absent conversion.
中文说明
这个测试对标题所宣称的两种语义仍未钉住。mock 只替换 loadSettings，因此真实的 operatorReviewSettings 再校验会把 {approachRounds: 0} 转成 undefined → 默认 5——标题宣称的回落语义位于 lib/review-settings.ts，而不在被测的 compose-review 接缝；后者的 ?? APPROACH_ROUNDS_DEFAULT 对字面量 0 并不回落。在第 6 轮，.not.toBeNull() 在"回落到 5"和"门被禁用"两种语义下同样触发。— 失败场景：若设置层再校验将来回归，手工写入的 approachRounds: 0——运维最自然的"关闭开关"——会到达 compose-review，信号将从第一个有基线的轮次起就触发；而本测试在整个回归过程中保持绿色，因为第 6 轮在两种语义下都触发。— 证据（在本 commit 上探测）：注入设置层回归后，出厂测试依旧全绿，而第 3 轮判别器触发（{"round":3,"src0":228,"srcDiffLines":920,…}）；撤销注入后同一判别器读作 null——两种状态下出厂测试都通过。建议修复：让语义可观测——在 approachRounds: 0 下构造一个低于默认阈值的轮次（prev ledger round 1、compose round 2）并断言 null；或给 compose-review 在此接缝显式的正整数守卫；或删掉此处的 0 分支，交给已经拥有 0→缺失转换的 review-settings.test.ts。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — same finding (R1-8), reported again on the round-2 re-post; the round-1 reply on this thread applies: this round hit the ~8-finding batch bound after the merge from main plus the Criticals' re-verification. Planned fix is unchanged: make the fallback observable at a below-default round under approachRounds: 0.
中文说明
延后到下一轮——同一 finding（R1-8）在第 2 轮重发；本线程第 1 轮的回复仍然适用：本轮在合并 main 与复查 Critical 之外已达到约 8 条的批次上限。计划修复不变：在 approachRounds: 0 下用低于默认阈值的轮次使回落语义可观测。
| approachRounds: { | ||
| type: 'number', |
There was a problem hiding this comment.
[Suggestion] R1-10 (still standing since round 1): The new review.approachRounds setting is added to the CLI schema, the generated vscode editor schema, and the settings dialog, but gets no row in the exhaustive #### review table in docs/users/configuration/settings.md — which lists all five pre-existing review.* settings, including the sibling review.reverseAuditRounds — so the only operator knob this feature introduces is undocumented in the canonical settings reference. Re-verified at this commit: the table (settings.md:112-118) has exactly five rows, the diff never touches the file, and no doc/schema sync test exists. — Failure scenario: an operator consulting that reference finds no mention of review.approachRounds; the setting is discoverable only by reading schema descriptions, and a future settings-doc sweep has no way to know the row is owed. Local convention: the sibling reverseAuditRounds got its docs row in its own feature PR (PR 9203). Suggested fix: add a review.approachRounds row to the #### review table — type number, default 0 (unset): advisory paragraph threshold; fires only with matching growth and never on an Approve; 0 keeps the built-in 5; non-whole/non-positive values ignored.
中文说明
新增的 review.approachRounds 设置已加入 CLI schema、生成的 vscode 编辑器 schema 和设置对话框，但 docs/users/configuration/settings.md 中详尽的 #### review 表格——列出了全部五个既有 review.* 设置，包括同类的 review.reverseAuditRounds——仍没有它的行，因此本功能引入的唯一运维开关在权威设置参考中是无文档的。在本 commit 上复查：该表格（settings.md:112-118）恰好五行、diff 从未触碰该文件、也不存在文档/schema 同步测试。— 失败场景：查阅该参考的运维人员找不到 review.approachRounds；这个设置只能通过读 schema 描述发现，而未来的设置文档清理也无从知道欠了这一行。本地惯例：同类设置 reverseAuditRounds 在它自己的功能 PR（PR 9203）中就补上了文档行。建议修复：在 #### review 表格中补一行 review.approachRounds（类型 number，默认 0（未设置））：提示段落阈值，仅在增长匹配时触发、Approve 上永不触发，0 保留内置的 5，非整数/非正值被忽略。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — same finding (R1-10), reported again on the round-2 re-post; the round-1 reply on this thread applies. The settings-reference row is scheduled for the next round.
中文说明
延后到下一轮——同一 finding（R1-10）在第 2 轮重发；本线程第 1 轮的回复仍然适用。设置参考文档行已排入下一轮。
|
|
||
| It fires when all of: | ||
|
|
||
| - the composed event is not `APPROVE` |
There was a problem hiding this comment.
[Suggestion] R2-2: The design doc's firing list says the signal fires when "the composed event is not APPROVE", but the implementation gates on baseEvent — the verdict BEFORE caps and presubmit downgrades (compose-review.ts:1950) — so a capped or presubmit-downgraded Approve, whose composed event is COMMENT, never fires. This bullet was accurate when round 1 wrote it; round 1's own R1-9 fix (the one-line change at compose-review.ts:1950) made it stale, so the doc is newly wrong at this commit. — Failure scenario: round 6, baseline grown 228 → 920, zero findings this round, composed COMMENT via cap or downgrade — all five documented conditions hold per the doc (the composed event is COMMENT, not APPROVE), yet nothing is emitted; a maintainer following the committed design doc would conclude the machinery malfunctioned. The exclusion is deliberate code behavior, pinned by the test "never fires when an APPROVE is downgraded to COMMENT", so the doc is the side that must change. — Witness: that pinned test was run at the reviewed commit: 1 passed (asserts baseEvent APPROVE, event COMMENT, approachSignal null).
| - the composed event is not `APPROVE` | |
| - this round confirmed at least one finding (the pre-cap verdict is not `APPROVE`) |
中文说明
设计文档的触发条件列表写的是"合成事件不是 APPROVE"，但实现的门是 baseEvent——即 caps 与 presubmit 降级之前的判定（compose-review.ts:1950）——因此一个被 cap 或被降级的 Approve（其合成事件为 COMMENT）永不触发。该条目在第 1 轮写入时是准确的；第 1 轮自己的 R1-9 修复（compose-review.ts:1950 处的一行改动）使它过时，因此在被审 commit 上文档是新近变错的。— 失败场景：第 6 轮、基线 228 → 920、本轮零发现、因 cap 或降级合成 COMMENT——按文档五个条件全部成立（合成事件是 COMMENT 而非 APPROVE），却什么也不会输出；按设计文档推理的维护者会认为机制出了故障。排除行为是代码的刻意设计，并由测试 "never fires when an APPROVE is downgraded to COMMENT" 钉住，因此该改的是文档。— 证据：在被审 commit 上运行该测试：1 通过（断言 baseEvent 为 APPROVE、event 为 COMMENT、approachSignal 为 null）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
已被后续 commit 取代，当前 head 需重新 review
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. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:2689 — [probe] COMMENT rendering path for a fired signal never exerciseddocs/design/2026-08-17-review-approach-signal.md:97 — [review] doc claims force-push disarms the signal; recovery re-arms itpackages/core/src/skills/bundled/review/SKILL.md:773 — [review] SKILL.md says "on a non-Approve round"; the gate is baseEventpackages/core/src/skills/bundled/review/DESIGN.md:779 — [review] narrative: "two PRs" lists three; "#9325 landed" is unmerged
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const approachRaw = verdict['approachSignal'] ?? null; | ||
| if (approachRaw !== null) { |
There was a problem hiding this comment.
[Suggestion] R1-1 (still standing since round 1): The new approachSignal branches in validateVerdict — the refusal arms for a present-but-malformed value and the absent-means-null upgrade path (verdict['approachSignal'] ?? null) — still have zero tests, while the sibling deferredCount block directly below has both shapes ("refuses a present deferredCount of the wrong shape", "reads an absent or null deferredCount as zero"). — Failure scenario: delete any one refusal arm and every save-artifact test stays green (the copy test only proves a VALID object passes through), so a composed.json carrying approachSignal: { round: -1 } or growth: NaN persists into the durable schema-v1 artifact instead of being refused; replacing ?? null with a presence-required pattern makes every pre-feature composed file fail to save, uncaught. — Witness (probe at this commit): mutant deleting the refusal block → 4 probe tests fail while all 26 existing tests stay green; mutant making the field presence-required → only the absent/null probe fails. Suggested fix: mirror the deferredCount pattern — an it.each of wrong shapes asserting toThrow(/approachSignal/), plus one case with the key deleted and one with approachSignal: null, asserting the artifact persists with verdict.approachSignal === null.
中文说明
validateVerdict 中新增的 approachSignal 分支——对"存在但形态错误"值的各拒绝分支，以及"缺失即 null"的升级兼容路径（verdict['approachSignal'] ?? null）——仍然没有任何测试（自第 1 轮起未处理）；而紧邻其下的同类字段 deferredCount 两种形态都有测试。— 失败场景：删除任意一个拒绝分支，全部 save-artifact 测试仍然全绿（copy 测试只证明合法对象能透传），于是携带 approachSignal: { round: -1 } 或 growth: NaN 的 composed.json 会被原样持久化进 schema-v1 产物而不是被拒绝；若把 ?? null 换成要求字段存在的模式，所有旧版本产物都会在保存时抛错且无测试拦截。— 证据（在本 commit 上探测）：删除拒绝块的变异体 → 4 个探针用例失败而既有 26 个测试全绿；"字段必须存在"变异体 → 仅缺失/null 探针失败。建议修复：仿照 deferredCount 的模式——it.each 断言 toThrow(/approachSignal/)，再补"键被删除"与 approachSignal: null 两个用例，断言产物以 verdict.approachSignal === null 持久化。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // entry keeps its existing coverage cap untouched, and this flag is | ||
| // never a trigger on its own — only a clause appended when the signal | ||
| // has already fired on rounds and growth. | ||
| roundCapStopped = isRoundCap; |
There was a problem hiding this comment.
[Suggestion] R1-3 (still standing since round 1): The corroborating-clause half of the signal — the roundCapStopped wiring here and both en/zh nonConverged branches in the rendered paragraph — still has no test: every firing test uses a plan with no stop file, so nonConverged is always false (the save-artifact fixture's nonConverged: true exercises only the artifact validator, never this rendering). — Failure scenario: delete roundCapStopped = isRoundCap; or the (approachSignal.nonConverged ? ... : '') branches in both language blocks and every test stays green — the clause silently vanishes from exactly the incident shape that motivated this feature (round-cap stop + ballooned diff). — Witness (probe at this commit): two mutants (wiring deleted; both clause ternaries deleted) → each gives 1 probe failure with all 288 existing tests green. Suggested fix (writeRoundCapStop is already imported in the test file):
const planPath = ballooned();
writeRoundCapStop(planPath, 5, 6);
// assert r.approachSignal?.nonConverged === true and
// r.body contains 'the reverse audit also stopped at its round cap without converging'中文说明
信号的佐证子句这一半——此处 roundCapStopped 的接线，以及渲染段落中 en/zh 两处 nonConverged 分支——仍然没有测试：所有触发用例的 plan 都没有 stop 文件，因此 nonConverged 恒为 false（save-artifact fixture 里的 nonConverged: true 只过产物校验器，到不了这里的渲染）。— 失败场景：删除 roundCapStopped = isRoundCap; 或两种语言块中的 (approachSignal.nonConverged ? ... : '') 分支，所有测试保持全绿——该子句会从恰恰催生本功能的形态（轮数上限停止 + diff 膨胀）中悄悄消失。— 证据（在本 commit 上探测）：两个变异体（删接线；删两处子句三元）→ 各产生 1 个探针失败，既有 288 个测试全绿。建议修复（writeRoundCapStop 已被测试文件引入）：ballooned() 后 writeRoundCapStop(planPath, 5, 6)，断言 nonConverged === true 且正文包含相应英文子句。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| src0: | ||
| Number.isInteger(prev.src0) && (prev.src0 as number) > 0 | ||
| ? (prev.src0 as number) | ||
| : 0, |
There was a problem hiding this comment.
[Suggestion] R1-4 (still standing since round 1): The one link of the src0 chain this read depends on — src0 surviving pr-context's persistRecoveredLedger into the side file — is pinned by no test on either side: src0 appears nowhere in any pr-context test (re-verified by grep at this commit: 0 matches), and the compose-review tests write the side file directly, bypassing that writer. The field flows today only via spread-based writers (the chain is intact at this commit), so this is a coverage gap, not a live bug. — Failure scenario: a future edit to persistRecoveredLedger swapping its whole-object spreads for explicit field selection — the pattern its own anonymous-recovery branch already uses — drops src0 from the side file; prevLedgerFor then reads src0: 0 every round, the approach signal never fires again in production, and every existing test stays green — the feature dies silently, failing toward silence. — Witness: grep over pr-context-persist.test.ts at the reviewed commit: 0 matches for src0; this is the only unpinned joint in the marker→parse→persist→read chain. Suggested fix: in pr-context-persist.test.ts, recover a ledger whose marker carries src0: 228, run persistRecoveredLedger, and assert the written side file still contains "src0": 228 — ideally also across the anonymous-over-existing branch.
中文说明
此处读取所依赖的 src0 链条中，有一环仍然没有任何一侧的测试约束——src0 能否经由 pr-context 的 persistRecoveredLedger 存活进 side file：所有 pr-context 测试中都搜不到 src0（在本 commit 上复查：0 处匹配），而 compose-review 测试是直接写 side file 的，绕过了那个写入器。该字段目前只靠基于展开（spread）的写入方式传递（本 commit 上链条完好），因此这是覆盖缺口，不是现存缺陷。— 失败场景：未来若把 persistRecoveredLedger 的整体展开改为显式字段挑选——它自己的匿名恢复分支已经在用这种模式——src0 就会从 side file 中掉出；prevLedgerFor 此后每轮都读到 src0: 0，approach 信号在生产上永远不再触发，而所有现有测试依旧全绿——功能静默失效。— 证据：在被审 commit 上对 pr-context-persist.test.ts grep src0：0 处匹配；这是 marker→parse→persist→read 链条上唯一没有测试钉住的接点。建议修复：在 pr-context-persist.test.ts 中恢复一个 marker 携带 src0: 228 的 ledger，运行 persistRecoveredLedger，断言写出的 side file 仍含 "src0": 228——最好连"匿名覆盖既有文件"分支也覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const round = prevRound + 1; | ||
| const rounds = | ||
| operatorReviewSettings().approachRounds ?? APPROACH_ROUNDS_DEFAULT; | ||
| if (round >= rounds) { |
There was a problem hiding this comment.
[Suggestion] R1-5 (still standing since round 1): The fires-at-exactly-the-threshold boundary is untested: every firing case uses round 6 against threshold 5, so an off-by-one mutant on round >= rounds stays green. — Failure scenario: mutating >= to > leaves the whole suite green; a PR whose prev ledger records round 4 (this round = 5 = threshold) should fire per the design doc ("at or past the threshold") and the schema's "must reach", but under the mutant the signal silently waits an extra round with no test noticing. — Witness (probe at this commit): mutant >= → > → 1 probe failure with all 288 existing tests green (probe: prev ledger round 4 → round 5 = exactly the default threshold). Suggested fix: one case with a prev ledger { v: 1, round: 4, findings: [], src0: 228 } and srcDiffLines: 920, asserting approachSignal is non-null at round 5 === the default threshold.
prevLedger(planPath, { v: 1, round: 4, findings: [], src0: 228 });
// compose round 5 === APPROACH_ROUNDS_DEFAULT → expect r.approachSignal non-null中文说明
"恰好在阈值处触发"的边界仍无测试：所有触发用例都用第 6 轮对阈值 5，因此对 round >= rounds 的差一变异（>= → >）仍然全绿。— 失败场景：变异后整套测试全绿；而 prev ledger 记录第 4 轮的 PR（本轮 = 5 = 阈值）按设计文档（"达到或超过阈值"）与 schema 的 "must reach" 应当触发，但在变异体下信号会无声地多等一轮且无测试察觉。— 证据（在本 commit 上探测）：变异体 >= → > → 1 个探针失败，既有 288 个测试全绿（探针：prev ledger 第 4 轮 → 本轮 5 === 默认阈值）。建议修复：补一个 prev ledger 为 { v: 1, round: 4, findings: [], src0: 228 } 且 srcDiffLines: 920 的用例，断言第 5 轮（=== 默认阈值）时 approachSignal 非空。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(r.approachSignal).toBeNull(); | ||
| }); | ||
|
|
||
| it('honours the operator round threshold, and falls back to the built-in on 0', () => { |
There was a problem hiding this comment.
[Suggestion] R1-8 (still standing since round 1): This test pins neither semantics of its title. The mock replaces only loadSettings, so the real operatorReviewSettings re-validation converts {approachRounds: 0} → undefined → default 5 — the fallback the title claims lives in lib/review-settings.ts, not in the compose-review seam under test, whose ?? APPROACH_ROUNDS_DEFAULT does not fall back on a literal 0. At round 6, .not.toBeNull() fires identically under "falls back to 5" and "gate disabled". — Failure scenario: if the settings-layer re-validation ever regresses, a hand-edited approachRounds: 0 — the natural operator off-switch — reaches compose-review and the signal fires from the first round carrying a baseline; this test stays green through the whole regression because at round 6 both semantics fire. — Witness (probe at this commit): with the settings-layer regression injected, the shipped test stayed green while a round-3 discriminator fired ({"round":3,"src0":228,"srcDiffLines":920,…}); reverted, the same probe read null — with the shipped test passing in both states. Suggested fix: make the semantics observable — compose a below-default round (prev ledger round 1, compose round 2) under approachRounds: 0 and assert null; or give compose-review an explicit positive-integer guard at this seam; or drop the 0 arm here and leave it to review-settings.test.ts, which already owns the 0→absent conversion.
中文说明
这个测试对标题所宣称的两种语义仍未钉住。mock 只替换 loadSettings，因此真实的 operatorReviewSettings 再校验会把 {approachRounds: 0} 转成 undefined → 默认 5——标题宣称的回落语义位于 lib/review-settings.ts，而不在被测的 compose-review 接缝；后者的 ?? APPROACH_ROUNDS_DEFAULT 对字面量 0 并不回落。在第 6 轮，.not.toBeNull() 在"回落到 5"和"门被禁用"两种语义下同样触发。— 失败场景：若设置层再校验将来回归，手工写入的 approachRounds: 0——运维最自然的"关闭开关"——会到达 compose-review，信号将从第一个有基线的轮次起就触发；而本测试在整个回归过程中保持绿色，因为第 6 轮在两种语义下都触发。— 证据（在本 commit 上探测）：注入设置层回归后，出厂测试依旧全绿，而第 3 轮判别器触发（{"round":3,"src0":228,"srcDiffLines":920,…}）；撤销注入后同一判别器读作 null——两种状态下出厂测试都通过。建议修复：让语义可观测——在 approachRounds: 0 下构造一个低于默认阈值的轮次（prev ledger round 1、compose round 2）并断言 null；或给 compose-review 在此接缝显式的正整数守卫；或删掉此处的 0 分支，交给已经拥有 0→缺失转换的 review-settings.test.ts。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — same finding (R1-8), reported again on the round-3 re-post; see the round-1 reply on this thread. Batch bound engaged this round (merge from main + eight implemented findings); this is scheduled next with the below-default-round discriminator approach.
中文说明
延后到下一轮——同一 finding（R1-8）在第 3 轮重发；见本线程第 1 轮的回复。本轮受批次上限约束（合并 main + 实施八条 finding）；本条已排入下一轮，采用低于默认阈值轮次的判别方案。
| 'Lower the reverse-audit loop\'s round cap for every high-effort review. The cap is normally chosen from the diff topology (10 small / 5 chunked; a huge diff is 3 when the run has a review deadline and 5 when it does not, because that reduction answers a CI ceiling and applies only where one exists) because a round costs one agent on a small diff and ~90 minutes on a huge one; this setting can only LOWER whichever tier applies, never raise it — a value that is not a whole number above zero, or that is out of range (below 3, or above the plan\'s own tier), is ignored and leaves the tier alone — JSON Schema has no integer type here, so a fraction validates in an editor and is then discarded at runtime. Understand what it buys before enabling: the loop ends on two consecutive dry rounds, so cutting the cap does not make reviews converge sooner, it makes them stop before converging more often — and every such stop is disclosed as unreviewed scope and caps the verdict at Comment, so a cheaper review is also one that can no longer Approve. To spend LESS on reviews generally, prefer "effort". Nothing here makes a loop run LONGER: a review deadline bounds a run rather than extending it, and on a huge diff setting one lowers the cap from 5 to 3 rather than raising it. Only honored from User, System, and SystemDefaults settings scopes; values set in Workspace settings are ignored, so a repository cannot set review policy for its reviewers.', | ||
| showInDialog: true, | ||
| }, | ||
| approachRounds: { |
There was a problem hiding this comment.
[Suggestion] R1-10 (still standing since round 1): The new review.approachRounds setting is added to the CLI schema, the generated vscode editor schema, and the settings dialog, but gets no row in the exhaustive #### review table in docs/users/configuration/settings.md — which lists all five pre-existing review.* settings, including the sibling review.reverseAuditRounds — so the only operator knob this feature introduces is undocumented in the canonical settings reference. Re-verified at this commit: the table (settings.md:112-118) has exactly five rows, the diff never touches the file, and no doc/schema sync test exists. — Failure scenario: an operator consulting that reference finds no mention of review.approachRounds; the setting is discoverable only by reading schema descriptions, and a future settings-doc sweep has no way to know the row is owed. Local convention: the sibling reverseAuditRounds got its docs row in its own feature PR (PR 9203). Suggested fix: add a review.approachRounds row to the #### review table — type number, default 0 (unset): advisory paragraph threshold; fires only with matching growth and never on an Approve; 0 keeps the built-in 5; non-whole/non-positive values ignored.
中文说明
新增的 review.approachRounds 设置已加入 CLI schema、生成的 vscode 编辑器 schema 和设置对话框，但 docs/users/configuration/settings.md 中详尽的 #### review 表格——列出了全部五个既有 review.* 设置，包括同类的 review.reverseAuditRounds——仍没有它的行，因此本功能引入的唯一运维开关在权威设置参考中是无文档的。在本 commit 上复查：该表格（settings.md:112-118）恰好五行、diff 从未触碰该文件、也不存在文档/schema 同步测试。— 失败场景：查阅该参考的运维人员找不到 review.approachRounds；这个设置只能通过读 schema 描述发现，而未来的设置文档清理也无从知道欠了这一行。本地惯例：同类设置 reverseAuditRounds 在它自己的功能 PR（PR 9203）中就补上了文档行。建议修复：在 #### review 表格中补一行 review.approachRounds（类型 number，默认 0（未设置））：提示段落阈值，仅在增长匹配时触发、Approve 上永不触发，0 保留内置的 5，非整数/非正值被忽略。
— qwen3.8-max via Qwen Code /review (v0.21.13)
There was a problem hiding this comment.
Deferred to the next round — same finding (R1-10), reported again on the round-3 re-post; see the round-1 reply on this thread. The settings-reference row remains scheduled for the next round.
中文说明
延后到下一轮——同一 finding（R1-10）在第 3 轮重发；见本线程第 1 轮的回复。设置参考文档行仍排在下一轮。
|
|
||
| It fires when all of: | ||
|
|
||
| - the composed event is not `APPROVE` |
There was a problem hiding this comment.
[Suggestion] R2-2: The design doc's firing list says the signal fires when "the composed event is not APPROVE", but the implementation gates on baseEvent — the verdict BEFORE caps and presubmit downgrades (compose-review.ts:1950) — so a capped or presubmit-downgraded Approve, whose composed event is COMMENT, never fires. This bullet was accurate when round 1 wrote it; round 1's own R1-9 fix (the one-line change at compose-review.ts:1950) made it stale, so the doc is newly wrong at this commit. — Failure scenario: round 6, baseline grown 228 → 920, zero findings this round, composed COMMENT via cap or downgrade — all five documented conditions hold per the doc (the composed event is COMMENT, not APPROVE), yet nothing is emitted; a maintainer following the committed design doc would conclude the machinery malfunctioned. The exclusion is deliberate code behavior, pinned by the test "never fires when an APPROVE is downgraded to COMMENT", so the doc is the side that must change. — Witness: that pinned test was run at the reviewed commit: 1 passed (asserts baseEvent APPROVE, event COMMENT, approachSignal null).
| - the composed event is not `APPROVE` | |
| - this round confirmed at least one finding (the pre-cap verdict is not `APPROVE`) |
中文说明
设计文档的触发条件列表写的是"合成事件不是 APPROVE"，但实现的门是 baseEvent——即 caps 与 presubmit 降级之前的判定（compose-review.ts:1950）——因此一个被 cap 或被降级的 Approve（其合成事件为 COMMENT）永不触发。该条目在第 1 轮写入时是准确的；第 1 轮自己的 R1-9 修复（compose-review.ts:1950 处的一行改动）使它过时，因此在被审 commit 上文档是新近变错的。— 失败场景：第 6 轮、基线 228 → 920、本轮零发现、因 cap 或降级合成 COMMENT——按文档五个条件全部成立（合成事件是 COMMENT 而非 APPROVE），却什么也不会输出；按设计文档推理的维护者会认为机制出了故障。排除行为是代码的刻意设计，并由测试 "never fires when an APPROVE is downgraded to COMMENT" 钉住，因此该改的是文档。— 证据：在被审 commit 上运行该测试：1 通过（断言 baseEvent 为 APPROVE、event 为 COMMENT、approachSignal 为 null）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(r.body).toMatch(/"src0":228/); | ||
| }); | ||
|
|
||
| it('compares the full-range size on an incremental round', () => { |
There was a problem hiding this comment.
[Suggestion] R3-1: Baseline carry-forward (src0 preserved unchanged in the emitted ledger marker) is pinned by a test only for non-incremental rounds — none of the incremental tests added by the R2-1 fix asserts the marker keeps the previous src0. The shrink-carry-forward test above uses a non-incremental plan; this test and the signal-null case with src0: 100 carry a previous baseline but assert only r.approachSignal, never the emitted marker in r.body. — Failure scenario: a mutant/refactor of ledgerMarkerFor's src0 = prevSrc0 > 0 ? prevSrc0 : measured that re-measures when the round is incremental leaves the whole suite green — the same-round signal reads prevSrc0 from the PREVIOUS ledger file, so same-round assertions still pass — while the marker the NEXT round reads is corrupted: the baseline is rewritten to the current full-range size, erasing the growth record the carry-forward rule exists to protect; a PR that grew 4x over six rounds can then never reach the 3x gate again and the advisory silently never fires. — Witness (probe at the reviewed commit, flips both ways): the mutant leaves 343 tests green; adding the assertion below with the mutant still applied fails (expected '⚠️ Round 6…' to match /"src0":228/ — the mutant emits "src0":920); mutant reverted → green. Suggested fix — add the marked line, plus one legacy-incremental case with a previous baseline asserting the same:
expect(r.approachSignal).toMatchObject({
src0: 228,
srcDiffLines: 920,
});
expect(r.body).toMatch(/"src0":228/); // marker keeps the previous baseline中文说明
基线进位（发射的 ledger marker 原样保留先前的 src0）只有非增量轮次的测试钉住——R2-1 修复新增的增量用例没有一个断言 marker 保留先前的 src0。上方的"缩小仍进位"用例使用非增量 plan；本用例与 src0: 100 的信号沉默用例都带有先前基线，却只断言 r.approachSignal，从不断言 r.body 中发射的 marker。— 失败场景：对 ledgerMarkerFor 的 src0 = prevSrc0 > 0 ? prevSrc0 : measured 做"增量轮重新测量"的变异（或重构）后整套测试仍全绿——同轮信号读的是上一轮 ledger 文件中的 prevSrc0，同轮断言依旧通过——而下一轮读到的 marker 已被污染：基线被改写为当前全量大小，抹掉进位规则本要保护的增长记录；一个六轮增长 4 倍的 PR 将永远无法再达到 3 倍门槛，提示永远静默。— 证据（在被审 commit 上探测，双向翻转）：变异体下 343 个测试全绿；变异体仍在时补上下方断言则失败（expected '⚠️ Round 6…' to match /"src0":228/——变异体发射 "src0":920）；撤销变异后全绿。建议修复：补上标记行断言，并补一个带先前基线的旧版增量（无 full-range 值）用例作同样断言。
— qwen3.8-max via Qwen Code /review (v0.21.13)
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/save-artifact.ts:229 — [probe] R1-1 (still standing): approachSignal validator absence/shape arms have no testpackages/cli/src/commands/review/compose-review.ts:1249 — [probe] R1-3 (still standing): nonConverged clause + COMMENT rendering path have no test pairingpackages/cli/src/commands/review/compose-review.ts:965 — [review] R1-4 (still standing): src0 survival through pr-context persistRecoveredLedger pinned by no testpackages/cli/src/commands/review/compose-review.ts:1960 — [probe] R1-5 (still standing): round-threshold boundary (>=) unpinned by any testpackages/cli/src/commands/review/compose-review.ts:2476 — [probe] R1-6 (still standing): Chinese half of the approach paragraph has no testpackages/cli/src/commands/review/compose-review.ts:1983 — [probe] R1-7 (still standing): growth-factor and floor boundaries unpinned by any testpackages/cli/src/commands/review/compose-review.test.ts:6409 — [review] R1-8 (still standing): threshold-fallback test pins neither semantics of its titlepackages/cli/src/config/settingsSchema.ts:781 — [review] R1-10 (still standing): review.approachRounds missing from the settings reference tabledocs/design/2026-08-17-review-approach-signal.md:39 — [review] R2-2 (still standing): design doc firing list says 'composed event'; the gate is baseEventpackages/cli/src/commands/review/compose-review.test.ts:6458 — [review] R3-1 (still standing): incremental rounds never assert marker baseline carry-forwardpackages/cli/src/commands/review/compose-review.ts:1968 — [review] load-bearing measurement expression hand-inlined twice (baseline stamp and growth numerator)packages/cli/src/commands/review/compose-review.ts:3560 — [probe] verdict-line rendering only half-paired with testspackages/core/src/skills/bundled/review/DESIGN.md:779 (+4 locations) — [probe] '#9325 landed' asserted of an unmerged PR; 'two PRs' miscounts threepackages/core/src/skills/bundled/review/SKILL.md:773 (+3 locations) — [review] SKILL.md + approachRounds descriptions describe composed verdict; the gate is baseEventpackages/cli/src/commands/review/compose-review.ts:2476 — [probe] zh '已增长 X 倍' states an increment; the interpolated value is a ratiopackages/cli/src/commands/review/lib/ledger.ts:75 — [probe] cross-account recovery seam drops the src0 baseline (foreign old-build winner)packages/cli/src/commands/review/compose-review.test.ts:6440 — [probe] baseline ratchet pinned only in the shrink direction; upward rewrite ships greenpackages/cli/src/config/settingsSchema.ts:787 — [review] setting description drops the 'source diff' growth qualifier
中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20310, 494, 1541, 1597, 3723, 529 passed。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 18 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Resolves the one conflict, in fetch-pr.test.ts: both sides added an import to the same block — `buildDiffPlan` here, `makeDiff` from the new lib/test-utils.js on main. Both are used, so both are kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
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.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20502, 494, 1578, 1597, 3747, 529 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:124 — [probe] APPROACH_ROUNDS_DEFAULT docstring misdescribes the round-boundary semantic the code implements
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20502, 494, 1578, 1597, 3747, 529 passed。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
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.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R13-1 prevLedgerFacts src0 round-gate gap — already recorded in the round-11 and round-12 deferral lists (compose-review.ts:1875, R12-9)
- R13-2 load-bearing measurement expression hand-inlined twice — already recorded in rounds 4/7/8/9/11/12 deferral lists (R12-2)
- R13-3 nonConverged false direction unpinned — already recorded in rounds 7-10 deferral lists (R12-4)
- R13-4 advisory paragraph carries no trim rank — already recorded in the round-10 and round-11 deferral lists (compose-review.ts:3454)
- R13-5 COMMENT-event render path has no fired-signal test — already recorded in rounds 2/3/7/8/9/10/12 deferral lists
- R13-6 approachSignal block detaches the deferredCount comment — already recorded in the round-8 and round-11 deferral lists (save-artifact.ts:226)
- R13-7 prevLedger helper hardcodes the side-file name — already recorded in the round-12 deferral list (compose-review.test.ts:10123)
- R13-8 zh round-cap corroborating clause untested — already recorded in rounds 8-10 deferral lists (R10-7)
- R13-10 review.approachRounds missing from the settings reference table — already reported inline in rounds 1-3 (comments 3795294579, 3798586864, 3799521882; lineage R1-10 / R10-1 / R12-5)
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20991, 495, 1685, 1653, 4164, 599 passed.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/pr-context-persist.test.ts:1043 — [review] src0 survival pinned on only two of the four persistRecoveredLedger write outcomespackages/cli/src/commands/review/compose-review.test.ts:10471 — [probe] APPROACH_ROUNDS_DEFAULT pinned only to (3, 5]; mutant default 4 ships greenpackages/cli/src/commands/review/compose-review.test.ts:10526 — [probe] APPROACH_GROWTH_FACTOR pinned only from above; mutant factor 2 ships greenpackages/cli/src/commands/review/compose-review.test.ts:10551 — [probe] LOW_SIGNAL_SRC_DIFF_LINES constrained only to [100, 300); mutant floor 299 ships greenpackages/cli/src/commands/review/compose-review.test.ts:10571 — [probe] round-cap stop 'never a trigger on its own' contract pinned by no fixturepackages/cli/src/commands/review/compose-review.ts:3539 — [probe] suggestions-only COMMENT baseEvent firing arm has no test
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 9 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 20991, 495, 1685, 1653, 4164, 599 passed。
收敛姿态下延后（第 13 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 21036, 495, 1685, 1654, 4190, 599 passed.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:3554 — [review] load-bearing measurement expression hand-inlined twice (baseline stamp and growth numerator)packages/cli/src/commands/review/compose-review.ts:4895 — [probe] COMMENT-event render path of a fired approach signal has no testpackages/cli/src/commands/review/fetch-pr.ts:1504 — [probe] fetch-pr's !scopedDelta supplier of fullSrcDiffLines (full-range and upToDate rounds) has no testpackages/cli/src/commands/review/compose-review.ts:1862 — [probe] prevLedgerFacts admits src0 without the round === 0 gate its sibling facts carrypackages/cli/src/commands/review/compose-review.test.ts:10411 — [probe] signal-side incremental measurement arm unpinned in both directions; two surviving mutantspackages/cli/src/commands/review/compose-review.test.ts:10591 — [probe] zh round-cap corroborating clause untestedpackages/cli/src/commands/review/compose-review.ts:3571 — [probe] nonConverged false direction and time-budget-stop discrimination unpinned; two surviving mutantspackages/cli/src/commands/review/compose-review.test.ts:10601 — [probe] zh growth figure (4.0 倍) and number framing asserted by no testpackages/cli/src/commands/review/compose-review.test.ts:10160 — [probe] approach paragraph's middle sentence asserted by no test in either languagepackages/cli/src/commands/review/pr-context-persist.test.ts:1044 — [probe] cross-account recovery seam drops the src0 baseline when the foreign winner carries none
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 21036, 495, 1685, 1654, 4190, 599 passed。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改：共 10 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
14 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- advisory paragraph carries no trim rank — already recorded in rounds 10/11/13/14 deferral lists (compose-review.ts)
- cross-account recovery seam drops the src0 baseline when the foreign winner carries none — already recorded in rounds 4/12/14 deferral lists (lib/ledger.ts / pr-context.ts)
- approachSignal block detaches the deferredCount comment — already recorded in rounds 8/11/13 deferral lists (save-artifact.ts)
- suggestions-only COMMENT baseEvent firing arm has no test — already recorded in the round-13 deferral list (compose-review.ts:3539)
- COMMENT-event render path of a fired signal has no test — already recorded in rounds 2/3/7/8/9/10/12/13/14 deferral lists
- nonConverged false direction and time-budget-stop discrimination unpinned — already recorded in rounds 7-14 deferral lists (compose-review.ts)
- threshold constants' silence side unpinned (growth-factor-2 and rounds-4 mutants ship green) — already recorded in the round-13 deferral list (compose-review.test.ts)
- doc claims force-push disarms the signal; recovery re-arms it — already recorded in rounds 2/3 deferral lists (design doc)
- zh round-cap corroborating clause untested — already recorded in rounds 8-10/13/14 deferral lists (R10-7 / R13-8)
- prevLedgerFacts admits src0 without the round === 0 gate — already recorded in rounds 11-14 deferral lists (R12-9 / R13-1)
- review.approachRounds missing from the settings reference table — already reported inline in rounds 1-3 (comments 3795294579, 3798586864, 3799521882; R1-10)
- src0 riding the marker on a capped round unpinned — already recorded in the round-11 deferral list (compose-review.test.ts)
- verdict-line numeric clause unpinned — already recorded in rounds 9/10/12/14 deferral lists (R12-10)
- zh growth figure (4.0 倍) asserted by no test — already recorded in rounds 9/10/12/14 deferral lists (R12-7)
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.test.ts:10212 — [probe] src0 marker carry-forward on APPROVE rounds pinned by no testpackages/cli/src/commands/review/compose-review.test.ts:10415 — [probe] 100-line floor's input (full-range vs incremental delta) unpinned by any test
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 14 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory。
收敛姿态下延后（第 15 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory.
Deferred under the convergence posture (round 16, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.ts:1862 — [probe] src0 recovered without the round gate its sibling facts carrypackages/cli/src/commands/review/save-artifact.ts:287 — [review] approachSignal block detaches the deferredCount comment from its codepackages/cli/src/commands/review/compose-review.ts:4895 — [probe] COMMENT-event render path of a fired signal has no testpackages/cli/src/commands/review/compose-review.ts:3576 — [probe] nonConverged false direction unpinned by any testpackages/cli/src/commands/review/compose-review.test.ts:10123 — [review] prevLedger helper hardcodes the side-file name independently of prNumberpackages/vscode-ide-companion/schemas/settings.schema.json:263 — [review] review.approachRounds missing from the settings reference tablepackages/cli/src/commands/review/compose-review.ts:4588 — [probe] advisory paragraph carries no trim rankpackages/cli/src/commands/review/compose-review.ts:3563 — [probe] foreign-winner recovery seam carries a stranger-planted src0 baselinepackages/cli/src/commands/review/compose-review.test.ts:10331 — [probe] threshold-fallback test's 0-half pins neither semantics
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory。
收敛姿态下延后（第 16 轮，非阻断）——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32681274809 🧠 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.
Partially reviewed — gaps disclosed.
15 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R17-1 load-bearing measurement expression hand-inlined twice (baseline stamp and growth numerator) — already recorded in rounds 4/7/8/9/11/12/14/16 deferral lists (compose-review.ts)
- R17-2 COMMENT-event render path of a fired approach signal has no test — already recorded in rounds 2/3/7/8/9/10/12/13/14/16 deferral lists (compose-review.ts:4895)
- R17-3 zh growth figure and number framing asserted by no test — already recorded in rounds 9/10/12/14 deferral lists (compose-review.test.ts)
- R17-4 APPROACH_ROUNDS_DEFAULT pinned only to (3, 5]; mutant default 4 ships green — already recorded in the round-13 deferral list (compose-review.test.ts:10471)
- R17-5 advisory paragraph carries no trim rank — already recorded in rounds 10/11/13/14/16 deferral lists (compose-review.ts)
- R17-6 approachSignal growth validator admits Infinity, persists null — already recorded in rounds 6/10/12 deferral lists (save-artifact.ts:244, R10-5)
- R17-7 nonConverged false direction and time-budget-stop discrimination unpinned — already recorded in rounds 7-14/16 deferral lists (compose-review.ts/test.ts)
- R17-8 prevLedger helper hardcodes the side-file name independently of prNumber — already recorded in rounds 12/16 deferral lists (compose-review.test.ts:10123)
- R17-9 prevLedgerFacts comment claims force-push/account-switch disarms; recovery re-arms — already recorded in rounds 2/3 deferral lists (design-doc instance of the same claim)
- R17-10 verdict-line numeric clause pinned by no test — already recorded in rounds 9/10/12/14/16 deferral lists (R12-10)
- R17-11 APPROACH_GROWTH_FACTOR pinned only from above; mutant factor 2 ships green — already recorded in the round-13 deferral list (compose-review.test.ts:10526)
- R17-12 prevLedgerFacts admits src0 without the round === 0 gate — already recorded in rounds 11/12/14/16 deferral lists (R12-9/R13-1, compose-review.ts:1875)
- R17-13 !scopedDelta supplier of fullSrcDiffLines has no test — already recorded in rounds 8/10/14/16 deferral lists (fetch-pr.ts:1515, R10-4)
- R17-14 review.approachRounds missing from the settings reference table — already reported inline in rounds 1-3 (comments 3795294579, 3798586864, 3799521882; R1-10)
- R17-15 legacy-incremental measurement arm unpinned with a baseline — already recorded in rounds 8/9 deferral lists (compose-review.test.ts:8508/8711)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 21226, 496, 1685, 1654, 4206, 626 passed.
Deferred under the convergence posture (round 17, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/compose-review.test.ts:10157 — [probe] COMMENT-branch standalone-paragraph placement unpinned (new facet of the COMMENT render path family)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 15 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 21226, 496, 1685, 1654, 4206, 626 passed。
收敛姿态下延后（第 17 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — no code changeThis round's feedback contained no new findings: all six automated review bodies are Diagnosis of the failed
|
|
🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
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.
10 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D18-1 load-bearing measurement expression hand-inlined twice (baseline stamp and growth numerator) — already recorded in rounds 4/7/8/9/11/12/14/16/17 deferral lists (compose-review.ts; R17-1)
- D18-2 COMMENT-event render path of a fired approach signal has no test — already recorded in rounds 2/3/7/8/9/10/12/13/14/16/17 deferral lists (compose-review.ts:4895; R17-2)
- D18-3 neutrality invariant cannot catch a promoting verdict-wiring mutant — already recorded in the round-12 deferral list (compose-review.test.ts:10194, 'neutrality invariant … verdict-wiring mutants ship green')
- D18-4 !scopedDelta supplier of fullSrcDiffLines has no test — already recorded in rounds 8/10/14/16/17 deferral lists (fetch-pr.ts:1515; R10-4/R17-13)
- D18-5 nonConverged false direction and corroborating-clause absence unpinned — already recorded in rounds 7-14/16/17 deferral lists (compose-review.test.ts; R12-4/R17-7)
- D18-6 advisory paragraph carries no trim rank — already recorded in rounds 10/11/13/14/16/17 deferral lists (compose-review.ts; R17-5)
- D18-7 zh round-cap corroborating clause untested — already recorded in rounds 8-10/13/14/17 deferral lists (compose-review.test.ts; R10-7/R13-8)
- D18-8 src0 recovered without the round gate its sibling facts carry — already recorded in rounds 11/12/14/16/17 deferral lists (compose-review.ts:1875; R12-9/R13-1/R17-12)
- D18-9 cross-account recovery seam drops the src0 baseline when the foreign winner carries none — already recorded in rounds 4/12/14/16 deferral lists (lib/ledger.ts / pr-context.ts; R12-11)
- D18-10 approachSignal block detaches the deferredCount comment from its code — already recorded in rounds 8/11/13/16/17 deferral lists (save-artifact.ts)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 23668, 21296, 496, 1688, 1654, 4226, 626 passed.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 10 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：lib/run-ledger.ts — no such file or directory; lib/ledger.test.ts — no such file or directory; lib/review-settings.test.ts — no such file or directory; 3266 passed — this review observed 23668, 21296, 496, 1688, 1654, 4226, 626 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action takenRound outcome: No changes. This round's feedback contains no new actionable findings, no failed checks, no unresolved inline threads, and no base conflict to resolve. Feedback triage
Informational notes (no code change implied)
中文说明Autofix 审查轮次 — 未采取任何行动**本轮结果：**无代码变更。本轮反馈中不包含新的可处理发现、没有失败的检查、没有未解决的行内讨论串，也没有需要解决的与 base 分支的冲突。 反馈分类
信息性说明（不涉及代码变更）
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Local deep-verification verdict: merge-ready — 17/17 scripted assertions passed for PR head 中文摘要结论：可合并。中心 A/B 已证明：base 不产生 approach signal；PR merge ref 在轮次、源码规模和增长倍数同时达标时产生 signal，并且不改变 Central claim and A/B resultThe same production
Tests and mutation proof
FindingsNo blocking or non-blocking product finding was reproduced in the exercised surface. Not covered
MethodologyMetadata was resolved on the host with |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken on PR #9340 this roundThis round's feedback contains no actionable items:
The comment's "Not covered" notes were reviewed and none is a defect claim against this PR:
No claim that current behavior is wrong was made, so there was nothing to reproduce, fix, decline, defer, or escalate. No code changes, no commits. 中文说明本轮对 PR #9340 不采取任何行动本轮反馈中没有任何可执行的事项：
该评论的 “Not covered”（未覆盖）部分已逐条审阅，没有一条是对本 PR 的缺陷指控：
没有任何“当前行为有错”的指控，因此没有需要复现、修复、拒绝、延期或升级的事项。未改动任何代码，未产生任何提交。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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: 148 passed · 0 failed · 148 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：148 通过 · 0 失败 · 148 总计 抖动门：✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at new head Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, mutation matrix, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 148 passed · 0 failed · 148 total 中文 — 判定：✅ 通过 · 可合入（agent 判定，follow-up 轮）本轮为 follow-up：上一轮在 head
Verification reportPR 9340 deep verification — follow-up round at head
|
| # | finding (first round) | severity | status at fd10856d |
|---|---|---|---|
| 1 | Firing body's deltas beyond the paragraph: the marker's src0 key and, on an otherwise-empty body, the attribution footer |
informational | stands — re-measured: after stripping paragraph + src0 key (any value) + footer, the firing/twin bodies are byte-identical (H1 F1 twin bodies identical mod paragraph+src0+footer PASS); the footer nuance reproduces (footerInFiring: true, footerInTwin: false). Same interaction every existing disclosure clause has; cannot move a verdict. Agree with the first round's assessment: no code change asked. |
| 2 | Plan numeric fields pass through Number() leniently (a string "920" coerces and fires) |
nit | stands — re-measured: H1 F2 probe fires on "srcDiffLines": "920". The untrusted side (src0 from posted markers) remains strictly positive-integer-validated at every read (H2 T4: 8 hostile shapes → silence). |
No corrections needed: nothing in the material I can see described the code inaccurately.
Delta since the previous round
The 8 commits added after the previously verified head 3c51d0ff are all Merge branch 'main' into feat/review-approach-signal — no feature commits. The base advanced 3b3818db → 717ad101 (main, incl. #9802 web-shell async submit). The effective HEAD^1..HEAD diff is the same 16-file feature (1221 insertions); every code fact the previous round asserted (R9-1 clamp, all guards, validator) was re-measured present at the new head. The review suite grew 3858 → 4953 tests with main's advance — numbers below are measured at the new head.
Central claim and A/B
Central claim (unchanged from round 1): when a non-Approve round reaches the round threshold (default 5, review.approachRounds) AND a recorded baseline exists AND the source diff is past the 100-line floor AND has grown ≥ 3x since the baseline, the composed review carries one advisory paragraph (EN, plus ZH for Han descriptions) and one verdict-line clause — disclosure-only: it never moves event, baseEvent, or caps, and never fires on an Approve.
Both arms drive the real composeReview (no module mocks) through its file seams, via harnesses parameterized by QWEN_TREE: plan JSON, the qwen-review-pr-8255-prev-ledger.json side file, the Step-4/5 + chunk + matrix transcripts, the production writeRoundCapStop writer, and QWEN_HOME-pointed settings. Witnesses: 01-ab-firing-head-arm.png, 02-ab-control-base-arm.png.
| cell (head arm unless noted) | oracle | head | base |
|---|---|---|---|
| incident fixture: round 6, src0 228 → 920, REQUEST_CHANGES | paragraph + verdict clause + marker | fires; exact text ⚠️ Round 6 … 4.0x … 228 → 920 source diff lines … Advisory only; clause reconsider the approach, not only the findings; marker "src0":228 |
silent; no src0 anywhere; same event |
| twin without baseline (same run) | event/baseEvent/cappedBy + body delta | signal null; event/baseEvent/cappedBy identical to firing cell; body identical mod paragraph+src0+footer | silent |
| APPROVE (same state) | paragraph absent | silent | silent |
| APPROVE downgraded to COMMENT | gate reads baseEvent |
silent | silent |
| silence matrix: early round (3, 20x), late no-growth (10, 1.1x), below floor (60, 12x), exactly at floor (100, 20x), 2.99x growth, round 4 | signal null | 6/6 silent | 6/6 silent |
| boundaries fire: 101 lines; growth exactly 3.0x; round exactly 5 | signal non-null | 3/3 fire | n/a |
| prevRound at the ledger cap (10 000) | round named consistently | signal/paragraph/marker all name 10000; 10001 appears nowhere |
silent |
| round-cap stop this round | corroborating clause | nonConverged: true; EN clause renders |
n/a |
| Han-character description | zh half | ⚠️ 第 6 轮 + zh corroborating clause render |
n/a |
| incremental: delta 138/full 920; legacy w/o full size; delta 350/full 120; baseline from full 950 | full-range comparison | 4/4 as designed | n/a |
| baseline lifecycle: measure when absent (340); carry unchanged on shrink (228) | marker "src0" |
2/2 | marker never carries src0 |
side-file type edges: src0 = −5, 2.5, "228", true |
dropped → silence + re-measure | 4/4 silent, re-measured 920 | n/a |
operator approachRounds via real loadSettings: 8→suppress, 4→fires at round 5, 0→built-in, workspace 999→ignored |
threshold honoured | 4/4 | 4/4 silent |
Totals: H1 head 50/50, H1 base 25/25. Control purity asserted before trusting the base arm: node_modules/@qwen-code/qwen-code-core realpath resolves into the head tree, whose base→head diff is docs-only (SKILL.md, DESIGN.md); package.json/package-lock.json untouched; the base worktree has no node_modules of its own and resolves to the same root as head.
Secondary claims
S1 — src0 baseline lifecycle (H2, head 26/26, base 11/11). Round-trip keeps src0; a 400-finding ledger truncates under the 8192-byte cap, drops sha and keeps src0 — the truncation ruling; the -- comment escape coexists with src0 (no literal -- inside the comment, title round-trips verbatim); parse drops every non-positive-integer src0 (0, −3, 2.5, "228", true, null, [228], {n:228}) to silence, keeps a large positive integer, and a garbled marker parses null without throwing; serializer never writes a non-positive-integer baseline; cross-version cells: base ignores a head-shaped marker, head reads base-shaped markers as absence.
S2 — review.approachRounds + artifact validator (H3, head 20/20, base 3/3). Through the real loadSettings(): 8→8, 1→1, 0/−2/2.5/"6"/true/null→absent. saveReviewArtifact over real files: a valid signal round-trips exactly; absent/null load as null; nine wrong shapes each refused with a message naming the offending field. Base arm: a composed file carrying approachSignal still saves, minus the unknown field (forward-compatible). Consumer census: approachSignal is read only by compose-review.ts (producer) and save-artifact.ts (validator) — no web-shell/acp-bridge/core consumer exists to trip on the field.
Mutation matrix — 7/7 killed, zero survivors (witness: 03-mutation-matrix-all-killed.png) — re-run at the new head in a scratch worktree at HEAD:
| mutant | red tests | killed at intended assertion |
|---|---|---|
| M0 control: default 5→6 | 1 | fires at exactly the round threshold (positive control) |
| M1: revert R9-1 clamp | 1 | names the round AT the ledger cap — expected { round: 10001, … } to match { round: 10000, … } |
M2: drop prevSrc0 > 0 |
4 | stays silent when the previous round recorded no baseline + the disclosure twin + two pre-existing body-trim tests |
M3: drop baseEvent !== 'APPROVE' |
2 | both APPROVE cells |
M4: >= 3x → > 3x |
1 | fires at exactly the growth factor |
M5: serializer stops writing src0 |
8 | all baseline tests incl. survives truncation where the anchor does not |
M6: floor > → >= |
1 | stays silent at exactly the source-diff floor |
| unmutated control | 0 | green: 858 passed, 1 skipped (6 changed test files) |
Every revert failed the intended behavioral assertion with expected-vs-actual values; the scratch tree ended git status CLEAN.
Targeted gates (witness: 04-targeted-gates.png)
- Head, full review suite (
packages/cli,vitest run src/commands/review/): 101 files, 4953 passed, 4 skipped, 0 failed. - run-ledger A/A:
lib/run-ledger.test.ts72/72 on head and 72/72 on base at uid 1000; the PR touches neitherrun-ledger.tsnor its test (0 diff lines). The failure the PR description names is the root-environment case (chmod does not bind as root); this container runs as uid 1000, so attribution stays clean and environmental. - Generated artifact: re-running
npm run generate:settings-schemareproduces the committedsettings.schema.jsonbyte-identical (sha2560d8cc6cf…on both sides) — theapproachRoundsentry was generated, not hand-edited.
Findings
Both carried findings stand as informational (status table above); neither blocks and neither asks for a code change. No new findings this round.
Not covered
- Per-commit attribution. The depth-2 merge-ref checkout reaches only 3 of the 20 commits named in the metadata (
git rev-list --count HEAD^1..HEAD^2= 1 at the shallow boundary). The aggregateHEAD^1..HEADdiff is what was verified; the delta since the previous round is merge-only (8Merge branch 'main'commits, no feature commits after3c51d0ff), and every commit's behavioral claim maps to a re-run matrix cell. fetch-pr's live network path (gh pr view, real diff fetch): no token in this container. ThefullSrcDiffLinescomputation is covered by the PR's ownfetch-pr.test.tsadditions (passing within the 4953-test suite) and consumed end-to-end by the H1 incremental cells; the wire itself was not driven.- Full base-arm suite. The base side is the control and its assertions ran (H1/H2/H3 base cells + run-ledger A/A); a full base suite was not re-run, matching the first round's scoping.
- The flakiness gate (5 identical rounds over changed test files) is run by the workflow, not by this round.
Methodology
CI verify container (node:22-bookworm), Node v22.23.2, uid 1000, no GH_TOKEN; working tree = refs/pull/9340/merge at depth 2, npm ci + npm run build pre-run. Harnesses (harness/h1-firing-matrix.ts, h2-ledger.ts, h3-settings-artifact.ts, run via tsx) import the real production modules from the tree named by QWEN_TREE (head tree vs tmp/base-tree worktree at HEAD^1) with no module interception; they drive the code through file/config seams: plan JSON, prev-ledger side files, agent transcripts, stop files written by the production writer, QWEN_HOME-pointed settings, and real artifact files for saveReviewArtifact. Mutation matrix (harness/mutate.py) applied one single-point mutant at a time in a scratch worktree at HEAD (tmp/mut-tree, wired with symlinked dist/ and nested node_modules), ran the six changed test files, recorded red tests verbatim, and restored (final status CLEAN). Raw logs in logs/, evidence images in evidence/.
Evidence images
— Qwen Code · sandboxed verification
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/cli/src/commands/review/compose-review.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/compose-review.test.ts
file packages/cli/src/commands/review/fetch-pr.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/fetch-pr.test.ts
file packages/cli/src/commands/review/lib/ledger.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/ledger.test.ts
file packages/cli/src/commands/review/lib/review-settings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/review-settings.test.ts
file packages/cli/src/commands/review/pr-context-persist.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/pr-context-persist.test.ts
file packages/cli/src/commands/review/save-artifact.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/save-artifact.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/compose-review.test.ts: PPPPP
packages/cli/src/commands/review/fetch-pr.test.ts: PPPPP
packages/cli/src/commands/review/lib/ledger.test.ts: PPPPP
packages/cli/src/commands/review/lib/review-settings.test.ts: PPPPP
packages/cli/src/commands/review/pr-context-persist.test.ts: PPPPP
packages/cli/src/commands/review/save-artifact.test.ts: PPPPP
verdict: pass
summary: 6 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/fetch-pr.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/review-settings.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/pr-context-persist.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/fetch-pr.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/review-settings.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/pr-context-persist.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/fetch-pr.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/review-settings.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/pr-context-persist.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/fetch-pr.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/review-settings.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/pr-context-persist.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/fetch-pr.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/review-settings.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/pr-context-persist.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/save-artifact.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.22.2. |
4 participants
What this PR does
Adds one advisory paragraph to a posted review, and one clause to the terminal verdict line, when a pull request has taken enough rounds and grown enough since the review first measured it that the shape of the change — rather than the current patch — looks like the open question.
It is disclosure only: no finding, no cap, no change to the event, and it never fires on an Approve.
Why it's needed
Every finding
/reviewemits is anchored to afile:lineinside the current diff. That is what a finding is — and it means a review can report where an approach leaks, but never that a different approach would retire all of the leaks at once.Measured on this repository. One change to
extractAndStripMetatook three attempts across two PRs:timeoutto the vm callAll 74 findings were individually correct. Each round found a real hole the previous patch did not cover — a getter deferring its work to the host, a serializer sharing a lexical scope with the literal it walked, a promise reaction under
microtaskMode, unbounded allocation. The review was not wrong at any point. It simply had no way to say the thing that ended it, and every one of those 74 findings went away with the mechanism.The signal was already there, filed as the wrong kind of thing.
did not converge within the reverse-audit round capwas emitted four times across the two PRs — as a coverage gap ("we did not finish looking") rather than as a conclusion about the change. Nothing was responsible for reading it as "stop patching".Why it is not a finding
Findings are what the autofix loop consumes. That loop patching each finding in turn is exactly the pattern being interrupted, so a finding here would be fixed rather than read. This addresses the human deciding what happens next, which is why it is a body paragraph and a verdict-line clause instead.
For the same reason it must not block: a pull request that is legitimately large and legitimately iterated should pay nothing for a false positive beyond one paragraph. It mirrors
lowSignal, the existing disclosure-only field, down to the "the event never moves on it" contract.Why the baseline is a baseline
Ledger.src0records the source-diff size at the first round that measured one and carries it forward unchanged. #9136 went 228 → 920 across six rounds — about 1.3x per round, which no per-round delta would ever notice, but 4.0x cumulatively. Re-measuring each round would also let a diff that later shrinks rewrite its own baseline and erase the growth already on record.It is the one marker field that survives truncation. The existing ruling withholds
shafrom a partial finding list because a truncated work list must not certify a commit range; a measurement of the diff certifies nothing, and stays true either way.What this deliberately does not do
Stated here rather than discovered in review, because two of these limit the fix to less than the incident that motivated it:
src0 > 0arm is what makes the absent case degrade to silence rather than to a false "no growth".Design doc:
docs/design/2026-08-17-review-approach-signal.md.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/— 3266 passed, 8 skipped, 1 failure.That failure is
run-ledger.test.ts > refuses to append over a ledger it could not read, and it is pre-existing and unrelated: I confirmed it fails identically on a clean tree with my changes stashed. It relies onchmodmaking a file unreadable, which does not bind when the suite runs as root. This PR does not touchlib/run-ledger.ts(verified to be CLI session bookkeeping, not review rounds).Coverage added:
compose-review.test.ts— a newapproach signalblock (10 cases): it fires on a round-6 4x-grown REQUEST_CHANGES and reaches both the body and the verdict line; the event, baseEvent and caps are byte-identical to the same run without it (the disclosure-only contract); it is silent on APPROVE with identical state, silent with no baseline, silent at round 4 with 10x growth, silent at round 9 with 1.2x growth, silent below the source-diff floor; the operator threshold is honoured and 0 falls back to the built-in; and the baseline carries forward unchanged when the diff shrinks, while a round with no prior baseline measures one.lib/ledger.test.ts— serialization, the explicit truncation ruling (a 400-finding ledger losessha, keepssrc0), rejection of every non-positive-integer baseline, and a round trip alongside the--comment escaping.lib/review-settings.test.ts— positive-integer re-validation ofapproachRoundsand its dialog membership.save-artifact.test.ts— the field round-trips throughvalidateVerdict; absent still loads.To see the shape it produces, the fixture in the first case is the real incident's numbers: round 6,
src0: 228,srcDiffLines: 920.Evidence (Before & After)
Before: a sixth round on a diff that has quadrupled reads exactly like a first round on a stable one — a list of line-anchored findings, and a coverage note saying the audit did not finish.
After, on the same run, one paragraph above the findings and one clause on the verdict line:
The Chinese half ships in the same
Biobject, so a bilingual body carries both.Tested on
Environment (optional)
Unit tests, Node 22.23.0 on Linux.
Risk & Scope
src0is untrusted body data, like every other marker field, and unlike a finding a bare number has nothing to re-assert it against. A forged small value fires the paragraph; a forged large one silences it. The blast radius is one advisory paragraph either way, and the field's doc comment says so.srcDiffLines, which excludes tests — consistent withlowSignaland the topology metric, but it means a PR ballooning purely in test code will not trip the growth arm.diffLineswould catch more and fire more often.coverageEntriespath is untouched, so its cap and its exact phrase (pinned by two tests) behave as before; no newReviewEvent, no newcappedByid, noFindingtype change and therefore no Web Shell change.Linked Issues
None. The incident behind it is #9097 / #9136 / #9325.
中文说明
这个 PR 做了什么
当一个 PR 已经经过足够多轮、并且自本审查首次测量以来 diff 增长了足够多时，在发布的审查正文里加一段提示、并在终端判定行上加一句：真正待决的可能是这次改动的整体形态，而不是当前这版补丁。
它只是披露：不产生 finding、不加 cap、不改变判定结论，并且在 Approve 上永不触发。
为什么需要
/review产出的每一条 finding 都锚定在当前 diff 的某个file:line上。finding 的定义本就如此——这意味着审查能指出一个方案在哪里漏了，却无法说明换一个方案就能一次性消除全部问题。在本仓库实测。对
extractAndStripMeta的一处改动，跨两个 PR 尝试了三次：timeout这 74 条 finding 每一条都是对的。每一轮都找出了上一版补丁没覆盖到的真实漏洞——推迟到宿主侧执行的 getter、与所遍历字面量共享词法作用域的序列化器、
microtaskMode下的 promise reaction、无界分配。审查在任何一个环节都没有出错。它只是没有办法说出那句终结问题的话；而最终，这 74 条 finding 随着机制一起消失了。信号其实早就存在，只是被归错了类。
did not converge within the reverse-audit round cap在两个 PR 中出现了四次——被当作覆盖缺口（"我们没看完"），而不是关于这次改动的结论。没有任何环节负责把它读成"别再打补丁了"。为什么不做成 finding
finding 正是 autofix 循环消费的东西。而"逐条修 finding"的循环，恰恰是这里要打断的模式——做成 finding 只会被修掉，而不会被读到。这段话是说给"接下来要做决定的人"听的，所以它是正文段落和判定行子句。
同理它绝不能阻断：一个确实很大、确实该多轮迭代的 PR，为一次误报付出的代价不应超过一个段落。它对标既有的
lowSignal披露字段，连"判定结论永不因它移动"这条契约都一致。为什么基线是"基线"
Ledger.src0记录首次测量到的源码 diff 规模，并原样向后传递。#9136 六轮从 228 走到 920——每轮约 1.3 倍，任何"逐轮增量"都发现不了，但累计是 4.0 倍。而且每轮重新测量，会让一个后来缩小的 diff 改写自己的基线，抹掉已经记录在案的增长。它是唯一在截断后仍然保留的 marker 字段。既有规则之所以在 finding 列表被截断时丢弃
sha，是因为部分工作列表不得认证一个 commit 范围；而"diff 有多大"这个测量本身不认证任何东西，无论如何都仍然为真。有意不做的事
在此直接写明，而不是留给评审去发现——其中两条使本修复的覆盖范围小于促成它的那次事件：
src0 > 0这一条正是让"缺失"退化为沉默、而不是退化为一个假的"没有增长"。设计文档：
docs/design/2026-08-17-review-approach-signal.md。审阅者验证方案
如何验证
cd packages/cli && npx vitest run src/commands/review/——3266 通过、8 跳过、1 失败。那个失败是
run-ledger.test.ts > refuses to append over a ledger it could not read，属于既有失败且与本改动无关：我把改动 stash 掉后在干净树上确认它同样失败。它依赖chmod让文件不可读，而测试以 root 运行时该前提不成立。本 PR 不触碰lib/run-ledger.ts（已确认它是 CLI 会话簿记，与审查轮次无关）。新增覆盖：
compose-review.test.ts——新增approach signal组（10 个用例）：在第 6 轮、4 倍增长的 REQUEST_CHANGES 上触发，且同时到达正文与判定行；event、baseEvent 与 caps 与"没有该信号的同一次运行"逐字节一致（披露-only 契约）；在状态相同的 APPROVE 上沉默、无基线时沉默、第 4 轮即便 10 倍增长仍沉默、第 9 轮仅 1.2 倍增长时沉默、低于源码 diff 下限时沉默；运维阈值生效且 0 回落到内置值；diff 缩小时基线原样保留，而无先前基线的一轮会测量出一个。lib/ledger.test.ts——序列化、截断规则的明确断言（400 条 finding 的 ledger 丢掉sha、保留src0）、对一切非正整数基线的拒绝，以及与--注释转义并存的往返。lib/review-settings.test.ts——approachRounds的正整数再校验及其对话框成员资格。save-artifact.test.ts——该字段可通过validateVerdict往返；缺失时仍可加载。想看它产出的形态：第一个用例的 fixture 就是真实事件的数字——第 6 轮、
src0: 228、srcDiffLines: 920。证据（前后对比）
修复前：一个 diff 已经翻了四倍的第六轮，读起来与一个稳定改动的第一轮完全一样——一串行锚定的 findings，外加一条"审计没跑完"的覆盖说明。
修复后，同一次运行，findings 之上多一段、判定行多一句：
中文半边与英文放在同一个
Bi对象里，因此双语正文两者兼有。测试环境
环境（可选）
单元测试，Linux 上的 Node 22.23.0。
风险与范围
src0与其他 marker 字段一样是不可信的正文数据，而且与 finding 不同，一个裸数字没有任何东西可供再断言。伪造一个小值会触发该段落，伪造一个大值会让它沉默。两种情况下影响半径都只是一个建议性段落，字段的文档注释里也写明了这一点。srcDiffLines衡量，不含测试代码——这与lowSignal和拓扑指标保持一致，但也意味着一个纯粹在测试代码上膨胀的 PR 不会触发增长条件。改用diffLines会抓到更多，也会更常触发。coverageEntries"路径完全未动，其 cap 与确切措辞（由两个测试固定）行为如常；不新增ReviewEvent、不新增cappedByid、不改Finding类型，因此也不涉及 Web Shell 改动。关联 Issue
无。促成它的事件是 #9097 / #9136 / #9325。