fix(cli): say review coverage gaps in the author's units, not chunk ids

[wenshao]

What this PR does

/review's posted body rendered coverage disclosures with the run's own bookkeeping as subjects: bare chunk ids, unsorted, one subject per chunk. This PR makes the posted body say chunk-level gaps in the PR author's units instead, and stops the opener from certifying over a body that denies it.

Three changes, all at render time — the structural entries, the event caps, the caller-echo dedup and the stderr remediation still key on chunk ids, which is where the id is the selector a reader can act on:

• Coverage now returns the plan's chunk→files table. DiffChunk.files was already written into the plan JSON; the coverage type slice dropped it.
• compose-review renders chunk gaps through a new describeChunkGap: every planned chunk collapses to "the entire diff", a narrow gap with known files names the files, and anything wider is counted against the plan's total ("38 of the diff's 49 sections"). Applied to the receipt sentence, the uncoverable sentence (bare CLI entries only — caller-authored entries like chunk 5 (src/big.min.js) still render verbatim) and the grouped per-cause sentences.
• The COMMENT opener may no longer say "Reviewed." above a disclosure set that denies it: when no chunk is both covered and undisclosed — or no chunk universe could be read at all — it opens with "⚠️ This run could not certify that any of this diff was reviewed." A rewritten launch demonstrably read its chunk, so coverage alone is not the test; certified means covered with no disclosure against it.

Why it's needed

The review posted on PR #7268 (this comment) is the motivating incident. The run certified nothing — 11 chunks ran on rewritten prompts, the other 38 plus two roles were never launched with their built prompts — and the posted body enumerated all 49 chunk ids across two sentences, the first group in transcript order (chunk 28, chunk 49, chunk 6, …), while opening with "Reviewed. Suggestions are inline."

Two defects in one body: the opener certified the exact thing every following sentence took back, and the disclosure vocabulary was the operator's, not the author's — nothing on the PR page maps chunk 28 to code, and the register comments in coverage.ts already require the posted gap text to be author-facing. The commands were scrubbed from that register; the nouns were not.

Reviewer Test Plan

How to verify

1. Re-run the feat(serve): Hot-reload workspace trust changes #7268 shape (49 chunks, 11 rewritten, 38 unlaunched, Step 4/5 not launched verbatim) through composeReview. The body is now:

   ⚠️ This run could not certify that any of this diff was reviewed. Suggestions are inline. Not reviewed: 11 of the diff's 49 sections — launched with a prompt that is not the one the CLI built. Not reviewed: 38 of the diff's 49 sections — its prompt was built, but no agent on record was launched with it. Not reviewed: Test coverage matrix (whole-diff) — … Not reviewed: reverse audit — … Not reviewed: verification — …

   against the previous ~250-word body that enumerated every id and opened "Reviewed."

2. Confirm a partial gap still names what the author owns: one unlaunched chunk over src/a.ts renders "the diff section covering src/a.ts", and the opener stays "Reviewed." while any chunk is certified.

3. Confirm the operator's channel is untouched: remediation still carries the exact --chunk <id> rebuild selectors, and check-coverage's stderr output is unchanged.

4. npx vitest run src/commands/review/ from packages/cli — 784 tests pass. eslint --max-warnings 0 on the three changed files passes; tsc -p packages/cli reports no errors in commands/review (the pre-existing acp-integration errors are stale-dist noise, present on main).

Evidence (Before & After)

Before (posted on #7268): "Reviewed. Suggestions are inline. Not reviewed: chunk 28, chunk 49, chunk 6, chunk 46, chunk 39, chunk 4, chunk 26, chunk 27, chunk 7, chunk 38, chunk 48 — launched with a prompt that is not the one the CLI built. Not reviewed: chunk 1, chunk 2, chunk 3, chunk 5, chunk 8, … chunk 47, Test coverage matrix (whole-diff), Agent 1c: Cross-file tracer — its prompt was built, but no agent on record was launched with it. …"

After: the body quoted in step 1 above — no chunk ids anywhere in the posted body, one phrase per cause, and an opener that matches the disclosures. No UI change, so screenshots are N/A.

Tested on

OS	Status
🍏 macOS	⚠️ not tested
🪟 Windows	⚠️ not tested
🐧 Linux	✅ tested

Risk & Scope

• Main risk or tradeoff: an operator who reads only the posted PR body loses the chunk-id detail there; it remains on stderr (remediation and check-coverage), which is the channel the register comments assign it to. A plan written before chunks carried files falls back to counts — the field is optional in the coverage reader.
• Not validated / out of scope: the roster role labels (Agent 1c: Cross-file tracer) still render verbatim in the body — reworking those labels forks the register shared with stderr and is left for a follow-up, as is collapsing the near-duplicate reverse-audit/verification rewritten sentences.
• Breaking changes / migration notes: none. CoverageFromTranscripts gains an additive plannedChunks field; all body changes are phrasing.

中文说明

本 PR 做了什么

/review 发布到 PR 上的 review body 此前用运行时的内部记账作为披露主语：裸的 chunk 编号、乱序、每个 chunk 一个主语。本 PR 让公开 body 以 PR 作者的单位来陈述 chunk 级缺口，并禁止开场白与后文的披露自相矛盾。

三处改动都在渲染层——结构化条目、verdict 上限、caller 回显去重和 stderr 修复命令仍以 chunk id 为键，那里 id 才是读者可以执行的选择器：

• coverage 现在返回 plan 的 chunk→files 映射表。DiffChunk.files 本来就写进了 plan JSON；只是 coverage 的类型切片把它丢掉了。
• compose-review 通过新的 describeChunkGap 渲染 chunk 缺口：覆盖全部 planned chunk 时收敛为 "the entire diff"；文件已知且数量少的窄缺口直接点名文件；更宽的缺口按 plan 总数计数（"38 of the diff's 49 sections"）。应用于 receipt 句、uncoverable 句（仅 CLI 自己加入的裸条目——caller 提供的 chunk 5 (src/big.min.js) 这类条目仍原样渲染）以及按 cause 分组的句子。
• COMMENT 开场白不得再在否定它的披露之上写 "Reviewed."：当没有任何 chunk 同时满足"被读过且无披露指向它"——或根本读不到 chunk 全集时——开场改为 "⚠️ This run could not certify that any of this diff was reviewed."。rewritten launch 的 agent 确实读了它的 chunk，所以仅凭 coverage 不够；"认证"意味着已覆盖且无披露与之相悖。

为什么需要它

动机事件是发布在 PR #7268 上的这条 review。那次运行什么都没认证——11 个 chunk 跑在改写过的 prompt 上，其余 38 个加两个 role 从未按构建的 prompt 启动——而发布的 body 用两句话枚举了全部 49 个 chunk 编号（第一组还是 transcript 顺序：chunk 28, chunk 49, chunk 6, …），开头却写着 "Reviewed. Suggestions are inline."。

一个 body 两个缺陷：开场白认证了后面每句话都在收回的东西；披露词汇是 operator 的而非作者的——PR 页面上没有任何东西能把 chunk 28 映射到代码，而 coverage.ts 的语域注释本来就要求公开 gap 文案面向作者。命令被挡住了，名词没有。

Reviewer 测试计划

如何验证

1. 用 feat(serve): Hot-reload workspace trust changes #7268 的形态（49 chunk、11 rewritten、38 未启动、Step 4/5 未按原样启动）重跑 composeReview，body 现在是上文英文部分第 1 步引用的样子——对比之前约 250 词、逐个枚举编号、以 "Reviewed." 开头的 body。
2. 确认部分缺口仍点名作者拥有的东西：一个未启动、覆盖 src/a.ts 的 chunk 渲染为 "the diff section covering src/a.ts"；只要还有 chunk 被认证，开场白保持 "Reviewed."。
3. 确认 operator 通道不受影响：remediation 仍携带精确的 --chunk <id> 重建选择器，check-coverage 的 stderr 输出不变。
4. 在 packages/cli 下 npx vitest run src/commands/review/——784 个测试全部通过；三个改动文件通过 eslint --max-warnings 0；tsc -p packages/cli 在 commands/review 下无错误（acp-integration 的既有报错是 stale-dist 噪音，main 上同样存在）。

证据（Before & After）

Before（发布在 #7268 上）：逐个枚举 49 个 chunk 编号的两句话，开头 "Reviewed. Suggestions are inline."。

After：英文部分第 1 步引用的 body——公开 body 中不再出现任何 chunk 编号，每个 cause 一个短语，开场白与披露一致。无 UI 变更，截图 N/A。

测试平台

OS	状态
🍏 macOS	⚠️ 未测试
🪟 Windows	⚠️ 未测试
🐧 Linux	✅ 已测试

风险与范围

• 主要风险或权衡：只看 PR body 的 operator 会失去那里的 chunk-id 细节；细节保留在 stderr（remediation 与 check-coverage），这正是语域注释为它指定的通道。chunk 不携带 files 的旧 plan 回退到计数——coverage 读取端该字段是可选的。
• 未验证或范围外：roster 的 role label（Agent 1c: Cross-file tracer）仍原样渲染进 body——改写这些 label 会分叉与 stderr 共享的语域，留作后续；reverse-audit 与 verification 的 rewritten 近重复句子的合并同样留作后续。
• Breaking changes / migration notes：无。CoverageFromTranscripts 新增 plannedChunks 字段（增量式）；body 改动均为措辞层。

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓ (minor: no ## Linked Issues heading, though #7268 is referenced in the body — not blocking).

Problem: observed bug with direct evidence — the motivating incident is the review posted on PR #7268 (linked comment), where the body opened "Reviewed. Suggestions are inline." above two sentences disclosing all 49 chunks by id. The contradiction and the operator-facing vocabulary are both demonstrated, not hypothetical.

Direction: aligned. The review command's posted body is author-facing output; rendering internal bookkeeping (chunk ids) there contradicts the register comments in coverage.ts that already require author-facing gap text. CHANGELOG shows active investment in review output quality (/code-review, /ultrareview improvements), so this area is clearly in scope.

Size: not applicable — all three files are in packages/cli/src/commands/review/, no core paths touched. 170 production lines, 153 test lines.

Approach: the scope feels right. Three focused changes — pass the plan's chunk→files table through coverage, translate chunk ids to author-facing phrases via describeChunkGap, and fix the opener to not certify what the disclosures deny. No unrelated changes or drive-by refactors. The caller-authored entries (like chunk 5 (src/big.min.js)) correctly stay verbatim, and the stderr remediation channel is untouched.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓（小问题：缺少 ## Linked Issues 标题，但正文中引用了 #7268——不阻塞）。

问题：已观测到的 bug，有直接证据——动机事件是发布在 PR #7268 上的 review（有链接），body 以 "Reviewed. Suggestions are inline." 开头，紧接着两句话用编号披露了全部 49 个 chunk。矛盾和面向 operator 的词汇都有实证，不是假设。

方向：对齐。review 命令发布的 body 是面向作者的输出；在那里渲染内部记账（chunk 编号）与 coverage.ts 中已要求面向作者的语域注释相矛盾。CHANGELOG 显示 review 输出质量有持续投入（/code-review、/ultrareview 改进），这个方向明确在范围内。

规模：不适用——三个文件都在 packages/cli/src/commands/review/ 下，未触及核心路径。170 行生产代码，153 行测试代码。

方案：范围合理。三处聚焦改动——通过 coverage 传递 plan 的 chunk→files 映射表、通过 describeChunkGap 将 chunk 编号翻译为面向作者的短语、修复开场白不与披露矛盾。无无关改动或顺手重构。caller 提供的条目（如 chunk 5 (src/big.min.js)）正确地原样保留，stderr 修复通道不受影响。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at e83979c5d243848e53620f23431bf3c245f93b1d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Independent proposal (from title + motivation only): I'd (1) pass the plan's chunk→files table through CoverageFromTranscripts — it's already in the plan JSON, just dropped by the type slice; (2) write a translation function that collapses chunk ids to author-facing phrases — "the entire diff" when all chunks are affected, file names for narrow gaps, counts for wide ones; (3) fix the opener so it can't say "Reviewed." above disclosures that deny it.

Comparison with the diff: the PR's approach matches this almost exactly. describeChunkGap implements the three-tier translation cleanly — dedupe, sort, check if the gap covers the whole plan, then try naming files (≤4, all known), then fall back to counting. The "one unknown chunk poisons the file list" rule is the right call: naming three files over a gap that also covers a fourth, unnameable one would overclaim. The opener's nothingCertified test correctly distinguishes "covered" from "certified" — a rewritten launch demonstrably read its chunk, so coverage alone isn't the test; certified means covered with no disclosure against it.

No critical blockers. No AGENTS.md violations (ESM, no any, kebab-case, collocated tests). The comments explain the why (register constraints, the #7268 incident) without narrating the what. The files.length <= 4 threshold is a reasonable heuristic — more than four files and counting reads better than a sprawling list.

One observation (not blocking): the f?.path defensive access in coverage.ts is good — a malformed plan entry won't crash the renderer.

Real-Scenario Testing

This PR changes the review body rendering (text posted to GitHub), not the TUI. The describeChunkGap function was exercised directly from the built dist:

$ node /tmp/demo-describe-chunk-gap.mjs 2>&1
=== describeChunkGap demo ===
All chunks → {"phrase":"the entire diff","plural":false}
One chunk  → {"phrase":"the diff section covering src/b.ts, src/c.ts","plural":false}
Two chunks → {"phrase":"the diff sections covering src/a.ts, src/d.ts","plural":true}
Wide gap   → {"phrase":"2 of the diff's 3 sections","plural":true}
No plan    → {"phrase":"2 sections of the diff","plural":true}

Unit tests (90 in compose-review.test.ts, 784 across the full review suite):

$ cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts 2>&1 | tail -15

 RUN  v3.2.4 /home/github-runner/actions-runner-10/_work/qwen-code/qwen-code/.qwen/worktrees/triage/packages/cli
      Coverage enabled with v8

 ✓ src/commands/review/compose-review.test.ts (90 tests) 1232ms
   ✓ composeReview — stacked states compose, none erased > every non-empty body ends with the model footer  361ms

 Test Files  1 passed (1)
      Tests  90 passed (90)
   Start at  10:06:52
   Duration  22.27s (transform 4.32s, setup 128ms, collect 12.75s, tests 1.23s, environment 466ms, prepare 154ms)

Full review suite: 31 test files, 784 tests — all pass. ESLint clean on all three changed files. TypeScript reports no errors in commands/review.

中文说明

代码审查

独立方案（仅基于标题和动机）：我会 (1) 通过 CoverageFromTranscripts 传递 plan 的 chunk→files 映射表——它本来就在 plan JSON 里，只是被类型切片丢掉了；(2) 写一个翻译函数，将 chunk 编号收敛为面向作者的短语——所有 chunk 受影响时说 "the entire diff"，窄缺口点名文件，宽缺口计数；(3) 修复开场白，使其不能在否定它的披露之上写 "Reviewed."。

与 diff 的比较： PR 的方案几乎完全匹配。describeChunkGap 干净地实现了三层翻译——去重、排序、检查缺口是否覆盖整个 plan，然后尝试点名文件（≤4 个，全部已知），最后回退到计数。"一个未知 chunk 污染文件列表"的规则是正确的：在一个也覆盖了第四个不可命名 chunk 的缺口上点名三个文件会过度声称。开场白的 nothingCertified 测试正确地区分了"被覆盖"和"被认证"——rewritten launch 确实读了它的 chunk，所以仅凭 coverage 不够；认证意味着已覆盖且无披露与之相悖。

无关键阻塞项。无 AGENTS.md 违规（ESM、无 any、kebab-case、同目录测试）。注释解释了 why（语域约束、#7268 事件），没有叙述 what。files.length <= 4 阈值是合理的启发式——超过四个文件时计数比冗长列表更易读。

一个观察（不阻塞）：coverage.ts 中的 f?.path 防御性访问很好——格式错误的 plan 条目不会让渲染器崩溃。

真实场景测试

本 PR 更改的是 review body 渲染（发布到 GitHub 的文本），而非 TUI。describeChunkGap 函数直接从构建产物中执行验证。

单元测试（compose-review.test.ts 中 90 个，完整 review 套件 784 个）：全部通过。ESLint 在三个改动文件上无警告。TypeScript 在 commands/review 下无错误。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at e83979c5d243848e53620f23431bf3c245f93b1d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — clean across every stage; the problem is demonstrated, the fix is minimal and correct, and the tests are thorough.

The motivating incident (#7268's review body) is a real, linked artifact — not a hypothetical. The fix does exactly what's needed: translate chunk ids to the author's units (files, "the entire diff", or counts) in the posted body, and stop the opener from certifying what the disclosures deny. The implementation matches my independent proposal almost line-for-line. describeChunkGap is 30 lines, handles edge cases cleanly (unknown chunks poison the file list, no-plan fallback counts), and the "certified ≠ covered" distinction in the opener is the right call — a rewritten launch read its chunk, so coverage alone can't be the test.

Every change in the diff serves the stated goal. No drive-by refactors, no scope creep. The stderr remediation channel keeps its chunk ids (where they're actionable selectors), and caller-authored entries render verbatim. 784 tests pass, ESLint and TypeScript are clean.

Would merge without hesitation.

中文说明

置信度：5/5——每个阶段都干净；问题有实证，修复最小且正确，测试充分。

动机事件（#7268 的 review body）是真实的、有链接的产物——不是假设。修复恰好做了需要做的事：在发布的 body 中将 chunk 编号翻译为作者的单位（文件、"the entire diff" 或计数），并阻止开场白认证披露所否定的内容。实现几乎逐行匹配我的独立方案。describeChunkGap 30 行，干净地处理边界情况（未知 chunk 污染文件列表、无 plan 回退计数），开场白中"认证 ≠ 覆盖"的区分是正确的——rewritten launch 确实读了它的 chunk，所以仅凭 coverage 不够。

diff 中的每一处改动都服务于既定目标。无顺手重构，无范围蔓延。stderr 修复通道保留 chunk 编号（那里它们是可执行的选择器），caller 提供的条目原样渲染。784 个测试通过，ESLint 和 TypeScript 均干净。

毫不犹豫地合并。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at e83979c5d243848e53620f23431bf3c245f93b1d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[gwinthis]

Review & Local Verification Report

代码审查

设计评价：以用户为中心的信息设计修复。 本 PR 解决了 review 覆盖缺口报告中使用内部 chunk ID（"chunk 1, chunk 2"）的问题——这些 ID 对 PR 作者毫无意义。

核心变更：

1. 用文件路径替代 chunk ID：describeChunkGap() 将 chunk 映射到其覆盖的文件（"the diff section covering src/a.ts"）
2. 全量缺口聚合：所有 chunk 未覆盖时说 "the entire diff"，而非列举 49 个 chunk ID
3. 零认证警告：当没有任何 chunk 被认证时，开头从 "Reviewed." 改为 "⚠️ This run could not certify that any of this diff was reviewed."

关键修复（#7268 回归）： 之前 body 以 "Reviewed. Suggestions are inline." 开头，然后列出所有未覆盖的 chunk——第一句就"认证"了后面每一句都在否定的东西。

测试覆盖： 大量测试更新 + 新增测试，验证：

• chunk ID 不出现在 body 中（not.toMatch(/chunk \d/)）
• 全量缺口说 "the entire diff"
• 零认证时不以 "Reviewed." 开头
• 部分认证时保留 "Reviewed." 开头

结论

LGTM。 将内部簿记语言转换为用户可理解的语言，同时修复了自相矛盾的认证声明。

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

[yiliang114]

LGTM. Clean, well-motivated change — chunk ids are internal bookkeeping and the author needs their own units (files, or the diff itself). The describeChunkGap translation is correct across all edge cases: full-plan collapse, narrow fileNamed gaps, wide gaps, unknown-chunk poisoning, and no-plan fallback. The zero-certification opener closes the #7268 regression structurally.

Test coverage is thorough — 144 added lines covering every branch of describeChunkGap plus the opener's certified/partial/zero states.

— qwen3.8-max-preview via Qwen Code /review

[doudouOUC]

Reviewed. Suggestions are inline. Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.

— qwen3.7-max via Qwen Code /review

[doudouOUC]

[Suggestion] The bare-chunk-ID regex /^chunk (\d+)$/ plus Number(m[1]) appears three times in this file — here (uncoverable split), at the byReason split, and in the opener's disclosed-ID collection. The two split-render sites also duplicate the surrounding bareIds/named partition loop and the shown-array assembly.

Failure scenario: If the chunk subject format ever changes (e.g. chunk-5, Chunk 5, or additional parenthetical context), three sites need updating in lockstep — a missed update silently leaks raw chunk IDs in one sentence type while the others render file names.

Extract a parseBareChunkId(s: string): number | null helper, and for the two split-render sites a splitChunkEntries(entries: string[]): { bareIds: number[]; named: string[] } that uses it.

— qwen3.7-max via Qwen Code /review

[doudouOUC]

[Suggestion] The uncoverable-chunk rendering path's bare-ID branch is never exercised by any integration test. All existing uncoverable tests pass caller-named entries like 'chunk 5 (src/big.min.js)'; the bare format chunk <id> (produced by uncoverable.push(prefix) when cov.uncoverableChunks has entries no caller has named) is dead in the test suite.

Failure scenario: The underlying describeChunkGap is unit-tested, but there is no integration test verifying that a bare uncoverable entry renders as a file phrase rather than leaking a chunk ID through the full composeReview path.

Add one test that feeds an uncoverable entry without a caller-named variant alongside a plan whose chunks carry files, and assert the body contains the file name rather than chunk 1.

— qwen3.7-max via Qwen Code /review

[wenshao]

Review — APPROVE (C=0)

Clean, well-scoped render-time fix. The two defects from the #7268 incident (contradictory opener, internal chunk IDs in the author-facing body) are both addressed correctly. All changes are additive and backward-compatible; the operator channel (stderr, remediation) is untouched.

Three suggestions below — none blocking.

Verification: Build passes across all 13 workspaces. No test failures attributable to this diff (pre-existing shellExecutionService.ts environment issue on macOS). Security, removed-behavior, and cross-file traces all clean.

[wenshao]

Suggestion (non-blocking): The same /^chunk (\d+)$/ parse-and-extract logic appears at three independent sites added by this diff (here, line 701, and line 816). If the subject format ever changes, all three must update in lockstep — miss one and that site silently stops recognising chunk subjects, routing them into the named/callerNamed bucket and rendering raw bookkeeping IDs in the posted body.

Consider extracting a small helper:

function parseChunkSubject(s: string): number | undefined {
  const m = /^chunk (\d+)$/.exec(s);
  return m ? Number(m[1]) : undefined;
}

[wenshao]

Suggestion (non-blocking): describeChunkGap([], planned) with a non-empty plan returns "the diff sections covering " — trailing space, no file names. All three current callers guard with .length > 0, so this is unreachable today, but the function is exported and directly unit-tested.

A future caller that skips the guard posts a malformed sentence. Consider an early return (if (uniq.length === 0) return { phrase: 'none of the diff', plural: false };) or adding files.length > 0 to this branch condition.

[wenshao]

Suggestion (non-blocking): The count fallback uses uniq.length (all gap IDs, including any not present in planned) as the numerator against planned.length. If a caller-written bare "chunk 99" enters bareIds and 99 is not in the plan, the phrase reads "2 of the diff's 1 sections" — self-contradictory on the public PR page.

Reachability is narrow (caller entries typically carry file annotations, routing to callerNamed; coverage-derived entries always use planned IDs), but counting only IDs present in the plan when one is available would make the phrase robust:

const knownInGap = uniq.filter((id) => byId.has(id));