diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index e33f95b2aa4..cb7ac40d2a6 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -1238,6 +1238,52 @@ describe('coverage is recomputed, never accepted', () => { ).toHaveLength(1); }); + it('a chunk whose launch failure is already disclosed leaves the nobody-read sentence — cause, not consequence twice', () => { + // #7166's first post-grouping body carried seventeen chunks in BOTH the + // "nobody read them" sentence and the not-launched roster sentence: the + // consequence restated beside its cause. The cap and remediation keep the + // full list; only the posted sentence dedupes. + const p = plan(); + recordBuilt(p, 1); + recordBuilt(p, 2); + // chunk 2 reviewed properly; chunk 1 built and never launched — its + // territory therefore unread, and its cause on record. + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const r = composeReview({ planPath: p, env: ENV, modelId: MODEL }); + expect(r.cappedBy).toContain('chunk-nobody-read'); // the cap keeps the fact + expect(r.remediation.join(' ')).toContain('chunks nobody read'); + expect(r.body).toContain('chunk 1'); + // …but only under its cause: no second sentence restating the consequence. + expect(r.body).not.toContain('nobody read them'); + }); + + it('keeps the nobody-read sentence for a chunk with no disclosed cause', () => { + // The 3A shape: chunks are not roster requirements, so an unread chunk has + // no launch-side disclosure to explain it — the receipt sentence is the + // only place the author learns those lines went unread. + const p = join(dir, 'plan-3a.json'); + writeFileSync( + p, + JSON.stringify({ + diffPathAbsolute: DIFF, + srcDiffLines: 100, + diffLines: 200, + files: [ + { path: 'a.ts', kind: 'source', removedLines: 0, heavy: false }, + ], + chunks: [ + { id: 1, startLine: 1, endLine: 100 }, + { id: 2, startLine: 101, endLine: 200 }, + ], + }), + ); + const old = new Date(2020, 0, 1); + utimesSync(p, old, old); + const r = composeReview({ planPath: p, env: ENV, modelId: MODEL }); + expect(r.body).toContain('nobody read them'); + expect(r.body).toMatch(/chunk 1, chunk 2 — no agent reported covering/); + }); + it('does not merge two invariant files under one label — the em-dash is part of the subject', () => { // An invariant agent's label legitimately carries an em-dash segment // (`Invariant agent A … — src/foo.ts`). A first-dash dedup key would @@ -1311,9 +1357,12 @@ describe('coverage is recomputed, never accepted', () => { ); expect(r.remediation.join(' ')).toMatch(/do not relaunch the old prompt/); // Blind agents read nothing, so the chunks they owned are also chunks - // nobody read — that disclosure's repair must ride along too. Deleting the - // missingReceipts push used to fail no test: no fixture reached it. - expect(r.body).toContain('no agent reported covering'); + // nobody read — the CAP and the repair ride along, while the posted body + // says it once, under the cause: the blind sentence already explains the + // unread territory, and restating it as "nobody read them" beside it was + // the #7166 double-disclosure. + expect(r.cappedBy).toContain('chunk-nobody-read'); + expect(r.body).not.toContain('no agent reported covering'); expect(r.remediation.join(' ')).toMatch( /chunks nobody read: build each with/, ); diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index 1ced52d693b..255d12ed1aa 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -576,11 +576,26 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // as a line too long to read, which is true of an *uncoverable* chunk and a // fabrication about one nobody receipted — the author would be told the diff // defeated the reader, when in fact no reader turned up. - notReviewedParts.push( - `Not reviewed: ${missingReceipts - .map((id) => `chunk ${id}`) - .join(', ')} — no agent reported covering these; nobody read them.`, + // + // But a chunk whose disclosure entry already says WHY it went unread — its + // launch never happened, or happened on a rewritten prompt — is one fact, + // not two: "nobody read chunk 2" beside "chunk 2 — its prompt was built, + // but no agent on record was launched with it" restates the consequence + // next to its cause, and #7166's first post-grouping body carried + // seventeen chunks twice exactly this way. The cap and the remediation + // above keep the FULL list — only the posted sentence dedupes, and only + // for subjects another sentence already explains. + const disclosedSubjects = new Set(coverageEntries.map((e) => e.subject)); + const unexplainedReceipts = missingReceipts.filter( + (id) => !disclosedSubjects.has(`chunk ${id}`), ); + if (unexplainedReceipts.length > 0) { + notReviewedParts.push( + `Not reviewed: ${unexplainedReceipts + .map((id) => `chunk ${id}`) + .join(', ')} — no agent reported covering these; nobody read them.`, + ); + } } if (uncoverable.length > 0) { notReviewedParts.push(