refactor(review): build the incremental scope from the PR's diff, not a check - #9267
Conversation
|
Re-run at the author's request — the PR has moved a long way since the first gate: nine autofix rounds addressing review Criticals, the stacked #9100 merged underneath it, and a retarget to Template looks good ✓ Problem: observed, with an unusually strong evidence trail. Six review rounds on #9100 each closed the containment oracle's reported entrances and the next round found new ones — count-less headers, deletion junctions, lossy UTF-8 collapsing distinct bytes onto U+FFFD, a deletion budget spent across hunks, content matched without position. The round-16 structural finding said plainly that the surface cannot be closed entrance by entrance. And the failure mode is all-or-nothing: one comment anchored on a line GitHub's PR diff does not display 422s the entire Create Review call, losing every finding of the round. Real reliability defect, not theoretical hardening. Direction: aligned — this is the repo's own review infrastructure, and the fix removes the unbounded acceptance surface instead of patching it a seventh time. It is subtractive where it matters: the Size: core paths are touched only through the bundled review skill ( Approach: the right shape — build the round's scope out of the PR's own diff (construction) instead of proving containment after the fact (verification), and fail closed to the full range on any ambiguity. Two non-blocking notes for the author: (1) the body still describes the intermediate design — "assembled out of the full capture's own hunks — the ones those ranges overlap" — while the final implementation narrows per FILE, emitting whole sections; the code comments argue convincingly that hunk positions are unstable across two independent Myers alignments, and the body's "−255/+29" line counts are likewise stale after nine autofix rounds — worth a one-line refresh. (2) Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明应作者要求重新运行——自首次 gate 以来 PR 变化很大：九轮 autofix 处理评审 Critical、堆叠的 #9100 已合入、改基到 模板完整 ✓ 问题：已观测到，且证据链异常充分。#9100 上六轮评审每轮都封堵了 containment oracle 当轮报告的入口，下一轮又发现新的——无计数头部、删除 junction、有损 UTF-8 把不同字节塌成 U+FFFD、跨 hunk 花掉的删除预算、只比内容不比位置。第 16 轮的结构性结论说得很直白：这个面无法靠逐个入口封堵。且失败模式是全有或全无：一条评论锚定到 PR diff 不显示的行，整个 Create Review 调用返回 422，本轮全部发现丢失。是真实的可靠性缺陷，不是理论性加固。 方向：对齐——这是仓库自身的评审基础设施，修复方式是移除无界的接受面，而不是第七次打补丁。在关键处是做减法： 规模：核心路径仅通过内置 review skill 触及（ 方案：形状正确——从 PR 自己的 diff 构造本轮范围（构造式），而不是事后证明包含关系（验证式），任何歧义都 fail-closed 回退全量范围。给作者两个非阻塞提示：(1) PR 正文仍描述中间设计——"由全量 capture 自己的 hunk 拼装——即被这些区间命中的那些"——而最终实现按文件收窄、整节输出；代码注释有说服力地论证了 hunk 位置在两个独立 Myers 对齐之间不稳定，且正文的"−255/+29"行数在九轮 autofix 后也已过时——建议顺手刷新一句。(2) 风险：无升级风险信号——未触及任何与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal before reading the diff was exactly this shape: stop proving containment after the fact and build the round's scope out of the PR's own diff — use
Null-safety at the No Criticals. Two non-blocking nits, already named in the gate comment: the PR body still describes the intermediate hunk-overlap design and pre-autofix line counts, and the lockfile's unexplained Files changed (14)
TestingCI on the reviewed head is fully green — no pending checks at review time:
The skips are the repo's CI design (macOS/Windows/integration run only in the merge queue; the skipped named jobs satisfy the required contexts), not a gap this PR caused. The ubuntu suite includes this PR's new integration battery. The author reports the review suite at 3205/3206 locally with the single failure being The central claim is behavioural — which bytes land in the round's published scope — and the PR's own suite pins it directly (per-line containment over the whole output on real three-commit histories), which CI runs green. The sandboxed lane is already engaged: the deep-verification run triggered by this 中文说明代码审查读 diff 之前我的独立方案正是这个形状：不再事后证明包含关系，而是从 PR 自己的 diff 构造本轮范围——
无 Critical。两个非阻塞小项，gate 评论里已点名：PR 正文仍描述中间的 hunk-overlap 设计与 autofix 前的行数；lockfile 对 lightningcss 平台二进制移除 测试评审时 head 上的 CI 全绿、无 pending。skip 是仓库 CI 设计（macOS/Windows/集成测试仅在 merge queue 运行；被跳过的具名 job 满足必需 check 上下文），不是本 PR 造成的缺口。ubuntu 套件包含本 PR 的新集成测试组。作者报告本地 review 套件 3205/3206，唯一失败 核心主张是行为性的——哪些字节进入本轮发布范围——而 PR 自己的套件直接钉住了它（真实三提交历史上对整个输出逐行断言包含关系），并在 CI 中通过。沙箱通道已启动：本次 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, subtractive redesign with a green suite and an invariant-based integration battery; the only nits are cosmetic (stale body text, unexplained lockfile metadata drift). Stepping back: this is what a good fix looks like after a problem has been understood properly. The first six rounds on #9100 tried to answer the containment question better; this PR removes the question. My independent proposal was the same shape, and everywhere the implementation diverges from the naive version of it — file-level instead of hunk-level narrowing, fatal UTF-8 instead of replacement-char scanning, the merge-base exit split — the divergence is justified by a defect class that actually surfaced, and the code says so where it happens. The failure direction is consistently safe: every refusal keeps the full range, which costs a wider review and never a wrong one, and the one judgment left (which sections the delta corroborates) fails closed toward over-inclusion. On the "is every change necessary" question: yes — even the merge-base probe rework, which looks adjacent at first glance, is load-bearing, because without the oracle the base-less arms needed a deterministic-vs-retryable classification they previously didn't have (one of them used to publish the delta unchecked). The 35 open Suggestion threads are closed-by-policy past round 5 and I don't see anything in them that rises to a blocker. If I were maintaining this in six months, the dense comment style is the one thing I'd have to adjust to — but the comments document real attack surfaces, so they earn their keep. Approving: all stages clean, CI settled green on the reviewed head with nothing pending, author is a maintainer, and the earlier dismissed approval correctly does not carry over to this head. 中文说明置信度：4/5 —— 干净的减法式设计，套件全绿，集成测试组以不变量为断言；唯一的小项是外观性的（正文过时、lockfile 元数据漂移未解释）。 退一步看：这是问题被真正理解之后应有的修复形态。#9100 的前六轮都在试图把包含性问题"答得更好"；这个 PR 把问题本身消除了。我的独立方案与之同形，而实现偏离朴素版本的所有地方——按文件而非按 hunk 收窄、严格 UTF-8 而非扫描替换字符、merge-base 退出码三分——都由实际出现过的缺陷族论证，且代码就写在发生处。失败方向始终安全：每次拒绝都保留全量范围，代价只是更宽的评审，从不是错误的评审；唯一剩下的判断（delta 佐证哪些节）也朝着过度包含 fail-closed。 关于"是否每处改动都必要"：是——即便是乍看相邻的 merge-base 探测改造也是承重的：没有 oracle 之后，base 缺失的分支需要它们此前没有的"确定性 vs 可重试"分类（其中一个分支过去会未经检查就发布 delta）。35 个未解决的 Suggestion 线程按第 5 轮后的策略关闭，其中没有我看到足以阻塞的项。若六个月后由我维护，唯一需要适应的是密集的注释风格——但这些注释记录的是真实攻击面，值回篇幅。 予以批准：各阶段干净，CI 在评审 head 上落定全绿且无 pending，作者是维护者，此前被 dismiss 的批准按规则不延续到当前 head。 — Qwen Code · qwen3.8-max Reviewed at |
| // cannot be built out of what GitHub displays, so it is not built: | ||
| // the round keeps the full range, which on a base-free PR is itself | ||
| // nothing — the same state such a round reaches without `--since`. | ||
| demote('capture-failed'); |
There was a problem hiding this comment.
[Suggestion] R1-5: This merged fullText === null branch reports capture-failed even when no capture threw — when mergeBaseSha === null (unrelated history, or a deleted base branch on a fresh CI clone), fullText is null without any capture ever attempted, contradicting the enum's own doc ("a capture that threw"). Pre-PR this arm emitted containment-unverified (the deterministic class). — Concrete cost: the bundled review skill's retry policy classes capture-failed as retryable infrastructure ("the component that failed … is re-run by the re-run"), while the same paragraph's partition-failed carve-out documents this exact null-mergeBaseSha / baseFetchFailed: false state as NOT retryable — but the carve-out is scoped to partition-failed. A clean round posts the ledger sha, the base branch is then deleted, and the next round with the recovered anchor lands here: the orchestrator re-runs fetch-pr --since <same sha> once over a condition that deterministically reproduces, spending a worktree rebuild + capture, and the reason misnames the cause for any operator reading the report. Bounded (the re-run is base-free → planless → fail-closed → the ledger withholds the sha → the chain converges), hence Suggestion.
Suggested fix: keep a distinct reason for the base-free arm (or only emit capture-failed when a capture actually threw), or extend SKILL.md's capture-failed retry rule with the same mergeBaseSha === null carve-out the partition-failed rule already carries.
中文说明
这个合并后的 fullText === null 分支即使在没有任何 capture 抛出时也报告 capture-failed——当 mergeBaseSha === null（不相关历史，或全新 CI 克隆上被删除的 base 分支）时，fullText 为 null 而根本没有尝试过任何 capture，与该枚举自身的文档（「抛了异常的 capture」）相矛盾。本 PR 之前这个分支发出的是 containment-unverified（确定性类别）。— 具体代价：bundled review skill 的重试策略把 capture-failed 归为可重试的基础设施类（「失败的组件……会被重跑再跑一次」），而同一段落里 partition-failed 的豁免条款恰好把这个 mergeBaseSha 为 null 且 baseFetchFailed: false 的状态记载为不可重试——但豁免只限定于 partition-failed。一轮干净的评审把 ledger sha 发布出去，随后 base 分支被删除，下一轮带着恢复出的锚点落到这里：编排者会对一个确定性复现的状态重跑一次 fetch-pr --since <同一 sha>，白白花掉一次 worktree 重建 + capture；而且对任何读报告的运维者来说，reason 都报错了指向的原因。影响有界（重跑是无 base → 无 plan → fail-closed → ledger 不盖 sha → 链条收敛），故为 Suggestion。
建议修复：给无 base 分支保留一个独立的 reason（或只在确实有 capture 抛出时才发 capture-failed），或者把 partition-failed 规则已有的 mergeBaseSha === null 豁免同样加进 SKILL.md 的 capture-failed 重试规则。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round. Verified real on the current code: when mergeBaseSha === null the merged fullText === null arm reports capture-failed although no capture was ever attempted, and SKILL.md's retry carve-out for this exact state is scoped to partition-failed. This round's batch was filled by the four Criticals plus the test pinning they demanded (~8 findings), so this one rides into the next round rather than being dropped — the fix direction (a distinct reason for the base-free arm, or emitting capture-failed only when a capture actually threw, plus aligning SKILL.md's retry rule) is recorded in the round summary.
中文说明
推迟到下一轮。 已在当前代码上核实为真：当 mergeBaseSha === null 时，合并后的 fullText === null 分支在根本没有尝试过任何 capture 的情况下也报告 capture-failed，而 SKILL.md 中针对这一确切状态的重试豁免只限定于 partition-failed。本轮批次已被四个 Critical 及其所需的测试钉住填满（约 8 条），因此该条进入下一轮而非被丢弃——修复方向（给无 base 分支保留独立 reason，或只在确有 capture 抛出时才发 capture-failed，并对齐 SKILL.md 的重试规则）已记录在本轮总结中。
| const headerEnd = | ||
| firstHunk === undefined ? file.diffEnd : firstHunk.diffStart - 1; |
There was a problem hiding this comment.
[Suggestion] R1-6: The hunk-less-section logic on BOTH sides of narrowToDelta is entirely untested — v8 coverage measured over the combined suites: the firstHunk === undefined ternary arm here shows branch hits [0] vs [14]; no fixture anywhere carries a hunkless section (all four integration scenarios are content-only edits), and there is no narrow-diff.test.ts. The deleted oracle battery exercised mode-only, binary, and rename shapes in both directions; the replacement tests exercise none. This untested path is exactly where R1-2's live bug sits. — Concrete cost: two one-line mutants survive the entire suite green — collapsing the ternary to firstHunk.diffStart - 1 (kills hunkless emission) and dropping firstHunk !== undefined from the guard below (silently drops hunkless sections) — so which outcome a mode/rename/binary round gets is unpinned, and a regression there is silent.
Suggested fix: add cases — a chmod-after-anchor that base..head also shows (expect the hunkless section emitted verbatim); a reverted chmod where the delta is mode-only and the full section carries content hunks (expect null or omission, per the chosen semantics); a pure rename likewise.
中文说明
narrowToDelta 两侧的无 hunk section 逻辑完全没有被测——对合并套件测得的 v8 覆盖率显示：这里 firstHunk === undefined 三元的分支命中为 [0] 对 [14]；任何 fixture 都不含无 hunk 的 section（四个集成场景全是纯内容编辑），也不存在 narrow-diff.test.ts。被删除的 oracle 测试组曾双向覆盖 mode-only、二进制与重命名形状；替代测试一个都没有覆盖。这条未测路径正是 R1-2 活缺陷所在的位置。— 具体代价：两个单行 mutant 在整个套件全绿的情况下存活——把三元压缩成 firstHunk.diffStart - 1（杀死无 hunk 发射），以及从下方守卫中去掉 firstHunk !== undefined（静默丢弃无 hunk section）——因此 mode/重命名/二进制轮次会得到哪种结果完全没有被钉住，回退发生时也不会有任何红色信号。
建议修复：补充用例——锚点后 chmod 且 base..head 也显示它（期望无 hunk section 被原样发射）；delta 只有 mode 而 full section 带内容 hunk 的「撤销 chmod」（按选定语义期望 null 或被省略）；纯重命名同理。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| } | ||
| }); | ||
|
|
||
| it('falls back rather than scoping when nothing of the PR diff changed', () => { |
There was a problem hiding this comment.
[Suggestion] R1-7: No integration scenario ever asserts a null result against real-git captures, and this test — the one titled for the fallback — never reaches it: its own comment admits untracked-elsewhere.txt IS in both captures (so narrowing succeeds), and its assertions sit inside if (narrowed !== null) guards, as do scenario 2's expect(narrowed ?? '').not.toContain('X1') and the undo test's bonus assertions. Coverage measured: the if (out.length === 0) return null arm fires 0 times in the integration-suite-only run; its single combined-run hit comes from fetch-pr.test.ts hand-written fixtures — the artifact class this file's header declares untrustworthy. — Concrete cost: a regression breaking overlap detection for deletion-wrapped delta hunks (the exact shape the old oracle lost to six times) makes real undo rounds return null and silently fall back to full-range review on every --since undo round — a whole-feature degradation that passes every scenario vacuously through its null guards.
Suggested fix: add a scenario whose delta genuinely overlaps no full hunk (e.g. base edits lines 5 and 12; anchor additionally inserts X1–X3; head reverts the insertion keeping the edits) and assert expect(narrowToDelta(full, delta)).toBeNull() unconditionally; drop or rename the misleading title.
中文说明
没有任何集成场景在真实 git capture 上断言过 null 结果，而这个以 fallback 为标题的测试自己也到不了 fallback：它的注释承认 untracked-elsewhere.txt 在两次 capture 中都在（因此收窄成功），其断言都坐在 if (narrowed !== null) 守卫后面，场景 2 的 expect(narrowed ?? '').not.toContain('X1') 与 undo 测试的附加断言也是如此。覆盖率实测：if (out.length === 0) return null 分支在仅跑集成套件时命中 0 次；合并跑中唯一的一次命中来自 fetch-pr.test.ts 的手写 fixture——正是本文件头部声明「不可信」的那类产物。— 具体代价：一个破坏「删除包裹的 delta hunk」重叠检测的回退（正是老 oracle 六次败北的形状）会让真实 undo 轮次返回 null，并在每个 --since undo 轮次静默回退到全量评审——整个特性的退化却能穿过所有场景的 null 守卫空转通过。
建议修复：增加一个 delta 确实不与任何 full hunk 重叠的场景（例如 base 编辑第 5 与第 12 行；锚点额外插入 X1–X3；head 撤销该插入但保留编辑），无条件断言 expect(narrowToDelta(full, delta)).toBeNull()；删除或改掉这个误导性标题。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| describe('narrowToDelta on real-git captures', () => { | ||
| it('keeps only the PR hunks the anchor round did not already cover', () => { |
There was a problem hiding this comment.
[Suggestion] R1-8: The deleted integration suite's control case — "accepts a delta whose deletion the PR diff performs too" (the control that kept the old rule from being "refuse every deletion") — has no successor: no added test pins the ACCEPTANCE direction for a deletion-carrying delta. All four scenarios use additions or edits for the delta's overlapping content; scenario 2's assertions hold whether narrowToDelta returns null or not; the fetch-pr.test.ts fixtures are additions-only. — Concrete cost (measured by the verifier's mutant run): skipping newCount === 0 hunks in the touched-range loop flipped a deletion probe to expected null not to be null, while narrow-diff.integration.test.ts + fetch-pr.test.ts passed 90/90 under the same mutant — zero red signal. Current behaviour is correct (the probe passes 2/2 on unmodified code); the gap is that nothing pins it. Outcome of a regression is safe (wider review, never wrong), hence Suggestion.
Suggested fix: port the old control shape — base has lines, anchor edits elsewhere, head deletes lines that stood at the base; assert narrowed is non-null, contains the full capture's deletion hunk, and satisfies everyLineIsDisplayed.
中文说明
被删除的集成套件里的对照用例——「接受 PR diff 同样执行了该删除的 delta」（正是它防止老规则退化成「拒绝一切删除」）——没有继任者：没有任何新增测试钉住「携带删除的 delta」的接受方向。四个场景的 delta 重叠内容全是新增或编辑；场景 2 的断言无论 narrowToDelta 返回 null 与否都成立；fetch-pr.test.ts 的 fixture 只有新增。— 具体代价（验证者 mutant 实测）：在 touched 区间循环中跳过 newCount === 0 的 hunk，会让删除探针翻成 expected null not to be null，而同一 mutant 下 narrow-diff.integration.test.ts + fetch-pr.test.ts 依然 90/90 全绿——零红色信号。当前行为是正确的（探针在未改动代码上 2/2 通过）；缺口在于没有任何东西钉住它。回退的后果是安全的（更宽的评审、不会错），故为 Suggestion。
建议修复：移植老的对照形状——base 有若干行，锚点在别处编辑，head 删除 base 时代就存在的行；断言 narrowed 非 null、包含 full capture 的删除 hunk，且满足 everyLineIsDisplayed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| /** True when the FINAL published diff is the incremental delta. */ | ||
| let scopedDelta = false; |
There was a problem hiding this comment.
[Suggestion] R1-9: The mechanism changed but the old mechanism's vocabulary remains across the code this diff touched or depends on: this doc still says the published diff is "the incremental delta" (it is now the narrowed scope); fetch-pr.ts:615 still justifies the full-range capture as "the containment oracle the clamp cannot be"; fetch-pr.test.ts keeps the test name 'refuses to scope when the containment oracle was LOST, not absent' (:698) and comments like "the reason says the oracle could not rule" (:785) and "hunks the oracle just proved" (:688) — asserting a reason-level distinction the merged arm deliberately removed — plus stale references at :663, :701, :742, :802, :1123, :1405; agent-prompt.test.ts:2457 still constructs a refused-ruling fixture with reason: 'hunks-outside-pr-diff', a value the producer can no longer emit (it compiles only because agent-prompt.ts types incremental as unknown); pr-context.test.ts:1323 cites it as a live example. — Concrete cost: a maintainer grepping the old reasons finds apparent live uses and concludes they are still contract; a future change special-casing live reasons in the buildRoleBrief guard passes the dead-reason fixture unchallenged; and the comments assert a model (oracle, delta-published) the code now contradicts — the same mismatch class behind R1-3.
Suggested fix: sweep the vocabulary in the same change — replace the fixture literal with a current reason, update the example comment, rename/reword the two tests, and fix fetch-pr.ts:615 and this doc to the narrowing model.
中文说明
机制换了，但旧机制的词汇仍散布在本 diff 触及或依赖的代码里：这条文档注释仍说发布的 diff 是「incremental delta」（现在是收窄后的范围）；fetch-pr.ts:615 仍把全量 capture 正当化为「the containment oracle the clamp cannot be」；fetch-pr.test.ts 保留着测试名 'refuses to scope when the containment oracle was LOST, not absent'（:698）以及「the reason says the oracle could not rule」（:785）、「hunks the oracle just proved」（:688）这类注释——断言着合并分支刻意移除的 reason 级区分——另有 :663、:701、:742、:802、:1123、:1405 等过期引用；agent-prompt.test.ts:2457 仍用 reason: 'hunks-outside-pr-diff' 构造拒绝裁决的 fixture，而这个值生产方已不可能发出（它能编译仅仅因为 agent-prompt.ts 把 incremental 定为 unknown）；pr-context.test.ts:1323 把它当作活着的例子引用。— 具体代价：维护者 grep 旧 reason 会找到看似仍在使用的引用，误以为它们仍是契约；未来若在 buildRoleBrief 的守卫里对活 reason 做特判，这个死 reason fixture 会毫无阻力地通过；注释断言的模型（oracle、delta 即发布）与代码现状相矛盾——正是 R1-3 背后同一类不一致。
建议修复：在同一次改动里清扫词汇——把 fixture 字面量换成当前的 reason，更新示例注释，重命名/改写这两个测试，并把 fetch-pr.ts:615 与这条文档注释改为收窄模型的表述。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round as one coherent sweep. The vocabulary cleanup spans fetch-pr.ts:615, several fetch-pr.test.ts names/comments, the dead hunks-outside-pr-diff fixture in agent-prompt.test.ts:2457, and pr-context.test.ts:1323 — a mechanical batch that deserves its own round after the four Criticals landed. This round rewrote only the doc/comment sentences the R1-3 fix itself made false: the incremental field doc (no longer "cover since..fetchedSha"), the diffBase doc, and both weld comments (fetch-pr.ts and agent-prompt.ts).
中文说明
推迟到下一轮，作为一次完整的清扫。 词汇清扫横跨 fetch-pr.ts:615、fetch-pr.test.ts 的若干测试名/注释、agent-prompt.test.ts:2457 的死 reason fixture hunks-outside-pr-diff，以及 pr-context.test.ts:1323——这是一个机械性批次，适合在四个 Critical 落地后单独成一轮。本轮只改写了 R1-3 修复本身使其失真的文档/注释句：incremental 字段文档（不再写「cover since..fetchedSha」）、diffBase 文档，以及两处焊点注释（fetch-pr.ts 与 agent-prompt.ts）。
4c86f15 to
4cdbb9f
Compare
7a598b1 to
20909de
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
Rebased onto #9100's new head ( The rebase surfaced a real correctness point for this PR, so it is worth stating rather than burying in a resolution. #9100 now carries So That is the same failure class the oracle kept being defeated by, and it is worth noting how it lands under the new design: as a refusal to narrow that costs a wider review, rather than as an unsound acceptance that costs the round. New test: Suite 3116/3117, typecheck clean under 中文说明在 #9100 分支 rebase 到 main 之后，本 PR 已 rebase 到其新 head（ 这次 rebase 暴露出一个与本 PR 相关的真实正确性问题，值得明说而不是埋在解决方案里。 #9100 现在带有 因此 这与 oracle 屡次被击败的是同一类失效，而它在新设计下的落点值得记录：表现为拒绝收窄、代价是评审面更宽，而不是不成立的接受、代价是整轮丢失。 新增测试： 套件 3116/3117， |
… a check The containment oracle proved, after the fact, that a separately captured `anchor..head` carried no hunk the PR's own `base..head` diff lacked — because a comment anchored on a line GitHub does not display answers 422 and takes the whole all-or-nothing Create Review call with it. That proof was a hand-written match over two rendered unified diffs, and six review rounds each closed the reported entrances while the next found new ones: count-less headers, deletion junctions, lossy UTF-8 decodes that collapse distinct bytes onto U+FFFD, a budget spent across hunks, content matched without position. Every one was the same shape — something the delta carried that the PR's diff did not display, arriving through a gap in the match. The surface is unbounded because it is a match over arbitrary content. So the scope is no longer checked against the PR's diff; it is built from it. The delta is read only for the post-image ranges it touched, and the published text is assembled out of the full capture's own hunks. Every line the review sees is a line GitHub displays, by construction rather than by proof. The oracle goes, and with it the two refusal reasons that existed to report its verdicts: `hunks-outside-pr-diff` and `containment-unverified`. One reason replaces them, and it names a fact rather than a failed proof — `nothing-to-narrow`, when the PR's diff has no hunk left in the range that changed since the anchor. That is the "undo per feedback" round, where the commits since the anchor put lines back the way the base had them: there is genuinely nothing there to re-review, and the round keeps the full range, which is the review it would have done anyway. The invariant is asserted directly against real-git captures rather than argued per shape: every line of the narrowed output must appear in the full capture, checked over the whole output on each scenario, including the undo-per-feedback history that defeated the oracle six times. A mutant that assembles from the delta's bytes instead fails three of the four. Net effect on the tree: -255 lines of production code and the ~765-line battery that existed to pin it, against +29 and a four-case integration suite.
#9100 has landed, so the paragraphs this would have conflicted with are settled and the deferred half of this PR can go in. SKILL.md's recovery taxonomy still enumerated `hunks-outside-pr-diff` and `containment-unverified` — reasons the report can no longer carry, since the oracle that produced them is gone. It names `nothing-to-narrow` now, and says what actually produces it: an "undo per feedback" revert, which puts lines back the way the base had them so the PR's diff no longer shows that region, and a capture whose bytes do not survive a UTF-8 round trip. The retry classification moves with it. The old sentence said "the containment reasons re-rule identically"; the new one says the narrowing re-narrows identically, and why — the same two captures select the same hunks, and a capture that failed a round trip fails it again. Both remain deterministic for the same sha and outside the retryable set. Pinned in SKILL.test.ts beside its siblings: moving the reason into the retryable set fails that test rather than shipping green.
20909de to
a4339fe
Compare
|
#9100 merged as Rebased with The deferred half of this PR is in, now that the paragraphs it touches have settled. SKILL.md's recovery taxonomy no longer enumerates Two stragglers in tests still named the retired reason and now name the new one. Suite 3490/3490 against merged main — including @qwen-code /triage 中文说明#9100 已以 使用 本 PR 此前推迟的那一半现在补上了，因为它要改的段落已经稳定。SKILL.md 的恢复分类不再列举 另有两处测试仍写着已退役的理由，现已改为新理由。 针对已合入的 main，套件 3490/3490 全通过——包括 |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-dev-bot
added
the
autofix/takeover
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6a": trace selectHunkProbes / hunkProbeInputs pairing in test-efficacy.ts end-to-end to confirm whether undo-delta hunks become actual probe candidates under an …; "agent 6b": verify whether compose-review's anchor resolution discards every test-efficacy finding anchored outside the published diff's hunks (bounds the worst case of fin….
Test Plan (not a blocker): 1 passed — this review observed 1466, 1571, 494, 3654, 504 passed; 4 passed — this review observed 1466, 1571, 494, 3654, 504 passed.
[Critical] R1-1 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:99 (re-verified at a4339fe; the rebase changed nothing): touched is keyed by the delta capture's paths and looked up by the full capture's paths; whenever git's rename detection resolves differently across the two ranges, the section is silently skipped — a change both the delta performed and the PR's diff displays drops out of the published scope while the round reports incremental.effective: true, and the ledger then certifies head so no later round re-covers it. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-2 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:110 (re-verified by probe at a4339fe): a hunk-less delta section (a since-anchor mode change, pure rename, or binary replacement) projects to an EMPTY range list; when the full section for the same path HAS hunks, the guard drops the path's whole section — including this round's mode/rename change — while the round reports effective: true and the ledger certifies head. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-3 still stands at packages/cli/src/commands/review/fetch-pr.ts:694 (re-verified at a4339fe; agent-prompt.ts weld and test-efficacy.ts hunkProbeInputs re-read): incremental.diffBase is still welded to the ANCHOR sha while the published scope is assembled from base..head hunks, so Agent 7's test-efficacy probe recomputes anchor..HEAD — a range no longer equal to the published scope — and can spend its probe budget on, and report survivors about, changes absent from the round's diff. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-4 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:113 (threshold re-measured at a4339fe: largest OK spread 125,258 lines, first throwing 125,259): out.push(...spread) throws a RangeError when a selected hunk exceeds ~125k lines, and the fetch-pr.ts call site sits outside every try/catch — the whole round crashes with no report instead of degrading like every sibling failure path. Open round-1 thread; re-asserted because it still blocks.
中文说明
未探索到全部深度（达到工具调用预算）："agent 6a"：trace selectHunkProbes / hunkProbeInputs pairing in test-efficacy.ts end-to-end to confirm whether undo-delta hunks become actual probe candidates under an …；"agent 6b"：verify whether compose-review's anchor resolution discards every test-efficacy finding anchored outside the published diff's hunks (bounds the worst case of fin…。
Test Plan（非阻断）：1 passed — this review observed 1466, 1571, 494, 3654, 504 passed; 4 passed — this review observed 1466, 1571, 494, 3654, 504 passed。
[Critical] R1-1 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:99 (re-verified at a4339fe; the rebase changed nothing): touched is keyed by the delta capture's paths and looked up by the full capture's paths; whenever git's rename detection resolves differently across the two ranges, the section is silently skipped — a change both the delta performed and the PR's diff displays drops out of the published scope while the round reports incremental.effective: true, and the ledger then certifies head so no later round re-covers it. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-2 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:110 (re-verified by probe at a4339fe): a hunk-less delta section (a since-anchor mode change, pure rename, or binary replacement) projects to an EMPTY range list; when the full section for the same path HAS hunks, the guard drops the path's whole section — including this round's mode/rename change — while the round reports effective: true and the ledger certifies head. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-3 still stands at packages/cli/src/commands/review/fetch-pr.ts:694 (re-verified at a4339fe; agent-prompt.ts weld and test-efficacy.ts hunkProbeInputs re-read): incremental.diffBase is still welded to the ANCHOR sha while the published scope is assembled from base..head hunks, so Agent 7's test-efficacy probe recomputes anchor..HEAD — a range no longer equal to the published scope — and can spend its probe budget on, and report survivors about, changes absent from the round's diff. Open round-1 thread; re-asserted because it still blocks.
[Critical] R1-4 still stands at packages/cli/src/commands/review/lib/narrow-diff.ts:113 (threshold re-measured at a4339fe: largest OK spread 125,258 lines, first throwing 125,259): out.push(...spread) throws a RangeError when a selected hunk exceeds ~125k lines, and the fetch-pr.ts call site sits outside every try/catch — the whole round crashes with no report instead of degrading like every sibling failure path. Open round-1 thread; re-asserted because it still blocks.
— qwen3.8-max via Qwen Code /review (v0.21.12)
| /** path -> the post-image ranges the delta touched. */ | ||
| const touched = new Map<string, Array<[number, number]>>(); |
There was a problem hiding this comment.
[Suggestion] R2-1: The UTF-8 round-trip guard covers only the FULL capture, but the delta arrives already lossily decoded (deltaBytes.toString('utf8') at the call site in fetch-pr.ts). A full-capture path legitimately containing U+FFFD (valid UTF-8 bytes EF BF BD — the guard passes) can collide in the touched map with a delta path holding an invalid byte at the same position, so the delta's ranges are attributed to the wrong full section and hunks of a file that did NOT change since the anchor are published as the round's scope under incremental.effective: true. Soundness holds — every emitted line still comes from the full capture — so the consequence is over-inclusion (re-reviewing already-certified hunks; a scope that does not match what changed), not an un-displayed line. — Failure scenario: an undo round on a repo where two byte-distinct filenames decode to the same string (a legit U+FFFD name in the full capture, an invalid-byte name in the delta, net-zero since the anchor): the published scope becomes the unchanged file's hunk while effective: true.
Witness (probe against unmodified code, real-git captures with the pinned flags):
FULL capture: the legit file's hunk only
DELTA: the invalid-byte file's undo hunk, whose lossily-decoded path equals the legit path
E2E RESULT: NON-NULL (scoped) — published scope is the entire legit-file hunk, a file unchanged since the anchor
FLIP CHECK: adding a delta-side invalid-byte guard flips the same captures to null (safe full-range fallback); reverting restores NON-NULL
Suggested fix — take the delta as bytes too (the caller already holds deltaBytes) and apply the identical round-trip check before decoding, returning null on failure so the caller keeps the full range:
export function narrowToDelta(
fullBytes: Buffer,
deltaBytes: Buffer,
): Buffer | null {
// ... the existing full-side round-trip check ...
const deltaText = deltaBytes.toString('utf8');
if (!Buffer.from(deltaText, 'utf8').equals(deltaBytes)) return null;中文说明
UTF-8 往返守卫只检查了 full capture，但 delta 在调用方就已经被有损解码（fetch-pr.ts 调用点处的 deltaBytes.toString('utf8')）。full capture 中合法包含 U+FFFD 的路径（合法 UTF-8 字节 EF BF BD——能通过守卫）可能与 delta 中同一位置含非法字节的路径在 touched map 中发生键碰撞，于是 delta 的区间被归属到错误的 full section，一个自锚点以来并未改变的文件的 hunk 会被当作本轮范围发布，且报告 incremental.effective: true。健全性不受影响——每一行输出仍来自 full capture——因此后果是「多包含」（重审已认证的 hunk；范围与实际改动不符），而非发布未显示的行。— 失败场景：在两个字节不同、但解码后字符串相同的文件名存在的仓库上执行 undo 轮次（full capture 中的合法 U+FFFD 文件名，delta 中的非法字节文件名，自锚点以来净变化为零）：发布的范围变成了未改变文件的 hunk，同时 effective: true。
见证（在未改动代码上、用固定旗标的真实 git capture 执行的探针）：FULL capture 只含合法文件的 hunk；DELTA 为非法字节文件的 undo hunk，其有损解码后的路径与合法路径相同；E2E 结果 NON-NULL（已收窄）——发布范围是整个合法文件的 hunk，该文件自锚点以来并未改变；翻转验证：加入 delta 侧非法字节守卫后同样的 capture 翻转为 null（安全回退全量范围），还原后恢复 NON-NULL。
建议修复——把 delta 也以字节形式传入（调用方已持有 deltaBytes），在解码前套用完全相同的往返检查，失败时返回 null，让调用方保留全量范围（见上方代码）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round. The delta-side invalid-byte guard wants the (fullBytes, deltaBytes) signature change (the caller already holds deltaBytes); this round reworked the same function boundary for R2-4 (fatal decode on the full side), and landing both boundary edits together is the coherent change. The over-inclusion shape you measured (legit U+FFFD path colliding with a lossily-decoded delta path under effective: true) is confirmed soundness-safe — every emitted line still comes from the full capture — and stays open until the guard lands.
中文说明
推迟到下一轮。 delta 侧的非法字节守卫需要 (fullBytes, deltaBytes) 的签名变更（调用方已持有 deltaBytes）；本轮已为 R2-4 重做了同一函数边界（full 侧的 fatal 解码），两处边界编辑一起落地才是完整的改动。你实测的过度包含形状（合法 U+FFFD 路径与有损解码后的 delta 路径在 effective: true 下发生键碰撞）已确认健全性无虞——每一行输出仍来自 full capture——在该守卫落地前保持 open。
| const selected = file.hunks.filter((h) => | ||
| ranges.some((r) => overlaps([h.newStart, h.newEnd], r)), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-2: The narrowed scope selects WHOLE hunks of the full capture. Under the pinned --unified=3 flag git merges change regions ≤6 lines apart, so one post-anchor line edited inside a merged hunk that spans a file's earlier-round edits republishes the entire merged hunk — every already-reviewed line in it — instead of the few lines that changed. Pre-PR the delta itself was published, so this inflation is new; chunk count, agent fan-out and token cost all scale with the published diff, so iterative rounds on such files silently lose most of the incremental saving. All output lines remain displayed — this is an efficiency consequence of the by-construction design, not a correctness defect. — Failure scenario: round 1 makes 40 scattered edits across a 240-line file; round 2 edits one line between them.
Witness (probe on a real repo):
BASE: base..head = 1 file-spanning hunk, 285 lines; anchor..head = 1 hunk, 14 lines (@@ -118,7 +118,7 @@)
PR: narrowToDelta published 285 lines — inflation factor 20.4x; all 40 round-1 lines republished; 0 lines outside the PR diff
Suggested fix: if whole-hunk granularity is not a deliberate must, emit only the delta-overlapping sub-range of each selected hunk (bytes still come from the full capture; recompute the @@ counts for the emitted body); otherwise surface the narrowed-vs-delta size in the report so the skill can see when a "narrowed" round is effectively a full re-review.
中文说明
收窄后的范围选择的是 full capture 的完整 hunk。在固定的 --unified=3 旗标下，git 会把相距 ≤6 行的改动区域合并，因此在一个早前轮次编辑过的文件中、一个跨越那些编辑的已合并 hunk 内部再改一行，就会把整个合并 hunk（其中每一行都已审过）重新发布，而不是只发布真正改变的那几行。本 PR 之前发布的是 delta 本身，所以这种膨胀是新增的；chunk 数、agent 扇出与 token 成本都随发布的 diff 规模增长，此类文件上的迭代轮次会悄悄丢掉大部分增量收益。所有输出行仍是 GitHub 会显示的行——这是「按构造」设计的效率代价，不是正确性缺陷。— 失败场景：第 1 轮在一个 240 行文件中做 40 处分散编辑；第 2 轮在它们之间改一行。
见证（在真实仓库上执行的探针）：BASE：base..head = 1 个跨文件 hunk、285 行；anchor..head = 1 个 hunk、14 行（@@ -118,7 +118,7 @@）；PR：narrowToDelta 发布了 285 行——膨胀系数 20.4 倍；第 1 轮的全部 40 行被重新发布；0 行超出 PR diff。
建议修复：若「整 hunk 粒度」不是刻意坚持的必需项，则只发布每个被选中 hunk 与 delta 重叠的子区间（字节仍来自 full capture；为发布的正文重算 @@ 计数）；否则在报告中暴露「收窄后 vs delta」的大小对比，让 skill 能看到一次「收窄」轮实际上等同于全量重审。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Declined (direction conflicts with this PR's byte-fidelity contract). The suggested sub-range emission recomputes the @@ counts — i.e. rewrites hunk headers, emitting bytes git never produced. That is exactly the shape this PR refuses on principle for diffSha256 (stated in the PR's rebase note: re-encoding "would write bytes git never produced and give diffSha256 a value naming a file nobody captured"); a narrowed artifact whose headers are synthetic would name a file nobody captured in precisely the same way. The alternative — a narrowed-vs-delta size field in the report — is a report-contract addition for a maintainer to want; happy to take it as follow-up feedback. The inflation itself is bounded as measured: all output lines remain displayed (soundness holds), and the worst case re-reviews already-reviewed lines of a merged hunk — a token cost, never an un-displayed line.
中文说明
拒绝（方向与本 PR 的字节保真契约冲突）。 建议的子区间发射需要重算 @@ 计数——即改写 hunk 头部，发出 git 从未产生过的字节。这正是本 PR 基于原则为 diffSha256 拒绝的形状（PR 的 rebase 说明写明：重编码「会写出 git 从未产生过的字节，并给 diffSha256 一个指向无人抓取过的文件的值」）；一个头部为合成物的收窄产物，会以完全相同的方式指向一个无人抓取过的文件。替代方案——在报告中暴露收窄后 vs delta 的大小——属于报告契约的新增字段，应由维护者决定是否需要；若有此需求，愿作为后续反馈处理。膨胀本身如实测所述是有界的：所有输出行仍被显示（健全性成立），最坏情况是重审合并 hunk 中已审过的行——是 token 成本，绝非未显示的行。
| const fullText = fullBytes.toString('utf8'); | ||
| if (!Buffer.from(fullText, 'utf8').equals(fullBytes)) return null; |
There was a problem hiding this comment.
[Suggestion] R2-4: Buffer.from(fullText, 'utf8').equals(fullBytes) allocates a fresh full-size buffer — a further full-size copy beside fullBytes and the two live decode strings (the caller's fullText and this function's own decode) — on every incremental round, adding ~N to the process peak where the deleted oracle's U+FFFD scan over already-decoded strings allocated nothing. This path is engineered for large long-lived PRs (production maxBuffer is 512 MiB), so the extra ~N at the peak is a concrete cost on the named workload. — Failure scenario: a --since round on a PR with a ~210 MB base..head capture holds four full-size representations simultaneously at the comparison point, raising heap-pressure/OOM risk before any narrowing work begins.
Witness (end-to-end A/B, real narrowToDelta, 210 MB realistic capture, identical 2 MB output both arms):
PRISTINE: maxRSSAfterMB 1785 / 1788 / 1789
TEXTDECODER-FIX: maxRSSAfterMB 1581 / 1576
the patch flips the process peak by ≈207 MB ≈ N, the exact size of the re-encoded buffer
| const fullText = fullBytes.toString('utf8'); | |
| if (!Buffer.from(fullText, 'utf8').equals(fullBytes)) return null; | |
| const fullText = fullBytes.toString('utf8'); | |
| try { | |
| new TextDecoder('utf-8', { fatal: true }).decode(fullBytes); | |
| } catch { | |
| return null; | |
| } |
(Equivalence with the round-trip check verified over 10 fixtures including legitimate U+FFFD, overlong and truncated sequences. Note: the two fullText.split('\n') passes on this path are each larger still, if memory here is ever the target.)
中文说明
Buffer.from(fullText, 'utf8').equals(fullBytes) 会分配一个全新的全尺寸 buffer——在 fullBytes 与两份存活的解码字符串（调用方的 fullText 与本函数自己的解码）之外又多一份全尺寸副本——每个增量轮次都会如此，使进程峰值内存增加约 N；而被删除的 oracle 是在已解码字符串上扫描 U+FFFD，不做任何分配。这条路径正是为大型长命 PR 设计的（生产 maxBuffer 为 512 MiB），因此峰值上这份额外的 ~N 在该工作负载上是实打实的成本。— 失败场景：对 base..head capture 约 210 MB 的 PR 执行 --since 轮次时，比较点处同时持有四份全尺寸表示，在任何收窄工作开始之前就抬高了堆压力 / OOM 风险。
见证（端到端 A/B，真实 narrowToDelta，210 MB 真实感 capture，两臂输出相同 2 MB）：未改动 maxRSSAfterMB 1785/1788/1789；TextDecoder 修复后 1581/1576；该补丁使进程峰值翻转约 207 MB ≈ N，恰为重编码 buffer 的大小。
上方 suggestion 块给出一键修复（用 TextDecoder('utf-8', { fatal: true }) 拒绝无效序列，不再物化重编码副本；已在包含合法 U+FFFD、超长与截断序列的 10 个 fixture 上验证与往返检查等价）。另外：若将来真要针对这条路径做内存优化，两处 fullText.split('\n') 各自的开销比这更大。
— qwen3.8-max via Qwen Code /review (v0.21.12)
… and huge-hunk shapes - Refuse to narrow when a delta path does not cross into the full capture's keys: git's rename detection can resolve differently across the two ranges, and the unmatched section is a displayed change that would silently drop from the published scope while the round still certified head. The round keeps the full range instead. - Emit a full section whole when the delta touches it without hunks: a since-anchor mode change, pure rename, or binary replacement lives in the section header, and the old guard dropped the whole section while reporting `effective: true`. - Weld `incremental.diffBase` to the merge base, not the anchor: the published hunks are byte-identical hunks of `mergeBase..head`, so Agent 7's test-efficacy probe must recompute that range; the anchor range can carry undo hunks the PR's diff does not display at all. - Assemble the narrowed text without spreading selected hunks into a single `push`: a hunk past the ~125k-line argument ceiling crashed the whole fetch-pr round with a RangeError instead of degrading. - Validate the full capture with a fatal UTF-8 decode instead of re-encoding a full-size copy to compare, removing ~N of peak memory on the large long-lived PR workload. Pinned by real-git integration scenarios for each shape, including the deletion-acceptance control and an unconditional null fallback; the producer→consumer weld test now asserts the merge base end to end. Each new test was verified to fail against the pre-round code.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9267Commit: All four Criticals were reproduced first with probes driving the unmodified code through real-git captures (the witness shapes from the findings), then fixed, and every test pinning a fix was verified to fail against the pre-round source before the round's commit. Feedback dispositionsCritical (all fixed)
Suggestions
Not addressed this round (replies posted on their threads)
VerificationCommands actually run this round (all from the repository root unless noted):
Conflict: none ( 中文说明Autofix 评审轮次 — PR #9267提交： 四个 Critical 全部先行复现：用探针驱动未改动代码、以真实 git capture 跑出发现在 feedback 中描述的见证形状，然后修复；并且每个钉住修复的测试都已验证其在轮次前源码上会失败，之后才创建本轮提交。 反馈处置Critical（全部修复）
Suggestion
本轮未处理（已在各自线程回复）
验证本轮实际执行的命令（除注明外均在仓库根目录）：
冲突：无（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally.
Not explored to full depth (tool budget reached): chunk 8: executing SKILL.test.ts under vitest — neither the worktree nor the parent checkout has node_modules , and a full monorepo install was out of proportion for …; "agent reverse-audit (round 3)": none — all checks above ran to completion within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 8：executing SKILL.test.ts under vitest — neither the worktree nor the parent checkout has node_modules , and a full monorepo install was out of proportion for …；"agent reverse-audit (round 3)"：none — all checks above ran to completion within budget.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| } else if ( | ||
| mergeBaseSha === null || | ||
| fullBytes === null || | ||
| fullText === null | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-5 (re-report — deferred to this round in round 1, and independently re-detected by three review agents this round): this merged arm folds a deterministic cause — no common ancestor (mergeBaseSha === null with baseFetchFailed === false) — into capture-failed, which the union doc defines as "a capture that threw" and SKILL.md classifies as retryable infrastructure. Nothing threw and nothing was attempted (fullBytes is null by construction when no base resolved). Pre-PR this arm emitted containment-unverified (the deterministic class). SKILL.md's own null-mergeBaseSha carve-out already reads the discriminating field pair — but applies it only to partition-failed.
Concrete cost: a clean round posts the ledger sha; the base branch is then deleted or force-pushed to unrelated history; the next --since round with the recovered anchor lands here and the orchestrator re-runs fetch-pr with the same sha for an "infrastructure" refusal that deterministically reproduces — one wasted worktree rebuild + capture per round — and the reason misnames the cause for any operator reading the report.
Witness (flow probe through the real handler with base-free mocks): incremental {effective: false, reason: 'capture-failed'}, mergeBaseSha: null, baseFetchFailed: false, full-range capture attempted: false; restoring the pre-PR split arm reports containment-unverified for the identical input.
Suggested fix: split the arm so capture-failed keeps only the true infrastructure causes (baseFetchFailed ? 'capture-failed' : <deterministic reason> for the base-free shape), or extend SKILL.md's null-mergeBaseSha no-retry carve-out to cover capture-failed.
中文说明
[Suggestion] R1-5（重新报告——第 1 轮已被推迟到本轮处理，本轮又被三个评审 agent 独立重新发现）：这个合并后的分支把一个确定性原因——无公共祖先（mergeBaseSha === null 且 baseFetchFailed === false）——折进了 capture-failed；而 union 文档把该 reason 定义为「抛了异常的 capture」，SKILL.md 把它归为可重试的基础设施类。这里没有任何 capture 抛异常，甚至根本没有尝试过 capture（无 base 时 fullBytes 按构造就是 null）。本 PR 之前该分支发出的是 containment-unverified（确定性类别）。SKILL.md 自己的 null-mergeBaseSha 豁免条款已经在读这对判别字段——但只应用于 partition-failed。
具体代价：一轮干净评审发布了 ledger sha；随后 base 分支被删除或被强推为不相关历史；下一个带恢复锚点的 --since 轮次落到这里，编排者会对一个确定性复现的「基础设施」拒绝以同一 sha 重跑 fetch-pr——每轮白白花掉一次 worktree 重建 + capture——而且对任何读报告的运维者来说，reason 都报错了指向的原因。
见证（用无 base 的 mock 走真实 handler 的流程探针）：incremental {effective: false, reason: 'capture-failed'}、mergeBaseSha: null、baseFetchFailed: false、full-range capture attempted: false；恢复 PR 前的拆分分支后，同样的输入报告 containment-unverified。
建议修复：拆分该分支，让 capture-failed 只保留真正的基础设施原因（无 base 形状用 baseFetchFailed ? 'capture-failed' : <确定性 reason>），或把 SKILL.md 中 null-mergeBaseSha 的不可重试豁免扩展到 capture-failed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round — this round's batch was bounded (~8 findings, Criticals first: the rename-key divergence and the Windows exec-bit failures). The finding is accepted as reported; the planned fix is the suggested split arm: mergeBaseSha === null && !baseFetchFailed reports a deterministic reason (the pre-PR deterministic class, re-expressed in the new taxonomy) while capture-failed keeps only the true infrastructure causes, with the union doc, SKILL bullet, and handler tests updated alongside.
中文说明
推迟到下一轮——本轮批次有上限（约 8 项发现，Critical 优先：rename 键分歧与 Windows 可执行位失败）。该发现按报告原样接受；计划中的修复即建议的拆分分支：mergeBaseSha === null && !baseFetchFailed 报告一个确定性 reason（以新分类重新表达 PR 前的确定性类别），capture-failed 只保留真正的基础设施原因，并同步更新 union 文档、SKILL 条目与 handler 测试。
| effective: false, | ||
| reason: 'containment-unverified', | ||
| reason: 'capture-failed', |
There was a problem hiding this comment.
[Suggestion] R1-9 (re-report): the deferred "one coherent sweep" of stale vocabulary is still incomplete — 21 occurrences verified at this commit, in two species. (a) References to the deleted containment oracle — sharpest here: the comment two lines above (~791) says "the reason says the oracle could not rule" directly beside the capture-failed assertion this diff introduces; also the test titles at ~728 and ~1129, comments at ~669/694/748/808/1411, and the guard comment at fetch-pr.ts ~642. (b) Contract texts asserting the deleted delta-as-scope semantics, now false because the published diff is the merge-base range narrowed to the touched hunks and diffBase is always the merge base — fetch-pr.ts:187/261-268 (behind-merge-base justified as a 422 guard, impossible under build-from-full), :327 ("a deleted base branch must not cost a valid anchor its scope" — contradicted by the no-base demote arm this diff adds), :536-542/652, :752, :822 (status line "scoped to .."), fetch-pr.test.ts:573-574/1306-1307/1351-1354, and SKILL.md:155 "rebuilds the worktree, diff and chunk plan scoped to the delta" in the very paragraph this diff rewrites. The deletion also orphans references in untouched files: lib/diff-flags.ts:26 and lib/diff-flags.test.ts:21-23/52.
Failure scenario: a maintainer triaging a future failure of this test reads "the oracle could not rule" beside a capture-failed assertion and chases a deleted component; fetch-pr.ts:327 promises a base-deleted anchor keeps its scope while the code demotes it — two texts send a debugger in opposite directions at exactly the state where they disagree.
Suggested fix: finish the sweep in one pass — rename the two oracle-titled tests around the current mechanism, rewrite each stale comment to the build-from-full model, update the status line and the SKILL.md recovery phrase, and point diff-flags at the surviving consumers (parseDiff / the narrow stage).
中文说明
[Suggestion] R1-9（重新报告）：被推迟的「一次性彻底清扫」仍未完成——在本提交上核实了 21 处过期词汇，分两类。(a) 对已删除 containment oracle 的引用——最尖锐的就在本处上方：约 :791 的注释写着「the reason says the oracle could not rule」，紧挨着本 diff 引入的 capture-failed 断言；另有约 :728 与 :1129 的测试标题，约 :669/694/748/808/1411 的注释，以及 fetch-pr.ts 约 :642 的守卫注释。(b) 断言已删除「delta 即范围」语义的契约文本——现在它们是错的，因为发布的 diff 是「收窄到锚点以来改动」的 merge-base 区间，且 diffBase 恒为 merge base——fetch-pr.ts:187/261-268（把 behind-merge-base 正当化为 422 守卫，在 build-from-full 下不可能发生）、:327（「被删除的 base 分支不应让有效锚点失去范围」——与本 diff 新增的无 base 降级分支矛盾）、:536-542/652、:752、:822（状态行「scoped to ..」）、fetch-pr.test.ts:573-574/1306-1307/1351-1354，以及 SKILL.md:155 中「rebuilds the worktree, diff and chunk plan scoped to the delta」——就在本 diff 重写的同一段落里。删除还使未被改动文件中的引用变成孤儿：lib/diff-flags.ts:26 与 lib/diff-flags.test.ts:21-23/52。
失败场景：维护者排查该测试的未来失败时，读到 capture-failed 断言旁的「oracle could not rule」，去追一个已删除的组件；fetch-pr.ts:327 承诺被删 base 的锚点保留范围，代码却把它降级——两处文本恰好在它们相互矛盾的状态点上把调试者送往相反方向。
建议修复：一次扫完——按当前机制重命名两个以 oracle 为标题的测试，把每处过期注释改写为 build-from-full 模型，更新状态行与 SKILL.md 恢复段落措辞，并让 diff-flags 指向现存的消费者（parseDiff / 收窄步骤）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round — this round's batch was bounded (~8 findings, Criticals first). The sweep is accepted as scoped and will land as one coherent pass: the remaining oracle references renamed around the current mechanism, each stale contract text rewritten to the build-from-full model, the status line and SKILL.md recovery phrase updated, and diff-flags pointed at the surviving consumers (parseDiff / the narrow stage).
中文说明
推迟到下一轮——本轮批次有上限（约 8 项发现，Critical 优先）。该清扫按其界定范围接受，并将以一次连贯的扫掠落地：把剩余的 oracle 引用按当前机制重命名，把每处过期契约文本改写为 build-from-full 模型，更新状态行与 SKILL.md 恢复段落措辞，并让 diff-flags 指向现存的消费者（parseDiff / 收窄步骤）。
| let fullText: string; | ||
| try { | ||
| fullText = new TextDecoder('utf-8', { | ||
| fatal: true, | ||
| ignoreBOM: true, | ||
| }).decode(fullBytes); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-1 (re-report of the round-2 ledger entry, deferred to this round; independently rediscovered by a reverse-audit agent this round): decode fidelity is enforced one-sidedly. The full capture is fatal-decoded here, but the delta arrives pre-decoded lossily (deltaBytes.toString('utf8') at the call site in fetch-pr.ts), and the delta's decoded paths drive the guard and the join. A delta path holding an invalid byte decodes onto U+FFFD; if the full capture legitimately contains a different path with a literal U+FFFD character (valid UTF-8 bytes EF BF BD — passes the fatal decode), fullPaths.has(p) passes for the wrong file and the lossy file's ranges select the unrelated file's hunks — the published scope carries hunks of a file that did NOT change since the anchor, under effective: true. Soundness holds (every emitted line is still displayed), so the consequence is over-inclusion; the module docstring's "Null covers … a capture that did not decode" overclaims — only the full capture is checked.
Witness (probe at the reviewed commit, real git captures): FULL carries the legit U+FFFD file's hunk only; DELTA carries the invalid-byte file's undo hunk whose lossily-decoded path equals the legit path; narrowToDelta NON-NULL — the unchanged file's hunk is published. FLIP (delta-side invalid-byte refusal): null — safe full-range fallback — with the 12 integration tests green under the guard.
Suggested fix: take the delta as a Buffer and fatal-decode it inside narrowToDelta symmetrically with the full side (return null on failure, keeping the full range), or correct the docstring to say the delta decode is lossy and the path guard absorbs it.
中文说明
[Suggestion] R2-1（第 2 轮 ledger 条目的重新报告，当时被推迟到本轮；本轮又被一个反向审计 agent 独立重新发现）：解码保真度的执行是单边的。full capture 在这里做致命（fatal）解码，但 delta 在调用方（fetch-pr.ts 调用点）就已按 deltaBytes.toString('utf8') 有损解码，而 delta 解码后的路径驱动着守卫与合并。含非法字节的 delta 路径会解码成 U+FFFD；若 full capture 合法地包含另一个带字面 U+FFFD 字符的路径（合法 UTF-8 字节 EF BF BD——能通过 fatal 解码），fullPaths.has(p) 就会在错误的文件上放行，有损文件的区间会选中无关文件的 hunk——发布的范围携带了一个自锚点以来并未改变的文件的 hunk，且报告 effective: true。健全性不受影响（每一行输出仍然被显示），所以后果是「多包含」；模块 docstring 的「Null covers … a capture that did not decode」说过头了——只有 full capture 被检查。
见证（在被审提交上、用真实 git capture 执行探针）：FULL 只含合法 U+FFFD 文件的 hunk；DELTA 携带非法字节文件的 undo hunk，其有损解码后的路径与合法路径相同；narrowToDelta 非 NULL——未改变文件的 hunk 被发布。翻转（加入 delta 侧非法字节拒绝）：null——安全回退全量范围——且 12 个集成测试在该守卫下保持全绿。
建议修复：把 delta 也以 Buffer 形式传入，在 narrowToDelta 内与 full 侧对称地做 fatal 解码（失败返回 null，保留全量范围）；或修正 docstring，说明 delta 解码是有损的、由路径守卫兜底。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (narrowed !== null) { | ||
| expect(everyLineIsDisplayed(narrowed, full)).toBe(true); | ||
| expect(narrowed).not.toContain('q.ts'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-1 (1 of 2): every substantive assertion in 'narrows to a post-anchor file the PR diff also carries' sits behind this null guard, so a regression returning null for new-file delta sections passes with zero assertions executed — and this is the only test whose delta carries a new-file section. The mirror anti-pattern was explicitly outlawed by the no-overlap test's comment ("Unconditional: the fallback must not hide behind a null guard"); this is the same anti-pattern on the success side.
Failure scenario: a parseDiff change miskeying a --- /dev/null section (or a re-introduced conservative refusal) returns null for this shape; the test executes no assertion and passes; every fetch-pr round whose anchor added a new file then silently re-reviews the entire PR range — the exact over-inclusion this refactor exists to remove — with no red test.
Suggested fix: hoist expect(narrowed).not.toBeNull(); before the assertions and drop the guard (the scenario is constructed to narrow).
中文说明
[Suggestion] R3-1（2 处之 1）：『narrows to a post-anchor file the PR diff also carries』中所有实质断言都坐在这个 null 守卫后面，因此一个让新文件 delta section 返回 null 的回退会在零断言执行的情况下通过——而这是套件中唯一 delta 携带新文件 section 的测试。其镜像反模式已被 no-overlap 测试的注释明令禁止（「Unconditional: the fallback must not hide behind a null guard」）；这是成功侧的同一反模式。
失败场景：一次让 --- /dev/null section 键错的 parseDiff 改动（或重新引入的保守拒绝）会让该形状返回 null；本测试不执行任何断言就通过；此后每个锚点新增了文件的 fetch-pr 轮次都会静默重审整个 PR 范围——正是本次重构要消除的过度包含——且没有任何红色测试。
建议修复：把 expect(narrowed).not.toBeNull(); 提到断言之前并去掉守卫（该场景按构造就会收窄成功）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const headerEnd = | ||
| firstHunk === undefined ? file.diffEnd : firstHunk.diffStart - 1; |
There was a problem hiding this comment.
[Suggestion] R3-2 (residual of round-1 finding R1-6, whose delta-side arm is now tested): the emission branch where the FULL capture's section is hunk-less while the delta carries content hunks at that path (ranges.length > 0 with firstHunk === undefined) is reachable but exercised by no test — the firstHunk === undefined side of this ternary and the firstHunk !== undefined && conjunct in the guard above it are both untested.
Failure scenario: round 1 chmods AND edits m.sh; round 2 reverts only the content. base..head nets to a mode-only section (no hunks); anchor..head carries a content-reversion hunk. The mutant dropping the firstHunk !== undefined conjunct narrows to nothing, demotes to nothing-to-narrow, and republishes the full range — a different scope and a reason label the SKILL treats as deterministic-and-never-retried — shipping green through all 460 tests. Both outcomes are displayed-line-safe (hence Suggestion), but the label feeds recovery routing.
Witness (probe at the reviewed commit): FULL sections [m.sh hunks:0], DELTA sections [m.sh hunks:1]; narrowToDelta null? false; narrowed carries the mode line; everyLineIsDisplayed true.
Suggested fix: add the mirror scenario — round 1 chmod + one-line edit; round 2 reverts only the content; assert non-null, the new mode header lines, and everyLineIsDisplayed.
中文说明
[Suggestion] R3-2（第 1 轮发现 R1-6 的残余，其 delta 侧分支现已被测试覆盖）：当 FULL capture 的 section 无 hunk、而 delta 在该路径携带内容 hunk 时（ranges.length > 0 且 firstHunk === undefined）的发射分支可达但无任何测试覆盖——这个三元的 firstHunk === undefined 一侧与上方守卫中的 firstHunk !== undefined && 联结项都未被测试。
失败场景：第 1 轮对 m.sh 既 chmod 又编辑；第 2 轮只回退内容。base..head 归约为仅 mode 的 section（无 hunk）；anchor..head 携带内容回退 hunk。去掉 firstHunk !== undefined 联结项的 mutant 会收窄为空、降级为 nothing-to-narrow 并重新发布全量范围——不同的范围、以及被 SKILL 视为「确定性且永不重试」的 reason 标签——却能在全部 460 个测试全绿的情况下通过。两种结果都是「显示行安全」的（故为 Suggestion），但该标签会影响恢复路由。
见证（在被审提交上执行探针）：FULL sections [m.sh hunks:0]，DELTA sections [m.sh hunks:1]；narrowToDelta null？否；narrowed 携带 mode 行；everyLineIsDisplayed 为 true。
建议修复：补上镜像场景——第 1 轮 chmod + 单行编辑；第 2 轮只回退内容；断言非 null、包含 new mode 头部行、且 everyLineIsDisplayed 为 true。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // there left to re-review. Keeping the full range costs a wider review | ||
| // and never a wrong one. | ||
| demote('nothing-to-narrow'); |
There was a problem hiding this comment.
[Suggestion] R3-4: narrowToDelta returns null for four distinct shapes (undo no-overlap, fatal UTF-8 decode refusal, unparseable/empty delta, and the new fail-closed rename-key guard), all demoted to this one reason — but the reason's contract texts (the union doc added in this PR, this arm comment, and SKILL.md's bullet) describe only the undo shape, and assert a fact that is FALSE for the rename-key shape: the PR's diff DOES display the change there (the rename-fallback integration test shows the full capture carrying it) — narrowing only refused because the two captures keyed it differently. narrow-diff.ts's own docstring enumerates all four shapes honestly; the reason's contract texts do not.
Failure scenario: a --since round whose narrowing refuses on rename-key divergence reports "nothing left in the changed range" while reviewing the full range; the label collision makes R1-1's class invisible in round reports — nobody can count how often rename divergence fires in production, and its structural fix cannot be measured by the reason field. Routing happens to stay correct (all four shapes deterministic; SKILL routes the reason as never-retry), hence Suggestion.
Suggested fix: extend the union doc, this comment, and SKILL.md's bullet to enumerate all four null causes (including the rename-key refusal), or give the fail-closed refusal its own deterministic reason.
中文说明
[Suggestion] R3-4：narrowToDelta 会因四种不同形状返回 null（undo 无重叠、UTF-8 fatal 解码拒绝、不可解析/为空的 delta、以及本轮新增的 rename 键 fail-closed 守卫），全部降级到这同一个 reason——但该 reason 的契约文本（本 PR 新增的 union 文档、此分支注释、SKILL.md 的条目）只描述了 undo 形状，并且对 rename 键形状断言了一个错误的事实：那里的 PR diff 确实显示着该改动（rename-fallback 集成测试显示 full capture 携带它）——收窄只是因为两次 capture 的键不同而拒绝。narrow-diff.ts 自己的 docstring 诚实地列出了全部四种形状；reason 的契约文本没有。
失败场景：一个因 rename 键分歧而被收窄拒绝的 --since 轮次，一边审查着全量范围，一边报告「改动范围内已无任何内容」；标签冲突使 R1-1 的类别在轮次报告中不可见——没人能统计 rename 分歧在生产中发生的频率，其结构性修复也无法用 reason 字段度量。路由恰好仍然正确（四种形状都是确定性的；SKILL 把该 reason 归为永不重试），故为 Suggestion。
建议修复：把 union 文档、此注释与 SKILL.md 条目扩展为列出全部四种 null 原因（包括 rename 键拒绝），或给 fail-closed 拒绝一个独立的确定性 reason。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // probe welds --base into its brief): it covers every published hunk | ||
| // and never a byte the PR's diff does not display, while the anchor | ||
| // range can carry hunks an undo round netted out of it. | ||
| anchor.incremental.diffBase = mergeBaseSha; |
There was a problem hiding this comment.
[Suggestion] R3-5: setting incremental.diffBase to the merge base collapses the distinction SKILL.md's provenance rule (~1222) is built on — record diffBase on a delta-scoped round, mergeBaseSha otherwise, "since recording the merge base for a round that reviewed diffBase..head hands the later reader a scope the run never had" — pinned verbatim by SKILL.test.ts ('records the range the round actually reviewed in provenance'). After this change diffBase IS mergeBaseSha on every narrowed round (this line is the field's only assignment, verified by grep), so the branch is vacuous and the rationale inverted: the round reviews a narrowed subset of mergeBase..head, so the merge base is precisely a scope the run DID have. The agent-prompt consumer was updated and welded to BASE; the SKILL provenance consumer was not touched.
Failure scenario: any later attempt to simplify the now-vacuous delta/other branch fails the SKILL.test.ts pin, so the misleading text is actively preserved; a maintainer reasoning about why narrowed rounds record diffBase reads the opposite of the truth.
Suggested fix: update the SKILL.md provenance bullet to state that a narrowed round records the merge base (the range the published hunks were assembled from) and correct the rationale; update the SKILL.test.ts pin to match.
中文说明
[Suggestion] R3-5：把 incremental.diffBase 设为 merge base 后，SKILL.md 溯源规则（约 :1222）赖以建立的区分消失了——该规则要求：delta 范围轮记录 diffBase、其他轮记录 mergeBaseSha，理由是「为审查了 diffBase..head 的轮次记录 merge base 会把一个该运行从未有过的范围交给后来的读者」——并被 SKILL.test.ts 逐字钉住（『records the range the round actually reviewed in provenance』）。此改动之后，每个收窄轮的 diffBase 就是 mergeBaseSha（本行是该字段唯一的赋值点，已 grep 核实），于是该分支变得空洞、理由恰好反转：本轮审查的是 mergeBase..head 的收窄子集，merge base 正是该运行拥有过的范围。agent-prompt 消费者已更新并焊到 BASE；SKILL 溯源消费者未被触及。
失败场景：未来任何简化这一现已空洞的 delta/other 分支的尝试都会撞上 SKILL.test.ts 的钉扎而失败，于是误导性文本被主动保留；维护者推敲「为什么收窄轮要记录 diffBase」时读到的是与事实相反的说法。
建议修复：更新 SKILL.md 溯源条目，说明收窄轮记录的是 merge base（发布 hunk 的拼装来源区间）并修正理由；同步更新 SKILL.test.ts 的钉扎。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round — this round's batch was bounded (~8 findings, Criticals first). The finding is accepted: with diffBase welded to the merge base on every narrowed round, the provenance rule's delta/other branch is vacuous and its rationale inverted. Planned fix: rewrite the SKILL.md provenance bullet to state that a narrowed round records the merge base (the range the published hunks were assembled from) and update the SKILL.test.ts pin to match.
中文说明
推迟到下一轮——本轮批次有上限（约 8 项发现，Critical 优先）。该发现被接受：diffBase 在每个收窄轮都焊到 merge base 之后，溯源规则的 delta/other 分支变得空洞、理由恰好反转。计划修复：重写 SKILL.md 溯源条目，说明收窄轮记录的是 merge base（发布 hunk 的拼装来源区间），并同步更新 SKILL.test.ts 的钉扎。
| expect(narrowToDelta(captureBytes(base, 'HEAD'), delta)).toBeNull(); | ||
| }); | ||
|
|
||
| it('accepts a delta whose deletion the PR diff performs too', () => { |
There was a problem hiding this comment.
[Suggestion] R3-7: the whole-file-deletion shape (@@ -1,N +0,0 @@ → parseDiff's zero-count clamp to the point range [0,0]) reaches narrowToDelta through the EMISSION path in no test — this control covers a mid-file deletion (a different range shape), and the rename-fallback test exercises whole-file deletion only through the guard-refusal arm. Emission of a whole-file deletion rides on two unpinned implementation choices at once: the newCount === 0 → newEnd = newStart clamp (diff-plan.ts:415; the pre-existing pin is only newEnd >= newStart) and overlaps's inclusive <=.
Failure scenario: an off-by-one refactor of either choice (strict <, or dropping the clamp so newEnd = -1) passes all tests green and then silently drops file deletions from incremental scope: round 2 git rm f.ts + edits g.ts → the f.ts deletion matches no full hunk, the section is skipped, published scope is g.ts only while the PR diff displays the deletion and the ledger certifies head. Current code is correct; only the pin is missing.
Witness (probe): the suggested scenario passes on unmodified code (deletion emitted). MUTANT (overlaps → strict <): probe fails — "expected … to contain 'deleted file mode'" (scope silently narrows to g.ts only) — while all 143 tests across narrow-diff.integration + fetch-pr.test + diff-plan.test pass green under the mutant.
Suggested fix: add a scenario — base carries f.ts + g.ts; round 1 edits both; round 2 git rm f.ts and edits g.ts; assert non-null, deleted file mode / --- a/f.ts present, the g.ts edit present, and everyLineIsDisplayed.
中文说明
[Suggestion] R3-7：整文件删除形状（@@ -1,N +0,0 @@ → parseDiff 的零计数夹取，得到点区间 [0,0]）在没有任何测试中经由发射路径到达 narrowToDelta——本对照测试覆盖的是文件中部删除（不同的区间形状），而 rename-fallback 测试只经由守卫拒绝分支触及整文件删除。整文件删除的发射同时依赖两个未被钉住的实现选择：newCount === 0 → newEnd = newStart 夹取（diff-plan.ts:415；既有钉扎只是 newEnd >= newStart）与 overlaps 的含等号 <=。
失败场景：对任一选择做差一重构（严格 <，或去掉夹取使 newEnd = -1）都能在全绿之后静默丢弃增量范围中的文件删除：第 2 轮 git rm f.ts + 编辑 g.ts → f.ts 删除匹配不到任何 full hunk，section 被跳过，发布范围只剩 g.ts，而 PR diff 明明显示着删除、ledger 却认证了 head。当前代码是正确的；缺的只是钉扎。
见证（探针）：建议的场景在未改动代码上通过（删除被发射）。MUTANT（overlaps → 严格 <）：探针失败——「expected … to contain 'deleted file mode'」（范围静默收窄为仅 g.ts）——而同一 mutant 下 narrow-diff.integration + fetch-pr.test + diff-plan.test 全部 143 个测试全绿。
建议修复：新增场景——base 携带 f.ts + g.ts；第 1 轮两者都编辑；第 2 轮 git rm f.ts 并编辑 g.ts；断言非 null、包含 deleted file mode / --- a/f.ts、包含 g.ts 编辑、且 everyLineIsDisplayed 为 true。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| 'p2.ts': lines(50, 'R') | ||
| .replace('R10\n', 'R10-EDIT\n') | ||
| .replace('R40\n', 'R40-EDIT\n'), |
There was a problem hiding this comment.
[Suggestion] R3-10: the guarantee the deleted oracle battery pinned as "records EVERY hunk of a section, not just the first" has no surviving pin on the new emission path. This scenario carries two delta hunks in p2.ts (R10-EDIT and R40-EDIT, >6 lines apart → two distinct full hunks, both overlapped), but never asserts the second hunk is present in the narrowed output.
Failure scenario: a regression emitting only the FIRST matching hunk — file.hunks.filter(...) rewritten as find(...)/[0] — ships green through the whole suite: not.toBeNull() passes; git apply --check --reverse passes (a single-hunk subset patch still reverse-applies cleanly); everyLineIsDisplayed passes trivially; the end-to-end NARROWED fixture's delta overlaps exactly one hunk, so first-match-only emits byte-identical text. Concrete outcome: a --since round on a file with two post-anchor edit regions publishes only the first hunk under effective: true; the second — displayed by the PR diff and changed since the anchor — drops from scope, the ledger certifies head, permanently certified unreviewed. Current code is correct (filter); only the pin is missing.
Witness (probe): MUTANT filter + .slice(0, 1) → 102/102 tests green (integration + fetch-pr end-to-end). Same mutant + the assertions below → red: expected … to contain '+R40-EDIT'. Unmodified code + the assertions → 102/102 green. The probe flips.
Suggested fix: assert both hunks survived — expect(narrowed).toContain('+R10-EDIT'); expect(narrowed).toContain('+R40-EDIT');.
中文说明
[Suggestion] R3-10：被删除的 oracle 测试组曾钉住的保证——「记录一个 section 的每一个 hunk，而不只是第一个」——在新的发射路径上没有存活的钉扎。本场景的 p2.ts 携带两个 delta hunk（R10-EDIT 与 R40-EDIT，相隔 >6 行 → 两个不同的 full hunk，且都被重叠），但从未断言第二个 hunk 出现在收窄输出中。
失败场景：一个只发射第一个匹配 hunk 的回退——把 file.hunks.filter(...) 改写成 find(...)/[0]——会在整个套件全绿的情况下通过：not.toBeNull() 通过；git apply --check --reverse 通过（单 hunk 子集补丁仍能干净地反向应用）；everyLineIsDisplayed 平凡通过；端到端 NARROWED fixture 的 delta 恰好只重叠一个 hunk，首匹配发射的文本逐字节相同。具体后果：对一个锚点之后有两个编辑区域的文件的 --since 轮次，只发布第一个 hunk 且报告 effective: true；第二个——PR diff 显示着、且自锚点以来改变过的——掉出范围，ledger 认证 head，永久认证为已审查。当前代码是正确的（filter）；缺的只是钉扎。
见证（探针）：MUTANT filter + .slice(0, 1) → 102/102 测试全绿（集成 + fetch-pr 端到端）。同一 mutant + 下方断言 → 红：expected … to contain '+R40-EDIT'。未改动代码 + 该断言 → 102/102 全绿。探针翻转。
建议修复：断言两个 hunk 都存活——expect(narrowed).toContain('+R10-EDIT'); expect(narrowed).toContain('+R40-EDIT');。
— qwen3.8-max via Qwen Code /review (v0.21.12)
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 — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally.
Not explored to full depth (tool budget reached): chunk 8: executing SKILL.test.ts under vitest — neither the worktree nor the parent checkout has node_modules , and a full monorepo install was out of proportion for …; "agent reverse-audit (round 3)": none — all checks above ran to completion within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 8：executing SKILL.test.ts under vitest — neither the worktree nor the parent checkout has node_modules , and a full monorepo install was out of proportion for …；"agent reverse-audit (round 3)"：none — all checks above ran to completion within budget.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.12)
- Fail closed on a rename the full capture does not key as the SAME rename: when round 1 rewrites a file below git's rename threshold and round 2 renames it, `base..head` nets the chain to an addition plus a deletion while `anchor..head` carries a 100%-similarity rename keyed on the new path. The path guard passed — the new path is in the full capture, as the addition — while the rename's deletion half sat under the old path and dropped from the published scope under `effective: true`. `parseDiff` now exposes `rename from`, and the narrowing join refuses unless both captures key the same rename; the round keeps the full range, which still displays the deletion. - Fatal-decode the delta symmetrically with the full capture: `narrowToDelta` takes the delta's raw bytes and decodes them itself. A lossily pre-decoded delta folded an invalid path byte onto U+FFFD, which could collide with a legitimate U+FFFD path in the full capture and publish an unchanged file's hunks. - Record the executable bit through git itself in the mode scenarios (`git update-index --chmod=...` beside the filesystem chmod): `chmodSync` is invisible to git on Windows — libuv cannot set the exec bit and `core.fileMode` is false — so the two mode tests failed on the Windows merge-queue leg. Verified against a Windows-git model (`core.fileMode=false`): 2 failed before, 16/16 after. - Assert the narrowing outright where the scenarios are constructed to narrow: the null guards on the undo-per-feedback and post-anchor-file tests let an all-or-nothing refusal ship green with zero assertions executed. - Pin the unpinned emission shapes: a whole-file deletion riding the `+0,0` clamp and inclusive `overlaps`, a mode-only full section the delta touches with content hunks, and both hunks of a two-region file surviving the join — each verified red against the corresponding mutant. - Enumerate all four null shapes under `nothing-to-narrow` in the report's union doc, the demotion arm, and the skill's reason bullet; the routing (deterministic, never retried) already held for all four. The rewrite-then-rename regression test fails against the pre-round code (the probe published [new.ts, other.ts] with the deletion absent); the mode scenarios fail under the Windows model before the index-native recording.
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9267 (Critical-only mode)Feedback triage
Reproduction (before any fix)Both entrances were reproduced with real-git captures driven through the actual
FixThe guard no longer tries to discriminate a netted-out undo from a Myers misplacement — the captures cannot tell the two apart (that discrimination would need base content, which
Pinned expectations that changed (content evidence, per the test-weakening rule)The fail-closed option the finding sanctions deliberately ends the netted-undo section drop: an uncorroborated section is carried whole instead of dropped. Three tests pinned the old drop and were rewritten to pin the new behavior — none was deleted, and every scenario keeps its safety-invariant assertions (
Verification-gate note: the two new entrance tests fail against the pre-round branch (reproduction above), satisfying the round's pre-branch failure requirement. Documentation kept truthful
Not addressed this round
VerificationCommands actually run, final state (commit
Not run: bundled-CLI integration tests — the touched behavior is exercised directly by the vitest integration suite (real-git captures) and the fetch-pr handler unit tests, not only through the bundle. Settings sources unchanged, so no schema regeneration. 中文说明Autofix 轮次总结 — PR #9267（仅 Critical 模式）反馈分类
复现（修复前）两个入口均用真实 git 捕获、驱动真实
修复守卫不再试图区分「净零撤销」与「Myers 错位放置」——仅凭两份捕获无法区分二者（该区分需要 base 内容，而
被更改的既有期望（按测试削弱规则给出内容证据）发现所背书的 fail-closed 选项有意终结「净零撤销的 section 丢弃」：未被佐证的 section 现在整段携带，而不是被丢弃。三个测试固化了旧的丢弃行为，已改写为固化新行为——均未删除，且每个场景保留安全性不变量断言（
验证门槛说明：两个新入口测试在轮前分支上失败（见上方复现），满足本轮的轮前失败要求。 文档保持真实
本轮未处理
验证实际执行的命令，最终状态（提交
未运行：bundled-CLI 集成测试——本次触及的行为由 vitest 集成套件（真实 git 捕获）与 fetch-pr handler 单测直接覆盖，并非仅通过 bundle 行使。settings 源未变更，无需重新生成 schema。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R4-4 probe --base welded to the merge base re-expands test-efficacy selection — rediscovered twice this round — already reported (comment 3795442666), author deferred to the follow-up queue
- D6-1 probe-base test title/rationale asserts the inverse of the shipped merge-base semantics — rediscovered twice this round — already recorded as the round-6 deferral D6-1
- R1-9 stale oracle vocabulary sweep (fetch-pr.ts:911 rationale block, fetch-pr.test.ts:1369-1398 title/comment, diff-flags.ts:26 and diff-flags.test.ts references) — rediscovered three times this round — already reported (comment 3792882625)…
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally (only a Windows-git model probe from an earlier round stands in for the Windows leg).
Test Plan (not a blocker): 1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts:587 — [probe] D10-1: 'keeps every full hunk a single delta hunk overlaps' never reaches the overlap-selection branch it claims to pin — a first-match-per-range regression …packages/cli/src/commands/review/lib/narrow-diff.ts:222 — [review] D10-2: dead firstHunk === undefined arm; the comment above attributes the hunkless-full-section treatment to this line when the guard branch implements itpackages/cli/src/commands/review/lib/narrow-diff.ts:201 — [probe] D10-3: corroboration guard rebuilds changedLines per corroborated pair — O(M×L) on giant hunks with many scattered corroborated edits (measured 2.4 s at M=600)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally (only a Windows-git model probe from an earlier round stands in for the Windows leg)。
Test Plan（非阻断）：1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9267Feedback triage
Changed files
Verification
中文说明Autofix 轮次总结 — PR #9267反馈分诊
变更文件
验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- probe --base re-expands test-efficacy selection to the full PR range (R4-4) — rediscovered this round — already reported (comment 3795442666), author deferred to the follow-up queue
- probe-base test title/rationale asserts the inverse of the shipped merge-base semantics (D6-1) — rediscovered twice this round — already recorded as the round-6 deferral D6-1
- scopedDelta declaration comment still asserts the pre-PR delta-as-published semantics (fetch-pr.ts:921) — rediscovered this round — already recorded as the round-8 deferral R8-2
- corroboration guard rebuilds changedLines per corroborated pair, O(M×L) (narrow-diff.ts) — rediscovered this round — already recorded as the round-10 deferral D10-3
Not reviewed: reverse audit — stopped at the round cap of 5 without two consecutive dry rounds (rounds 1-4 each reported new findings; round 5's only report was a duplicate of the recorded D6-1).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally (only Linux runs stand in for the platform legs exercising the new platform-sensitive integration tests).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 3: executing fetch-pr.test.ts to confirm collection/passing — worktree and parent checkout both lack node_modules ( vitest unresolvable), so verification was stru….
Test Plan (not a blocker): 1 passed — this review observed 21525, 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 21525, 20503, 1578, 1597, 494, 3747, 529 passed.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts:1136 — [review] D11-1: vacuous not.toBeNull() behind a ! dereference — the null guard guards nothingpackages/cli/src/commands/review/fetch-pr.ts:979 — [review] D11-2: operator status line still announces the anchor range as the published scope (round-8 deferral, still stands)packages/core/src/skills/bundled/review/SKILL.md:167 — [probe] D11-3: planless nothing-to-narrow round from a retryable write fault ships under a non-retryable reasonpackages/cli/src/commands/review/fetch-pr.ts:767 — [review] D11-4: anchor-block comment asserts since..head scoping and no base consultation — both inverted by this diff (no inline anchor: unchanged lines outside every hunk)packages/cli/src/commands/review/fetch-pr.test.ts:1181 — [probe] D11-5: comment 'the PLAN is the delta's' falsified — the plan now counts the published narrowed textpackages/cli/src/commands/review/fetch-pr.ts:230 — [review] D11-6: behind-merge-base rationale claims an old anchor 'would scope WIDER than the PR's diff' — structurally impossible under build-from-full narrowing (also docstring 310-317)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the round cap of 5 without two consecutive dry rounds (rounds 1-4 each reported new findings; round 5's only report was a duplicate of the recorded D6-1)。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally (only Linux runs stand in for the platform legs exercising the new platform-sensitive integration tests)。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 3：executing fetch-pr.test.ts to confirm collection/passing — worktree and parent checkout both lack node_modules ( vitest unresolvable), so verification was stru…。
Test Plan（非阻断）：1 passed — this review observed 21525, 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 21525, 20503, 1578, 1597, 494, 3747, 529 passed。
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Four consecutive rounds reported the same class and each fix was defeated by the next round's entrance: whole-section miss, partial miss, then two shapes defeating the position-divergence guard's conjuncts, then content-only corroboration, then junction-keyed corroboration defeated by a delta hunk carrying two changes. The reviews were right about the cause each time and right about the pattern: matching hunks across the two captures is a heuristic over arbitrary content, which is what the containment oracle this file replaced also was. The two captures are independent Myers alignments over overlapping content, so which HUNK a change lands in is not stable between them — a run of identical lines lets the same edit be attributed to the run's front in one and its back in the other. Every guard here tried to recognise that divergence; none could, because it is a property of the alignment and not of the change. What IS stable is which FILE a change belongs to, and the path and rename guards already fail closed on the one way that could differ. So the unit of narrowing is the file: a section the delta touched is emitted whole, a section it did not touch is dropped. Nothing the delta performed can fall out of a section emitted entire, and the whole position-divergence family stops existing rather than being caught. That failure direction is the reason this could not stay as it was. A dropped hunk left the round reporting `effective: true`, and the ledger then certified head as the next anchor, so the change was never reviewed by any round. The deleted oracle failed toward more review; this failed toward silently less, recorded as complete. Cost, stated plainly: within a touched file the round now reviews all of that file's PR hunks, not only the ones that moved since the anchor. The saving incremental review exists for is the untouched files — a round touching 2 of 40 reviews 2 — and that is unaffected. Also: a base-free round no longer reports `capture-failed`. The fetch succeeded and `git merge-base` found no common ancestor, so nothing threw; naming an infrastructure fault put a deterministic state into the class the recovery flow retries. It reports `nothing-to-narrow`, and the two pins that asserted the old reason move with it.
|
5 Criticals from rounds 8–11 addressed at Four of the five were one class, and the fix is to stop trying to catch it. Rounds 8, 9, 10 and 11 each reported position divergence, each fix was correct about the shape it was shown, and each was defeated by the next round's entrance: whole-section miss → partial miss → two shapes defeating the guard's conjuncts → content-only corroboration → junction-keyed corroboration defeated by a delta hunk carrying two changes. The reviews were right about the mechanism every time, and right about the pattern. The mechanism, as R8-1 put it: the two captures are independent Myers alignments over overlapping content, so which hunk a change lands in is not stable between them — a run of identical lines lets the same edit be attributed to the run's front in one capture and its back in the other. Every guard tried to recognise that divergence. None could, because it is a property of the alignment, not of the change. That is the same reason What is stable is which file a change belongs to — and the path and rename guards already fail closed on the one way that could differ. So the unit of narrowing is the file: a touched section is emitted whole, an untouched one is dropped. Nothing the delta performed can fall out of a section emitted entire, so the class stops existing rather than being caught. The failure direction is why this could not stay: a dropped hunk left the round reporting Cost, stated rather than buried. Inside a touched file the round now reviews all of that file's PR hunks, not only the ones that moved since the anchor. The saving incremental review exists for is the untouched files — a round touching 2 of 40 reviews 2 — and that is unaffected. Witness, both directions, because "emit everything" would pass a one-sided test as well as the real rule: The fifth Critical, R11-1, was separate and correct: a base-free round reported
中文说明第 8–11 轮的 5 个 Critical 已在 其中四个是同一类，而正确的做法是不再试图「抓住」它。 第 8、9、10、11 轮各自报告了位置分歧，每次修复对当轮展示的形状都是对的，又都被下一轮的新入口击穿：整段落空 → 部分落空 → 两种形状分别击穿守卫的两个合取项 → 仅按内容佐证 → 按 junction 键佐证又被「一个 delta hunk 承载两处改动」击穿。评审每一次对机制的判断都是对的，对模式的判断也是对的。 机制正如 R8-1 所述：两份 capture 是对重叠内容的两次独立 Myers 对齐，所以一处改动落在哪个 hunk 在两者之间并不稳定——一段相同的行会让同一处编辑在一份 capture 里归到行段前部、在另一份里归到后部。所有守卫都在试图识别这种分歧。都做不到，因为它是对齐的性质，而非改动的性质。这与当初必须删掉 稳定的是一处改动属于哪个文件——而路径与 rename 守卫已经对唯一可能不同的那条路径 fail closed。所以收窄的单位改为文件：被触及的 section 整段发出，未被触及的丢弃。整段发出的 section 不可能漏掉 delta 执行过的任何东西，于是这一类问题不再需要被「抓住」，而是不复存在。 失效方向正是它不能维持原样的原因：丢掉一个 hunk 会让本轮报告 代价明说而非掩盖。 在被触及的文件内部，本轮现在会审查该文件在 PR diff 中的全部 hunk，而不只是自锚点以来变动的那些。增量评审真正的节省来自未触及的文件——一轮触及 40 个中的 2 个就只审 2 个——这一点不受影响。 变异验证，双向进行，因为「全部发出」也能通过单向测试： 第五个 Critical（R11-1）是独立且正确的：base-free 轮次在没有任何异常抛出的情况下报告了
|
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.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- scopedDelta doc still asserts the pre-PR delta-as-published semantics (fetch-pr.ts:921) — already recorded as the round-8 deferral R8-2, rediscovered in rounds 11 and 12
- agent-prompt.test.ts 'scopes Agent 7's probe base to the delta on an incremental round' test title/rationale — already recorded as the round-6 deferral D6-1, rediscovered in rounds 10, 11 and 12
Not reviewed: reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (all five rounds reported findings).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner).
Test Plan (not a blocker): 1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts:630 — [review] D12-1: 'carries a netted-undo section whole while its sibling still narrows' — title/comments assert a sibling narrowing the scenario contradicts (both file…packages/cli/src/commands/review/lib/narrow-diff.ts:31 (+6 locations) — [review] D12-2: stale hunk-join vocabulary — header/docstring describe a hunk-level corroboration ('overlaps … shares a changed line … keyed by new-side junction') the …packages/cli/src/commands/review/fetch-pr.ts:244 — [probe] D12-3: 'Every shape keeps the full range: wider, never wrong' — false for the planless no-merge-base shape of nothing-to-narrow (diffPath: null, nothing kept); SKILL.md:165 mirror c…packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts:82 — [probe] D12-4: everyLineIsDisplayed is membership-only — byte-exact in-order exactly-once assembly is pinned by nothing (probe: reordered sections pass 30/30; the sug…
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (all five rounds reported findings)。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner)。
Test Plan（非阻断）：1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The R11-1 fix keyed on `mergeBaseSha === null` alone, but that null has two
causes and only one is deterministic. A base that could not be FETCHED — a
fresh CI clone with no local base ref, hitting a transient fault — is
infrastructure: something did fail, and the re-run re-runs exactly the
component that failed. Reporting `nothing-to-narrow` there put a retryable
state into the never-retried class and pointed operators at "nothing to
narrow" instead of a fetch failure.
The arms are split by `baseFetchFailed` now: fetch failed keeps
`capture-failed`, no-common-ancestor keeps `nothing-to-narrow`. This is the
same distinction SKILL.md's recovery paragraph already draws for a planless
`partition-failed`, applied where the code makes the same choice.
The pin that was supposed to cover this asserted `nothing-to-narrow` over a
`{sha: null, baseFetchFailed: true}` fixture while its comment said "the
fetch succeeded" — a fixture contradicting its own comment, which is what
kept the gap invisible. It is one test over both fixtures now, each with the
reason its cause implies.
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.
8 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D11-5 'the PLAN is the delta's' comment (fetch-pr.test.ts:1187) — already recorded as the round-11 deferral D11-5, rediscovered this round
- R8-2 scopedDelta doc still asserts the pre-PR delta-as-published semantics (fetch-pr.ts:921) — recorded round-8 deferral, rediscovered in rounds 11, 12 and this round
- D11-4 anchor-block comment asserts since..head scoping and no base consultation (fetch-pr.ts:763-770) — already recorded as the round-11 deferral D11-4
- D6-1 agent-prompt.test.ts probe-base test title/rationale asserts the inverse of the shipped merge-base semantics — recorded round-6 deferral, rediscovered in rounds 10-13
- R8-5 dead fullText === null disjunct in the merged capture-failed arm (fetch-pr.ts:962) — recorded round-8 deferral
- D12-3 'every shape keeps the full range' false for the planless no-merge-base shape (fetch-pr.ts:244, SKILL.md:165) — recorded round-12 deferral, rediscovered this round
- R1-9 stale containment-oracle vocabulary sweep — already reported (comment 3792882625), author deferred as one coherent sweep
- R3-5 SKILL.md provenance rule falsified by the diffBase weld flip — already reported (comment 3792882635), author deferred to the follow-up queue
Not reviewed: reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 each reported findings; round 4's were duplicates of recorded deferrals).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — one suspicion examined and discarded: the retry paragraph's planless- partition-failed -with-null- mergeBaseSha -AND- baseFetchFailed: true exception ap…; chunk 3: run fetch-pr.test.ts under vitest (worktree lacks node_modules/dist; install+build exceeds chunk budget).
Test Plan (not a blocker): 1 passed — this review observed 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 1578, 1597, 494, 3747, 529 passed.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts:581 (+5 locations) — [review] D13-1: leftover per-hunk-era comments assert the inverse of the shipped per-FILE contract — extends recorded deferrals D12-1/D12-2packages/cli/src/commands/review/fetch-pr.ts:923 — [review] D13-2: 'FULL range is read once' rationale cites the deleted oracle and omits the capture's new narrowing-material role (R1-9 overlap disclosed)packages/cli/src/commands/review/fetch-pr.test.ts:1093 (+2 locations) — [probe] D13-3: fixtures are not the 'ONE coherent history' the doc block certifies — real git emits different hunks (probe-verified)packages/cli/src/commands/review/fetch-pr.test.ts:2112 — [probe] D13-4: test rationale echoes the falsified 'no capture threw ⇒ deterministic' contract (R13-1 sibling)packages/cli/src/commands/review/fetch-pr.ts:236 — [probe] D13-5: nothing-to-narrow census omits the empty-full-capture refusal shape (probe-verified)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 8 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 each reported findings; round 4's were duplicates of recorded deferrals)。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner)。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — one suspicion examined and discarded: the retry paragraph's planless- partition-failed -with-null- mergeBaseSha -AND- baseFetchFailed: true exception ap…；chunk 3：run fetch-pr.test.ts under vitest (worktree lacks node_modules/dist; install+build exceeds chunk budget)。
Test Plan（非阻断）：1 passed — this review observed 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 1578, 1597, 494, 3747, 529 passed。
收敛姿态下延后（第 13 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #9267 (round at
|
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- impossible b.ts fixture hunk (fetch-pr.test.ts:1101) — already recorded as round-13 deferral D13-3 ('fixtures are not ONE coherent history')
- stale per-hunk-era comments (narrow-diff.integration.test.ts:581, ~630, ~894) — already recorded as round-13 deferral D13-1 (extends D12-1/D12-2)
- agent-prompt.test.ts:2593 probe-base test title/rationale asserts the inverse of the shipped merge-base weld — already recorded as round-6 deferral D6-1, rediscovered in rounds 10-13
Not reviewed: reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 reported findings; all Suggestions of the already-confirmed stale-prose class, no Criticals).
Test Plan (not a blocker): 1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/narrow-diff.ts:31 — [review] D14-1: module header/JSDoc describe a hunk-level corroboration judgment that does not exist (narrowing is per FILE)packages/cli/src/commands/review/fetch-pr.test.ts:1572 — [probe] D14-2: probe-throw arm pins baseFetchFailed: false on a genuinely failed fetch; SKILL.md's rules-load repair keys on that field alone (probe-verified)packages/core/src/skills/bundled/review/SKILL.md:167 — [probe] D14-3: base-less nothing-to-narrow declared absolutely deterministic/never-retryable; git exits 1 identically for an object-store fault (probe-verified); once-cap asymmetry vs t…packages/cli/src/commands/review/fetch-pr.test.ts:1343 — [probe] D14-4: newly written test comment teaches a nonexistent corroboration mechanism (3-arm probe: the judgment is causally inert)packages/core/src/skills/bundled/review/SKILL.md:161 (+1 locations) — [review] D14-5: stale since..head scope model and provenance rationale falsified by the diffBase → mergeBaseSha changepackages/cli/src/commands/review/lib/merge-base.test.ts:104 — [test] D14-6: added merge-base test is revert-inert (JSDoc-only source hunk; mechanical)packages/core/src/skills/bundled/review/SKILL.md:167 (+1 locations) — [review] D14-7: 'select the same hunks' rationale restates the removed hunk-level operation (selection is per FILE)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 reported findings; all Suggestions of the already-confirmed stale-prose class, no Criticals)。
Test Plan（非阻断）：1 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed; 4 passed — this review observed 20503, 1578, 1597, 494, 3747, 529 passed。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Two conflicts, both "keep both", plus one defect the merge itself created. fetch-pr.ts's `mergeBase` probe: this branch had just split its exit statuses three ways — exit 1 is the only deterministic "no common ancestor", anything else is the surface and throws — while #9092 pinned `core.commitGraph=false` on the same call. Both kept; the pin is a cache bypass and a stale commit-graph answers the walk from data the object store no longer agrees with, which is a wrong merge base, the one input every clamp and the whole narrowing rest on. fetch-pr.test.ts's mock block: `gitExit` from this branch and `statSync` from main, both into the same `producerMocks` object. The defect: the config pin moves `-c` to the front of argv, and the exit-split test identified the command as `args[0] === 'merge-base'`. Each side was correct alone; together the mock stopped matching and fell through to the default mapping, which can only produce exit 0 and exit 1 — turning a surface failure into "no common ancestor" and reporting the deterministic reason for a retryable state. The predicate matches the subcommand now rather than argv[0], since a config pin can always precede it. Also restores the `gitOpt` import, which this branch dropped when `mergeBase` moved to `gitExit` while main's side still had two callers.
|
Conflict resolved at Two conflicts, both "keep both" — and one defect the merge itself created, which is the part worth reading. The conflicts. The defect. The config pin moves The predicate matches the subcommand now rather than Two smaller things:
One measurement, reported not acted on (this PR is past round 5, so Criticals only): main's Suite 4317 passed | 1 skipped, eslint clean, typecheck clean under Local environment needed two fixes first, neither a code problem: the merge brought main's new 中文说明冲突已在 两处冲突都是「两边都保留」——以及一个由合并本身引入的缺陷，那才是值得一读的部分。 冲突。 缺陷。 那个 config pin 会把 现在判据改为匹配子命令而非 另外两件小事：
一项只报告、不动手的测量（本 PR 已过第 5 轮，只处理 Critical）：main 的 套件 4317 通过 | 1 跳过，eslint 干净， 本地环境先卡了两道，均非代码问题：合并带入了 main 新增的 |
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.
8 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- impossible b.ts fixture hunk header (fetch-pr.test.ts:1174) — already recorded as round-13 deferral D13-3 ('fixtures are not ONE coherent history')
- stale per-hunk-era comments (narrow-diff.integration.test.ts:581, ~668, ~1039) — already recorded as round-13 deferral D13-1 (extends D12-1/D12-2)
- stale containment-oracle rationale comments (fetch-pr.ts ~1179, ~1550) — already reported as the R1-9 sweep / round-13 deferral D13-2
- SKILL.md provenance paragraph asserting a diffBase/mergeBaseSha divergence — already reported as R3-5 (comment 3792882635)
- a.ts docblock-vs-header geometry mismatch (fetch-pr.test.ts:1148) — same recorded class D13-3 ('fixtures are not ONE coherent history')
- unconditional determinism clause for the base-less nothing-to-narrow shape (SKILL.md) — already recorded as round-14 deferral D14-3
- stale since..head scope prose (SKILL.md:167 effective:true bullet and the --since yargs description) — already recorded as round-14 deferral D14-5
- capture comment asserting since..head scoping and no base consultation (fetch-pr.ts ~1030) — already recorded as round-11 deferral D11-4
Not reviewed: reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 reported findings; all Suggestions of the recorded stale-prose/unpinned-pin class, no Criticals).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner).
Not explored to full depth (tool budget reached): chunk 3: running fetch-pr.test.ts under vitest to confirm the suite passes — the worktree and all reachable parents have no node_modules / dist , and a full npm ci ….
Test Plan (not a blocker): 1 passed — this review observed 22437, 20711, 1638, 1616, 494, 3849, 553 passed; 4 passed — this review observed 22437, 20711, 1638, 1616, 494, 3849, 553 passed.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/fetch-pr.ts:952 — [probe] D15-1: untested rethrow half of the new resolveMergeBase catch — the blanket-catch mutant ships greenpackages/cli/src/commands/review/fetch-pr.ts:1213 — [probe] D15-2: mergeBaseUnavailable arm's ordering below the empty-delta upToDate arm is unpinned — arm-swap mutant ships greenpackage-lock.json:18549 — [probe] D15-3: 11 lightningcss peer:true removals are resolver churn — npm install --package-lock-only re-adds exactly those linespackages/cli/src/commands/review/fetch-pr.test.ts:1641 — [probe] D15-4: the real gitProbe.mergeBase success branch is driven by no test — success-to-null mutant ships green (3938 tests)packages/core/src/skills/bundled/review/SKILL.md:173 (+2 locations) — [review] D15-5: once-cap keys on the base-less shape only; base-untrusted and resolved-base capture-timeout retry uncappedpackages/core/src/skills/bundled/review/SKILL.test.ts:163 (+3 locations) — [probe] D15-6: rewritten pin block leaves three load-bearing SKILL.md clauses unpinned — five named mutants ship greenpackages/core/src/skills/bundled/review/SKILL.md:171 (+2 locations) — [probe] D15-7: capture-failed cause prose omits/misdescribes reachable members — write faults, probe faults, stale-ref exit-1
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 8 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the reverse-audit round cap of 5 without two consecutive dry rounds (rounds 3-5 reported findings; all Suggestions of the recorded stale-prose/unpinned-pin class, no Criticals)。
未审查：build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI and their suites did not run locally (the changed suites ran only on Linux; the new platform-sensitive integration tests' Windows leg stands on the round-2 Windows-git model probe, not a Windows runner)。
未探索到全部深度（达到工具调用预算）：chunk 3：running fetch-pr.test.ts under vitest to confirm the suite passes — the worktree and all reachable parents have no node_modules / dist , and a full npm ci …。
Test Plan（非阻断）：1 passed — this review observed 22437, 20711, 1638, 1616, 494, 3849, 553 passed; 4 passed — this review observed 22437, 20711, 1638, 1616, 494, 3849, 553 passed。
收敛姿态下延后（第 15 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
@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: 80 passed · 0 failed · 80 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：80 通过 · 0 失败 · 80 总计 抖动门：✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9267 deep verification (round 2) —
|
| # | finding | severity | status at new head 9d53584 |
|---|---|---|---|
| S1 | Base-free refusal (mergeBaseSha === null, fetch succeeded, no common ancestor) moved from the deterministic containment-unverified to the retryable capture-failed, costing one pointless fetch-pr --since re-run per round on cross-fork/unrelated-history PRs |
Suggestion | fixed. Commits 8ae07e3 + 45bf8fc split the base-less state three ways: fetch failed → capture-failed (retryable, re-runs the failed component); merge-base probe exit ≠ 0/1 (exit-128 fatal, timeout kill, spawn failure) → GitUnavailable throw → capture-failed; fetch succeeded but no common ancestor → nothing-to-narrow (deterministic, never retried — exactly S1's scenario). SKILL.md pins the routing: base-less nothing-to-narrow is NOT retryable, and the residual base-less capture-failed member (whose deterministic sub-member git exit status cannot split) retries once, not forever. Re-measured at the new head: the arm exists at fetch-pr.ts (the else if (mergeBaseSha === null) demotion), is pinned by two tests (splits a base-free round by WHY there is no base, splits merge-base probe exits — only exit 1 is "no common ancestor"), and the vacuity mutant (arm re-stamped capture-failed) turns both tests red with the reason-field assertion; restore is green (04-vacuity-s1-red-on-mutant.png). One residual, disclosed rather than new: the once-cap lives in the SKILL.md recovery text (prompt-level), consistent with the recovery flow's existing design where the skill drives retries. |
| C1 (correction) | PR body's stale-bundle.test.ts pre-existing failure did not reproduce in this container |
— | stands, re-measured. stale-bundle.test.ts passes 28/28 at the new head (logs/gate-review-head.log). |
| C2 (correction) | PR body's mutant count `3 failed | 1 passed` referred to the old 4-case suite | — |
Central claim and A/B proof
Central claim (as it stands after the delta). The incremental scope is built from the PR's own mergeBase..head capture at file granularity — a section the delta touched is emitted whole, a section it did not touch is dropped — so every published line is a line GitHub displays, by construction. The hunk-level join the previous round verified (position-divergence guards, junction-keyed corroboration) was itself defeated by further shapes in review rounds and then deleted in c25efbe: which FILE a change belongs to is stable across the two Myers alignments; which HUNK it lands in is not. Fail-closed guards: a delta path absent from the full capture → null; a rename keyed differently across captures → null; non-UTF-8 on either side → null; nothing selected → null. All nulls keep the full range.
A/B design. Identical bytes on both arms: real three-commit histories (base → anchor → head), captures under PINNED_DIFF_CONFIG/PINNED_DIFF_FLAGS, host git config isolated. Base arm: containmentRuling live-imported from the base dist (worktree at HEAD^1, packages/cli rebuilt there); publication semantics as shipped at base (ok → publish the raw delta, !ok → refuse). Head arm: narrowToDelta from the PR dist (non-null → publish the file-level subset, null → keep full range). Expected base misbehavior is encoded as assertions that pass when the base misbehaves as predicted. Full log: logs/ab-run2.log; witness: 01-ab-oracle-vs-narrowing.png.
| # | scenario (real git) | base oracle → published | head file-level narrowing → published | cell meaning |
|---|---|---|---|---|
| s1 | clean-incremental (round 2 edits a different file; 2 files in PR) | ok → raw delta |
publishes the touched file's section only; untouched file dropped | parity + the file-level economy holds |
| s2 | undo-mixed (delta = displayed edit + undisplayed -X deletions) |
not-ok → refuse, full range |
publishes u.ts section whole: carries +U25-EDIT, excludes X1 |
flip: base loses the round; head keeps it displayed-only |
| s3 | position-divergence in a 20-line identical run (deletion folded at @@ -1,5 in full, placed at @@ -18,4 in delta) |
not-ok → refuse, full range |
carries the divergent -R deletion + +E-EDIT + sibling edit via whole sections, ⊆ full |
the family that defeated the hunk-level design: base refuses, head publishes safely |
| s4 | rename-then-edit (full keys a rename; delta keys a plain edit of the new path) | ok → publishes raw delta with 4 undisplayed lines (diff --git a/new.ts b/new.ts, index …, --- a/new.ts, @@ -6,5 +6,5 @@ O5) |
publishes the rename-keyed section from the full capture, ⊆ full | load-bearing: the check accepted undisplayed bytes; construction cannot |
| s5 | deletion the PR diff also performs (positive control, BOTH arms) | ok → publish |
narrows, carries -D2, ⊆ full |
symmetric positive control |
| s6 | transient file (created after base, deleted before head) | not-ok → refuse |
null (path guard) |
fallback parity; reason renamed |
| s7 | pure undo reverting round 1 entirely (full capture empty) | not-ok → refuse |
null |
fallback parity |
| s8 | invalid UTF-8 byte (0xFF) in the full capture | unverified: true → refuse |
null (fatal decode) |
fallback parity |
| s9 | rewrite below rename threshold, then rename (delta keys the rename, full does not) | not-ok → refuse |
null (rename guard) |
fallback parity |
49/49 assertions, including the invariant "every published head line ∈ the full capture's lines" on all publishing cells and the two load-bearing s4 facts (base oracle passes and its published bytes contain undisplayed lines — offenders quoted in the capture). s3 is the new central cell for this round: the exact mechanism the file-level redesign exists for (Myers attributing one deletion to a run's front in one capture and its back in the other) is reproduced with the premise asserted from the real captures, and head carries the change whole instead of dropping it under effective: true — the failure direction the commit history names as worse than a refusal.
Secondary claim 1 — nothing-to-narrow taxonomy. All enumerated shapes resolve to null → full range, verified across A/B s6/s7/s8/s9, the suite's unparseable-delta test, and both-arm UTF-8 refusals. SKILL.md pins the reason in the deterministic never-retried set, including its base-less shape (see S1 row).
Secondary claim 2 — diffBase welded to the merge base. Producer stamps anchor.incremental.diffBase = mergeBaseSha on publish; consumer reads shaOrNull(inc.diffBase) ?? shaOrNull(report.mergeBaseSha) (agent-prompt.ts:1578). The weld test (welds Agent 7's --base to the range the published scope came from) and the consumer-side pins are green in the gate.
Corrections
Carried from round 1 and re-measured (see status table): the stale-bundle.test.ts failure quoted in the PR body does not reproduce on either arm in this container (28/28 green at head), and the PR body's 3 failed | 1 passed mutant count belongs to the earlier four-case suite — the same mutant now fails 24 of the shipped 30.
One new correction of a description inside the PR's own code — see N1.
Findings
N1 (Nit) — stale design comments in narrow-diff.ts describe the hunk-level mechanism the file-level redesign deleted
packages/cli/src/commands/review/lib/narrow-diff.ts lines 31–35 state: "The one judgment left — which of the full capture's hunks the delta's ranges corroborate — fails closed the same way. A delta hunk no full hunk corroborates (overlaps its new-side range AND shares a changed line with, keyed by new-side junction) … so its section is emitted whole", and the function docstring (lines 47, 61–65) speaks of "hunks that overlap what changed since the anchor" and "a delta whose ranges miss the full capture's hunks … the join fails closed for it". None of that machinery exists in the shipped code — the selection is touched.has(file.path) plus the path/rename guards; the file contains no range-overlap, changed-line, or junction logic (grep -n "newStart\|newEnd" narrow-diff.ts → no code hits; the accurate file-level account starts at line 142, "Whole SECTIONS, not selected hunks"). The paragraphs are residue of the intermediate junction-keyed design (commits cef5952→8c5d94b) that c25efbe replaced. Behavior is unaffected and the conclusion the comments draw (over-inclusion over dropping) still holds; a reader looking for the junction-keying they name will not find it. Comment-only fix; no code change requested.
Not covered
- Per-commit attribution. Depth-2 checkout: the metadata lists 18 commits,
git rev-list HEAD^1..HEAD^2reaches only the head. The delta since the previously verified headceccd556(9 commits: two main-merges incl. the2dde9e6conflict resolution, the three hunk-level guards, the file-level redesign, the two S1 fixes, the final main-merge) is verified in aggregate; the intermediate commits' individual claims are exercised via the shipped scenarios, not separately attributed. - End-to-end CLI against a real remote/GitHub. No network/token in the sandbox. The fetch-pr decision arms are exercised by the package's mocked-probe suite (green in the 3938-test gate) and the narrowing unit by real-git captures; a live
qwen review fetch-prover a real PR ref was not driven. - Windows mode-bit leg. Linux container; the
core.fileMode=falsemodel was not replayed (the mode scenarios record the exec bit viagit update-index --chmod=…, the author's Windows fix). - Perf on real large long-lived PRs beyond the synthesized 150k-line section (78 ms through
narrowToDeltaon this box; the fatal-decode memory path vs the old re-encode-and-compare was not benchmarked). - The
capture-failedretry once-cap is stated in SKILL.md's recovery text and consumed by the skill at runtime — prompt-level enforcement, consistent with the flow's existing design; nothing mechanically bounds a model that ignores the text. Disclosed, not a finding. - The
gitProbe.fetchrewrite (gitOpt→gitExit(...).status === 0) is semantically identical by inspection (gitOpt(...) !== null≡ status 0, both AND-ed with the samerefExists), but no behavioral probe is possible without network; it is covered only indirectly by the green gate. - The lockfile delta (11 lines) removes
"peer": truefrom thelightningcss-*optional platform binaries and nothing else (verified: no other+/−lines). The installed dependency set is unchanged — the packages remain present innode_modules— so the base control's node_modules reuse stays clean; noted for completeness, no gate applies inside the sandbox.
Gates
| gate | arm | result |
|---|---|---|
packages/cli src/commands/review (92 files) |
HEAD | 3938 passed | 4 skipped | 0 failed (logs/gate-review-head.log) |
packages/core src/skills (17 files) |
HEAD | 372 passed | 4 failed (logs/gate-skills-head.log) |
packages/core src/skills (17 files) |
BASE | 372 passed | 4 failed — byte-identical failing file and test names (logs/gate-skills-base2.log, diff exit 0) |
stale-bundle.test.ts |
HEAD | 28/28 passed |
narrow-diff.integration.test.ts unmutated control |
HEAD | 30/30 passed |
The 4 skills failures (SkillManager > listSkills > should list skills from both levels, … reads the committed cache …, getSkillsBaseDirs > should return all user-level base dirs, bundled skills > should prioritize user-level …) are the same environmental os.homedir()-mock failures the previous round recorded, at a head whose SKILL.md content differed — content-independent by construction; the PR's core diff is docs/tests-only and its own SKILL.test.ts (23 tests) is green on both arms. Δ introduced by the PR: +0 failing, +0 passing.
Gate-liveness proof: the vitest harness demonstrably goes red — 24/30 under M1, the per-guard killers under M2–M5, and 2 split-tests under the S1 revert mutant — so the green runs above are live evidence, not an unproven zero.
Methodology note on the BASE skills arm: the first base-side run resolved ajv to the root ajv@6.12.6, which lacks dist/2020.js — the head tree carries a nested packages/core/node_modules/ajv@8.20.0 that worktrees (tracked files only) do not — failing the skill-manager.test.ts collection and two registry tests as a base-worktree artifact. Symlinking the nested dependency dir into the base tree (gate-skills-base2.log) removed the artifact and produced the comparison above.
Methodology
Runs happened in the CI verify container (node:22-bookworm class; npm ci + npm run build pre-done at HEAD). Base control: git worktree add tmp/base-tree HEAD^1 (50d2700), rebuilt packages/cli there against the root node_modules; the build exits non-zero on unrelated core type errors (mime/lite, fdir, @lydell/node-pty declarations, test-file TS errors — logs/build-base-cli.log) but emitted the review subtree completely, proven by live-importing and smoke-running containmentRuling before the A/B. The lockfile delta is confined to lightningcss peer markers (installed set unchanged), so node_modules reuse is a clean dependency control; the workspace-link confound was asserted per contract — every node_modules/@qwen-code/* resolves into the head tree — and is inert because the core production diff is docs-only and neither unit under test imports @qwen-code/* (narrow-diff.js imports only relative diff-plan.js). Harnesses (ab-harness.mjs, sibling-probes.mjs, mutant-matrix.sh, vacuity-s1.sh, kept in this directory) drive real git under the pinned flags with host config isolated, import compiled dist/ units, and script every expectation — expected base-arm misbehavior is encoded as passing assertions, so fail counts only unexpected outcomes. Mutants were applied to source in place one at a time with sha256-byte-identical restore verified after each (original 00b2ca81… for narrow-diff.ts, f2dab536… for fetch-pr.ts); the head working tree ended clean. Raw logs: logs/; evidence images: evidence/.
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/cli/src/commands/review/agent-prompt.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/agent-prompt.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/merge-base.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/merge-base.test.ts
file packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/narrow-diff.integration.test.ts
file packages/cli/src/commands/review/pr-context.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/pr-context.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/agent-prompt.test.ts: PPPPP
packages/cli/src/commands/review/fetch-pr.test.ts: PPPPP
packages/cli/src/commands/review/lib/merge-base.test.ts: PPPPP
packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: PPPPP
packages/cli/src/commands/review/pr-context.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.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/agent-prompt.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/merge-base.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/agent-prompt.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/merge-base.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/agent-prompt.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/merge-base.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/agent-prompt.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/merge-base.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/agent-prompt.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/merge-base.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/narrow-diff.integration.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.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
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary (PR #9267) — no action takenCritical-only mode is active for this PR: the deterministic brake engaged because 5 change-producing rounds are complete in this counting window (diff growth this window: source 61 / test 456 net lines, budgets 400/400). This round's actionable sections were all empty:
The non-Critical feedback listed in the Result: no code changes were made and no commit was created this round. The PR head remains unchanged. 中文说明Autofix 轮次总结（PR #9267）— 本轮未采取任何操作本 PR 已进入仅处理 Critical 的模式：由于本计数窗口内已完成 5 个产生改动的轮次，确定性刹车机制已触发（本窗口 diff 净增长：源码 61 / 测试 456 行，预算 400/400）。 本轮所有可执行区域均为空：
结果： 本轮未做任何代码修改，未创建任何提交。PR 的 head 保持不变。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete and the PR's diff grew src 61 / test 456 net lines beyond this counting window's baseline (budgets: 400/400). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次，且本计数窗口内 diff 净增长已达 源码 61 / 测试 456 行（预算 400/400）。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ Subtractive, fail-closed redesign — the scope is built from the PR diff instead of checked against it, the suite pins the invariant on real git histories, and CI is green on this head.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
CI is green and every Critical across rounds R1-R5 is resolved (rename-key divergence, hunk-less projection, out.push(...spread) RangeError, header+content drop, executable-bit pins).
Remaining open threads are all Suggestion-level and deferrable per the review scope-convergence rule: test-coverage gaps (untested branches / missing mutation pins), stale vocabulary/doc references, and edge cases (binary sections, rename-with-content, UTF-8 decode fidelity). None are correctness/security/data-loss/regression, so they don't block this merge.
main's #9267 landed the half this branch and it had in common: the incremental scope is now BUILT from the PR's own diff instead of a separately captured delta that a containment oracle then tried to vouch for. `narrow-diff.ts` reaches the same place this branch's slicing did, and its comments say so in the same words — "Whole SECTIONS, not selected hunks", "the unit of narrowing is the file". Two implementations of one idea is one too many, so this branch drops its own. Gone with it, because main answers each more simply and fails closed: the restoration probe and `restored`/`restoredFileCount` (an undone file is absent from the PR's diff, so the narrowing refuses and the round reviews the full range), the rename ride-along and its carrier rule (main refuses when the two captures key a rename differently), `lineage-unfollowable`, `sliceDiffByLines`, `merge-base`'s status extension, and `fileLineCount`'s move out of `fetch-pr`. What is left is the part main does not have, and the part the token saving depends on: the one-hop import widening. The narrowing is sound in one direction only — a caller cleared against the callee's OLD shape is unchanged by definition, so no delta capture can show it, and a scope holding only the touched files retires that seam at the next re-anchor. `narrowToDelta` is split into `selectNarrowing` (the guards) and `assembleSections` (the emit), with `narrowToDelta` composing them unchanged — main's 30 narrowing tests pass untouched. The widening runs between the two, on the selection whose guards have already passed, and only ever adds: with no edge to follow it publishes exactly what the unwidened round would, so the narrowing is the floor rather than a second path that could disagree with it. `incremental.scope` is new on the report — `deltaFiles`, `interaction[]` with the edges, `contextFileCount` — which is what `agent-prompt` and `roster` read to brief a chunk agent at the seam. Seven of main's fetch-pr tests assert the whole `incremental` object and now name it too. Net against main: +1774/-13 over 15 files, down from +3462/-1940.
The base dropped its own slicing when main's #9267 landed the same mechanism, so this branch follows it down: - `sliceDiffByLines` moves here, where its only caller now lives (`capture-local`'s content anchor). The base carried it for the PR-side slicing it no longer does. - `renamedFrom` becomes `renameFrom`, main's spelling for the same `parseDiff` field. - The `fileLineCount` export the fetch-pr mock stood up is gone with the base's move of it; nothing here reads it (the comment claiming `capture-local` does predates the `rescope` subcommand's removal). DESIGN.md also loses a paragraph it should never have had two of: an earlier "keep both sides" resolution on this branch left the widening paragraph duplicated, and the base's rewritten section replaces it outright. This branch's own local content-anchor paragraph stays, once.
|
Released in v0.21.15. |
5 participants
What this PR does
Replaces the containment oracle in
fetch-prwith a narrowing step. Incremental review used to captureanchor..headseparately, publish it as the round's scope, and then try to prove after the fact that every hunk in it also appeared in the PR's ownbase..headdiff. That proof is gone. The delta is now read only for the post-image line ranges it touched, and the published scope is assembled out of the full capture's own hunks — the ones those ranges overlap. Every line the review sees is therefore a line GitHub displays, by construction rather than by proof.Two refusal reasons existed solely to report the oracle's verdicts and go with it:
hunks-outside-pr-diffandcontainment-unverified. One reason replaces them, and it names a fact rather than a failed proof —nothing-to-narrow, when the PR's diff has no hunk left in the range that changed since the anchor. That is the "undo per feedback" round, where the commits since the anchor put lines back the way the base had them; there is genuinely nothing there to re-review, so the round keeps the full range, which is the review it would have done anyway.Why it's needed
A comment anchored on a line GitHub's PR diff does not display answers 422, and the Create Review call is all-or-nothing — one bad anchor loses every finding of the round. The oracle existed to prevent that, and it could not: it was a hand-written match over two rendered unified diffs, and its acceptance surface is unbounded because the content it matches over is arbitrary.
Six review rounds on #9100 each closed the entrances that round reported, and the next round found new ones — count-less headers, deletion junctions, lossy UTF-8 decodes collapsing distinct bytes onto U+FFFD, a deletion budget spent across hunks, content matched without position. Every one was the same shape: something the delta carried that the PR's diff did not display, arriving through a gap in the match. The last round's finding put it plainly — the surface cannot be closed entrance by entrance.
Building the scope from the PR's own diff removes the question instead of answering it better. It is also smaller: -255 lines of production code and the ~765-line battery that existed to pin it, against +29 lines and a four-case integration suite.
Reviewer Test Plan
How to verify
The integration suite drives real
giton real three-commit histories under the flagsfetch-prpins, and asserts the invariant directly rather than arguing it per shape: every line of the narrowed output must appear in the full capture, checked over the whole output on each scenario. One scenario is the undo-per-feedback history that defeated the oracle six times — round 1 adds lines, round 2 removes them, so the delta deletes text standing at neither the base nor the head. It also applies the narrowed text withgit apply --check --reverseto confirm the subset is still a well-formed diff.To see the invariant test has teeth, assemble the output from the delta's bytes instead:
Full review suite: 3205 / 3206. The single failure is
stale-bundle.test.ts, which fails with the identical assertion on a pristineorigin/maincheckout in a separate worktree — pre-existing and unrelated.Evidence (Before & After)
N/A — no user-visible or TUI change. The behaviour difference is which bytes land in the round's diff file, covered by the tests above.
Tested on
Environment (optional)
Unit and integration tests only; the integration suite shells out to the system
gitwith host config isolated.Risk & Scope
anchor..headrange. A change the delta performs that the PR's diff no longer displays is not reviewed — which is the point, since it cannot be commented on either, but it does mean the round reviews slightly less than "everything that happened since the anchor" in that case.mainand carries its own SKILL.md update — the recovery taxonomy now namesnothing-to-narrowand no longer enumerates the two reasons the oracle produced.incremental.reasonenum loseshunks-outside-pr-diffandcontainment-unverifiedand gainsnothing-to-narrow. The field is consumed by the/reviewskill, which ships in the same repo.Linked Issues
Follows #9100 (merged). Addresses the structural finding raised in that PR's round-16 review.
中文说明
这个 PR 做了什么
把
fetch-pr里的 containment oracle 换成一个「收窄」步骤。增量评审此前会单独抓取anchor..head、将其作为本轮范围发布，然后事后去证明其中每个 hunk 也都出现在 PR 自己的base..headdiff 里。这个证明被去掉了。现在只读取 delta 的后像行区间，发布的范围由全量 capture 自己的 hunk 拼装而成——即被这些区间命中的那些。因此评审看到的每一行都是 GitHub 会显示的行，是按构造成立，而非靠证明。有两个拒绝理由仅仅是为了汇报 oracle 的裁决而存在，它们一并被删除：
hunks-outside-pr-diff与containment-unverified。取而代之的是一个理由，且它陈述的是事实而非失败的证明——nothing-to-narrow：PR 的 diff 在「自锚点以来发生变化」的区间内已无任何 hunk。这正是「undo per feedback」那一轮：锚点之后的提交把那些行改回了 base 原样，那里确实已无内容可重审，于是本轮保留全量范围——而那本来就是它会做的评审。为什么需要
评论若锚定在 GitHub 的 PR diff 不显示的行上会返回 422，而 Create Review 调用是全有或全无——一个坏锚点会让本轮全部发现丢失。oracle 正是为阻止这件事而存在，而它做不到：它是对两份渲染后的 unified diff 做手写匹配，其接受面是无界的，因为它所匹配的内容是任意的。
#9100 上连续六轮评审，每轮都堵上了当轮报告的入口，而下一轮又找到新的——无计数头部、删除 junction、把不同字节塌成 U+FFFD 的有损 UTF-8 解码、跨 hunk 花掉的删除预算、只比内容不比位置。每一个都是同一种形状：delta 携带了某些 PR diff 不显示的东西，从匹配的缝隙里钻进来。上一轮的 finding 说得很直白——这个面无法靠逐个入口封堵。
从 PR 自己的 diff 构建范围，是消除这个问题而不是把它答得更好。它同时也更小：生产代码 -255 行，加上为钉住它而存在的约 765 行测试组，换来 +29 行与一个四用例的集成套件。
评审者测试计划
如何验证
集成套件在真实三提交历史上驱动真实
git，使用fetch-pr所固定的旗标，并直接断言不变量而非逐形状论证：收窄输出的每一行都必须出现在全量 capture 中，每个场景都对整个输出做检查。其中一个场景正是六次击败 oracle 的 undo-per-feedback 历史——第 1 轮加入若干行，第 2 轮又删除它们，于是 delta 删除的文本在 base 和 head 都不存在。套件还用git apply --check --reverse应用收窄后的文本，确认该子集仍是格式良好的 diff。要确认该不变量测试确实有效，可改为用 delta 的字节拼装输出：
review 全套件：3205 / 3206。唯一失败是
stale-bundle.test.ts，在独立 worktree 中检出纯净origin/main时以完全相同的断言失败——属既有问题且与本 PR 无关。证据（前后对比）
N/A——无用户可见或 TUI 变化。行为差异在于本轮 diff 文件里落的是哪些字节，已由上述测试覆盖。
测试环境
运行环境（可选）
仅单元与集成测试；集成套件调用系统
git，并已隔离宿主机 git 配置。风险与范围
anchor..head。delta 执行了、但 PR diff 已不再显示的改动不会被评审——这正是目的所在（因为它也无法被评论），但确实意味着在这种情况下本轮评审的内容略少于「自锚点以来发生的一切」。main，并自带 SKILL.md 更新——恢复分类现在写的是nothing-to-narrow，不再列举 oracle 产生的那两个理由。incremental.reason枚举移除hunks-outside-pr-diff与containment-unverified，新增nothing-to-narrow。该字段由同仓库内的/reviewskill 消费。关联 Issue
承接 #9100（已合入）。回应该 PR 第 16 轮评审中提出的结构性发现。