fix(review): say an unread chunk once, under its cause - #7234
Merged
Conversation
The first post-grouping body on #7166 carried seventeen chunks twice: once in "nobody read them" and once in the not-launched roster sentence -- the consequence restated beside its cause. A chunk whose disclosure entry already says WHY it went unread (launch never happened, or happened on a rewritten prompt) now leaves the nobody-read sentence; the cap and the remediation keep the full list, and a chunk with no disclosed cause -- the 3A shape, where chunks are not roster requirements -- keeps the sentence, because it is the only place the author learns those lines went unread.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with concrete evidence — review 4730480309 on #7166 carried seventeen chunks twice (once in the "nobody read them" sentence, once in the not-launched disclosure). This is the last residual double-disclosure shape after #7190's grouping. Direction: aligned. This is a straightforward dedup fix in the review body composition — the consequence was restated beside its cause. CHANGELOG: no direct reference, but the area is actively maintained (#7190, #7166). Size: not applicable — no core paths touched. Both files are in Approach: the scope feels right. Two files, 78 lines total — one production file (+19 −4) and one test file (+52 −3). The fix filters Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有具体证据——#7166 上的 review 4730480309 把 17 个 chunk 说了两遍（一遍在 "nobody read them" 句，一遍在 not-launched 披露中）。这是 #7190 分组后最后一个残留的重复形状。 方向：对齐。这是 review 正文组合中的去重修复——后果在原因旁边重复陈述。 规模：不适用——未触及核心路径。两个文件均在 方案：范围合理。两个文件，共 78 行——一个生产文件 (+19 −4)，一个测试文件 (+52 −3)。修复逻辑：对已披露 subject 过滤 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: given the problem (chunks appearing in both the "nobody read them" sentence and a cause-specific disclosure), I would build a set of chunk subjects already explained by Findings: no critical blockers, no AGENTS.md violations. The implementation is clean:
Tests: 83/83 pass in Real-Scenario TestingThis PR changes internal body composition logic in The 中文说明代码审查独立方案： 给定问题（chunk 同时出现在 "nobody read them" 句和原因披露中），我会从 发现： 无关键阻塞，无 AGENTS.md 违规。实现干净：
测试：compose-review.test.ts 83/83 通过；全部 31 个 review 测试文件 769/769 通过。 真实场景测试本 PR 修改 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean, minimal fix for a real observed duplication; would merge without hesitation. The PR does exactly one thing: stops the "nobody read them" sentence from restating chunks that a disclosure entry already explains. The implementation matches my independent proposal line-for-line — build a set of disclosed subjects, filter, conditionally emit. No simpler path exists; the change is already at the minimum. What I like: the cap and remediation deliberately keep the full list (pinned by tests), so the dedup is purely cosmetic in the posted body — no information is lost from the actionable repair path. The two new tests pin both directions (suppress when cause exists, keep when it doesn't), and the updated blind-launch test asserts the deduped shape. Mutation-tested 2/2. No scope creep, no drive-by refactors, no unrelated changes. Every line in the diff serves the stated goal. 中文说明置信度：5/5 —— 干净、最小的修复，解决真实观测到的重复；毫不犹豫可合并。 PR 只做一件事：阻止 "nobody read them" 句重复陈述已有披露条目解释的 chunk。实现与我的独立方案逐行一致——构建已披露 subject 集合、过滤、条件输出。不存在更简路径；改动已是最小。 亮点：cap 和 remediation 刻意保留全量列表（测试钉住），去重纯粹是发布正文的视觉层面——可操作修复路径不丢失任何信息。两个新测试钉住两个方向（有原因时抑制、无原因时保留），更新的 blind-launch 测试断言去重后形状。突变测试 2/2 击杀。 无范围蔓延、无顺手重构、无无关改动。diff 中每一行都服务于既定目标。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
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. |
Collaborator
Author
|
@qwen-code /triage |
wenshao
commented
Jul 19, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| ).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('a chunk whose launch failure is already disclosed leaves the nobody-read sentence — cause, not consequence twice', () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] No test covers the partial-overlap case — some missing receipts disclosed, some not — which is the only scenario where unexplainedReceipts differs from both [] and missingReceipts. A future edit to the sentence template accidentally rendering missingReceipts instead of unexplainedReceipts would pass both existing tests unchanged: in the "all disclosed" test the guard prevents the push, and in the "none disclosed" test both lists are identical. Concrete cost: the exact #7166 double-disclosure this PR fixes could regress silently.
Consider adding a test with three chunks: chunk 1 built-but-not-launched (disclosed), chunk 2 reviewed, chunk 3 neither disclosed nor reviewed. Assert r.body contains chunk 3 under the "nobody read them" sentence but does NOT list chunk 1 in that sentence.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
approved these changes
Jul 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Approve ✅ — reviewed at the PR head.
A small, body-composition-only dedup fix. cappedBy, remediation, and every event decision are untouched, and I verified that claim against the code rather than taking it from the description.
Correctness — verified
- Subject match is exact, no false suppression. The filter keys on
coverageEntries[].subject === "chunk <id>". Traced throughcoverage.ts:roleLabel(req)returnschunk <id>for a chunk requirement, andlabel(rec, chunk)returnschunk <id>for blind/idle/unopened/rewritten chunk agents — both align exactly with the filter. A non-chunk subject can never equalchunk <id>(labelonly produces that form whenchunk !== null), so nothing is wrongly suppressed. - No information is lost. Every
coverageEntriesentry is independently rendered by the group-by-reason loop, so anychunk <id>dropped from the "nobody read them" sentence still appears in the body under its real cause. That invariant is what makes the dedup safe, and it holds. - Empty-clause guard prevents a dangling
Not reviewed: … — no agent…when every receipt is explained. - Cap and remediation keep the full list.
chunk-nobody-readand thechunks nobody readremediation stay keyed onmissingReceipts.length > 0, independent of the dedup — so the event and the orchestrator's repair are unchanged, exactly as described.
Tests
The two new pins plus the reworked blind-launch assertion are meaningful. nobody read them and no agent reported covering occur only in the suppressed sentence, so the not.toContain checks are precise.
One non-blocking note (already raised inline by the bot): there's no partial-overlap test where unexplainedReceipts is a proper non-empty subset of missingReceipts — e.g. a blind agent claiming chunk 1 while chunk 2 has no agent (chunk 1 suppressed, chunk 2 kept). Current behavior is correct there, but a future swap of unexplainedReceipts → missingReceipts in the template would pass all three existing tests, so a single such pin would close the gap. Fine as a follow-up.
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Says an unread chunk once, under its cause. A chunk whose disclosure entry already explains WHY it went unread — its launch never happened, or happened on a rewritten prompt — now leaves the
nobody read themsentence in the posted body. Thechunk-nobody-readcap and thechunks nobody readremediation keep the full list; only the posted sentence dedupes. A chunk with no disclosed cause — the 3A shape, where chunks are not roster requirements — keeps the sentence, because it is the only place the author learns those lines went unread.Why it's needed
The first body composed after #7190's grouping landed (review 4730480309 on #7166) carried seventeen chunks twice: once in
— no agent reported covering these; nobody read themand once in— its prompt was built, but no agent on record was launched with it. The consequence restated beside its cause, across two clause categories the per-subject dedup did not span — the last remaining double-disclosure shape.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review— 31 files, 769 tests green.nobody read themabsent; cap and remediation intact); a 3A plan with no launch-side disclosures keeps the receipts sentence verbatim. The existing blind-launch test now asserts the deduped shape (cause once, cap and repair riding along).Evidence (Before & After)
Before (#7166, live):
Not reviewed: chunk 2, chunk 3, … — no agent reported covering these; nobody read them. … Not reviewed: …, chunk 2, chunk 3, … — its prompt was built, but no agent on record was launched with it.After, same state: the seventeen twice-told chunks appear once, in the cause sentence; the receipts sentence renders only for chunks nothing else explains.
Tested on
Risk & Scope
cappedBy,remediationand every event decision are untouched (pinned).chunk <id>subject already carried by a disclosure entry — a chunk explained by ANY cause (not-launched, rewritten, idle, blind, unopened) defers to it.Linked Issues
Exhibit: review 4730480309 on #7166 — the first post-#7190 body, and its one residual duplication.
中文说明
本 PR 做了什么
未读的 chunk 只说一次、挂在其原因之下。凡披露条目已解释"为何没被读"(launch 未发生、或发生在改写的 prompt 上)的 chunk,不再出现在正文的
nobody read them句中;chunk-nobody-readcap 与修复命令保留全量列表——只有发布的句子去重。没有已披露原因的 chunk(3A 形状:chunk 不是 roster requirement)保留该句,因为那是作者得知这些行未被读的唯一出处。为什么需要
#7190 分组落地后的第一条正文(#7166 上的 review 4730480309)把 17 个 chunk 说了两遍:一遍在
nobody read them、一遍在its prompt was built, but no agent…——后果与原因跨两个子句类别复述,是按 subject 去重未能覆盖的最后一个重复形状。验证方式
cd packages/cli && npx vitest run src/commands/review—— 31 文件 769 测试全绿。nobody read them缺席;cap 与 remediation 原样);无 launch 侧披露的 3A plan 保留收据句。既有 blind-launch 测试改钉去重后的形状(原因一次,cap 与修复随行)。证据(前后对照)
修复前(#7166 实况):17 个 chunk 同时出现在
nobody read them句与 not-launched 句。修复后,同一状态:这 17 个只在原因句出现;收据句只为无其他解释的 chunk 渲染。
风险与范围
cappedBy、remediation、event 决策未触碰(已钉)。chunk <id>subject 精确匹配——被任一原因(not-launched/rewritten/idle/blind/unopened)解释的 chunk 均让位于该原因。关联
展品:#7166 上的 review 4730480309——#7190 之后的第一条正文及其唯一残留重复。