diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 09084867be2..650c0e3106a 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -818,6 +818,56 @@ describe('the roster — who should have been here', () => { expect(r.missingRoles[0]).not.toMatch(/--role/); }); + it("keeps per-role entries when every prompt was built and none was launched — the collapse is compose's job", () => { + // The first cut collapsed this shape HERE, into one "the run stopped at + // the prompt builder" line — and misfired: candidatesOf is also all-empty + // when every agent ran on a REWRITTEN prompt, so the aggregate claimed + // nothing launched beside forty-three rewritten-launch disclosures that + // said otherwise. Coverage now reports per role, structurally + // (`disclosures`), and compose-review groups same-reason subjects into + // the one sentence — after the caller's echoes have been deduped against + // the very subjects a coverage-side collapse would have discarded. + const p = planPr(); + for (const f of readdirSync(join(dir, 'subagents', 'S1'))) { + rmSync(join(dir, 'subagents', 'S1', f), { force: true }); + } + transcript('stray', wholeDiff(), { calls: 8 }); + + const r = coverageFromTranscripts(p, ENV); + expect(r.ok).toBe(false); + const roster = requiredAgents( + JSON.parse(readFileSync(p, 'utf8')) as RosterPlan, + ); + expect(roster.length).toBeGreaterThan(1); + expect(r.missingRoles).toHaveLength(roster.length); + expect(r.missingRoleSelectors).toHaveLength(roster.length); + // Structural twins, one per role, all sharing the one reason — what the + // compose-side grouping turns into a single sentence. + const notLaunched = r.disclosures.filter( + (d) => + d.reason === + 'its prompt was built, but no agent on record was launched with it', + ); + expect(notLaunched).toHaveLength(roster.length); + expect(new Set(notLaunched.map((d) => d.subject)).size).toBe(roster.length); + }); + + it('keeps the per-role not-launched text when only SOME launches are missing', () => { + // The collapse must not swallow the partial case: one unlaunched role + // beside launched siblings is that role's own line, naming it. + const p = planPr(); + rmSync(join(dir, 'subagents', 'S1', 'agent-r-1c.jsonl'), { force: true }); + transcript('sec', wholeDiff(), { calls: 8 }); + + const r = coverageFromTranscripts(p, ENV); + const gap = r.missingRoles.join(' '); + expect(gap).toContain('Cross-file tracer'); + expect(gap).toContain( + 'its prompt was built, but no agent on record was launched with it', + ); + expect(gap).not.toContain('every dimension'); + }); + it('tells the operator where it looked, so a wrong --plan is not a missing file', () => { // "The builder never ran" and "the builder ran against a different --plan" reach // this check as the same thing: an absent record. They are fixed differently, so diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index b6d1da5bcf2..f9e3de7b3b8 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -421,9 +421,12 @@ describe('composeReview — 422 recovery (round-7 Critical #1 & round-6: verdict // Before the 422: S=2. After dropping both anchors: recompose. const r = composeReview(base({ suggestionsDiscarded: 2 })); expect(r.event).toBe('COMMENT'); + // Self-contained for the PR author — the old text said "see the terminal + // output", a terminal only the operator has. expect(r.body).toContain( - '2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.', + '2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.', ); + expect(r.body).not.toContain('terminal output'); // Nothing is inline — the body must not claim otherwise while the // discarded sentence says the opposite (round-9: `s` included discarded). expect(r.body).not.toContain('Suggestions are inline.'); @@ -1071,6 +1074,179 @@ describe('composeReviewCommand handler (the CLI glue)', () => { }); describe('coverage is recomputed, never accepted', () => { + it('does not repeat a disclosure the caller echoed back — one subject, one line', () => { + // #7188: the orchestrator pasted the gate's own gap sentences into + // `unreviewedDimensions`, coverage recomputed the same gaps, and the + // public body carried every disclosure twice — 22 "Not reviewed" clauses + // for 11 roles. The chunk list already dedupes by its `chunk ` + // prefix; the role list dedupes by label now, and when both sides name + // the same subject the coverage-derived text wins. + const p = plan(); + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + recordBuilt(p, 1); + recordBuilt(p, 2); + // test-matrix is required by this plan's roster and never built → exactly + // one coverage-derived role gap. + const label = 'Test coverage matrix (whole-diff)'; + const r = composeReview({ + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + `${label} — the run described this gap in its own words`, + 'a subject only the caller noticed — the auditor returned nothing twice', + ], + }); + // One clause for the shared subject — and it is the machine's sentence, + // not the caller's paraphrase. + expect(r.body.split(label)).toHaveLength(2); + expect(r.body).toContain('no record shows its brief reaching an agent'); + expect(r.body).not.toContain('described this gap in its own words'); + // A subject the coverage recomputation cannot see survives untouched. + expect(r.body).toContain( + 'a subject only the caller noticed — the auditor returned nothing twice', + ); + }); + + it('says a shared cause once, with every subject on the one sentence', () => { + // #7166's posted body: ninety-nine disclosure paragraphs over FOUR causes + // — forty-three chunks all rewritten, fifty-five roles all unlaunched — + // with the six real findings buried beneath. Same cause, one sentence. + const p = plan(); + // Both chunk launches rewritten: recorded prompts exist, the agents ran + // on hand-written prompts that DROP the brief line — an add-only wrap + // would rightly pass the delivery check. + recordBuilt(p, 1); + recordBuilt(p, 2); + transcript( + 'a1', + `You are reviewing chunk 1 of 2.\nread_file(file_path="${DIFF}", offset=0, limit=100)`, + { toolCalls: 2 }, + ); + transcript( + 'a2', + `You are reviewing chunk 2 of 2.\nread_file(file_path="${DIFF}", offset=100, limit=100)`, + { toolCalls: 2 }, + ); + const r = composeReview({ planPath: p, env: ENV, modelId: MODEL }); + const reason = 'launched with a prompt that is not the one the CLI built'; + // One clause for the shared cause — not one per chunk… + expect(r.body.split(reason)).toHaveLength(2); + // …and both subjects ride it. + expect(r.body).toMatch( + new RegExp(`Not reviewed: [^.]*chunk 1[^.]*chunk 2[^.]*— ${reason}\\.`), + ); + }); + + it('an all-rewritten roster never claims nothing launched — precise cause, no contradicting aggregate', () => { + // The first cut collapsed all-empty verbatim matches into "the run + // stopped at the prompt builder" — but candidatesOf is also all-empty + // when every agent RAN on a rewritten prompt, and the aggregate then + // contradicted the rewritten-launch disclosures beside it. Reproduced + // and refused: both chunks rewritten, the whole-diff role unlaunched — + // each cause its own sentence, no "every dimension" claim anywhere. + const p = plan(); + recordBuilt(p, 1); + recordBuilt(p, 2); + transcript( + 'a1', + `You are reviewing chunk 1 of 2.\nread_file(file_path="${DIFF}", offset=0, limit=100)`, + { toolCalls: 2 }, + ); + transcript( + 'a2', + `You are reviewing chunk 2 of 2.\nread_file(file_path="${DIFF}", offset=100, limit=100)`, + { toolCalls: 2 }, + ); + const r = composeReview({ planPath: p, env: ENV, modelId: MODEL }); + expect(r.body).toMatch( + /Not reviewed: [^.]*chunk 1[^.]*chunk 2[^.]*— launched with a prompt that is not the one the CLI built\./, + ); + expect(r.body).not.toContain('every dimension'); + expect(r.body).not.toContain('stopped at the prompt builder'); + // And the chunks appear under their PRECISE cause only — to the roster + // they are also requirements with no verbatim launch, and repeating them + // under that vaguer cause would claim nothing launched about agents that + // demonstrably ran. + expect(r.body).not.toContain('no agent on record was launched with it'); + }); + + it('a reason carrying its own em-dash neither garbles the subject nor duplicates the line', () => { + // Reasons are free-form — internal failures interpolate raw error + // messages — so a subject/reason boundary reparsed from rendered prose + // regroups exactly the entries it garbles. The entries are structural + // now; the caller's echo of a dashed line still dedupes, by prefix + // against the known subject. + const p = plan(); + const r = composeReview({ + planPath: p, + // Transcripts unreadable: the coverage AND verification reasons both + // interpolate an error message — with an em-dash of their own. + env: { + QWEN_CODE_PROJECT_DIR: join(dir, 'nowhere — missing'), + QWEN_CODE_SESSION_ID: 'S1', + }, + unreviewedDimensions: [ + 'coverage — could not read the transcripts — echoed back by the caller', + ], + modelId: MODEL, + }); + // One coverage clause — the caller's dashed echo deduped by subject + // prefix, the machine's own text rendered once, subject intact. + expect(r.body.match(/Not reviewed: coverage/g)).toHaveLength(1); + expect(r.body).not.toContain('echoed back by the caller'); + }); + + it('caller echoes of per-role gaps fold into the one grouped sentence — the #7188 shape end to end', () => { + // The coverage-side collapse discarded the per-role subjects before the + // caller's echoes could collide with them, so the body carried the + // caller's per-role sentences PLUS an overlapping aggregate. Per-role + // subjects now survive to the dedup, and the grouping makes the one + // sentence afterwards. + const p = plan(); + recordBuilt(p, 1); + recordBuilt(p, 2); + // Chunks reviewed properly; the whole-diff role built but never launched. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const label = 'Test coverage matrix (whole-diff)'; + const r = composeReview({ + planPath: p, + env: ENV, + unreviewedDimensions: [ + `${label} — its prompt was built, but no agent on record was launched with it`, + ], + modelId: MODEL, + }); + expect(r.body.split(label)).toHaveLength(2); + expect( + r.body.match(/no record shows its brief reaching an agent/g) ?? [], + ).toHaveLength(1); + }); + + 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 + // merge two files into one subject and silently drop a disclosure. + const p = plan(); + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + recordBuilt(p, 1); + recordBuilt(p, 2); + const r = composeReview({ + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + 'Invariant agent A: state, timers — src/a.ts — the agent whiffed twice', + 'Invariant agent A: state, timers — src/b.ts — the agent whiffed twice', + ], + }); + expect(r.body).toContain('src/a.ts'); + expect(r.body).toContain('src/b.ts'); + }); + it('caps when no plan is given — nothing can show the diff was read', () => { const r = composeReview({ criticalsInline: 0, diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index c28eeb74b38..92479da368c 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -204,6 +204,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { input.unreviewedDimensions, 'unreviewedDimensions', ); + // The coverage-derived disclosures, kept STRUCTURAL ({subject, reason}) + // from the site that knows the boundary — reparsing the rendered prose for + // it was the bug. `unreviewed` above stays what the caller wrote, verbatim. + const coverageEntries: Array<{ subject: string; reason: string }> = []; // The fixes for the gaps above, for stderr — never for the body. The gap says // what the review cannot certify, to the PR author; the remediation names the // command that repairs it, to the orchestrator. #7012's public body was fourteen @@ -241,10 +245,12 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // What it supplies is `planPath` — a path, whose contents the CLI wrote. The // transcripts are found from the environment the CLI exported. if (!input.planPath) { - unreviewed.push( - 'coverage — no plan was given, so this run cannot show that any of the ' + - 'diff was read', - ); + coverageEntries.push({ + subject: 'coverage', + reason: + 'no plan was given, so this run cannot show that any of the diff ' + + 'was read', + }); } else { try { const cov = coverageFromTranscripts(input.planPath, input.env); @@ -261,9 +267,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { if (!already) uncoverable.push(prefix); } for (const label of cov.idleAgents) { - unreviewed.push( - `${label} — the agent made no tool call: it read nothing`, - ); + coverageEntries.push({ + subject: label, + reason: 'the agent made no tool call: it read nothing', + }); } if (cov.idleAgents.length > 0) { remediation.push( @@ -280,10 +287,12 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // this line: the line lands in the posted body, and `qwen review // agent-prompt` is not something a PR author can run. for (const label of cov.blindAgents) { - unreviewed.push( - `${label} — launched with a prompt that never named the diff file, ` + - 'so it could not have read it', - ); + coverageEntries.push({ + subject: label, + reason: + 'launched with a prompt that never named the diff file, so it ' + + 'could not have read it', + }); } if (cov.blindAgents.length > 0) { remediation.push( @@ -298,10 +307,12 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // spent its run somewhere else, which on a diff with deletions means it // reviewed a file the removed lines are simply not in. for (const label of cov.unopenedAgents) { - unreviewed.push( - `${label} — pointed at diff lines it never opened: it made tool calls, ` + - 'but none of them read the diff', - ); + coverageEntries.push({ + subject: label, + reason: + 'pointed at diff lines it never opened: it made tool calls, but ' + + 'none of them read the diff', + }); } if (cov.unopenedAgents.length > 0) { remediation.push( @@ -317,9 +328,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // prompt that is not the one the CLI built`), so push the label as-is — // wrapping it in a second ` — ` clause read as one run-on sentence with two // dashes. Same for `missingRoles` below; `unreadBriefs` already did this. - for (const label of cov.rewrittenPrompts) { - unreviewed.push(label); - } + // rewritten, missing-role and unread-brief entries arrive structurally + // (`cov.disclosures`, push order preserved) — their labels can carry + // em-dashes of their own, which is why they are never reparsed here. + coverageEntries.push(...cov.disclosures); if (cov.rewrittenPrompts.length > 0) { remediation.push( 'rewritten launches: re-run `"${QWEN_CODE_CLI:-qwen}" review ' + @@ -333,9 +345,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // A dimension nobody reviewed. This is exactly what `unreviewedDimensions` // has always meant, arrived at from the plan instead of from the orchestrator // noticing — which, on the run that never launched Agent 0, it did not. - for (const label of cov.missingRoles) { - unreviewed.push(label); - } + if (cov.missingRoles.length > 0) { remediation.push( 'missing briefs: build every required prompt in one call — ' + @@ -347,9 +357,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { } // Launched, but never read the brief it was pointed at: it reviewed with no // dimension, no severity definitions and no project rules. - for (const label of cov.unreadBriefs) { - unreviewed.push(label); - } + if (cov.unreadBriefs.length > 0) { remediation.push( 'unread briefs: relaunch each agent with the same printed prompt — ' + @@ -368,9 +376,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { err instanceof TranscriptsUnavailableError ? `could not read the agents' transcripts (${err.message})` : `the plan could not be used (${(err as Error).message})`; - unreviewed.push( - `coverage — ${why}, so this run cannot show that any of the diff was read`, - ); + coverageEntries.push({ + subject: 'coverage', + reason: `${why}, so this run cannot show that any of the diff was read`, + }); } // Step 4 (verify) and Step 5 (reverse audit) ran, and read their briefs? @@ -394,13 +403,27 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { { postsFindings: findingsToVerify > 0 }, input.env, ); - for (const gap of verification.gaps) unreviewed.push(gap); + for (const gap of verification.gaps) { + // The machine's own two subjects ('verification', 'reverse audit'), + // dash-free by construction — the first separator is the boundary. + const cut = gap.indexOf(' — '); + coverageEntries.push( + cut === -1 + ? { subject: gap, reason: '' } + : { + subject: gap.slice(0, cut), + reason: gap.slice(cut + ' — '.length), + }, + ); + } remediation.push(...verification.remediation); } catch (err) { - unreviewed.push( - `verification — could not check that Step 4 and Step 5 ran ` + + coverageEntries.push({ + subject: 'verification', + reason: + `could not check that Step 4 and Step 5 ran ` + `(${(err as Error).message})`, - ); + }); } } const contextUnavailable = toBool( @@ -451,7 +474,9 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { if (cannotTell.length > 0) cappedBy.push('cannot-tell-existing-critical'); if (missingReceipts.length > 0) cappedBy.push('chunk-nobody-read'); if (uncoverable.length > 0) cappedBy.push('uncoverable-chunk'); - if (unreviewed.length > 0) cappedBy.push('unreviewed-dimension'); + if (unreviewed.length + coverageEntries.length > 0) { + cappedBy.push('unreviewed-dimension'); + } if (contextUnavailable) cappedBy.push('context-unavailable'); let event: ReviewEvent = baseEvent; @@ -504,19 +529,70 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { `Not reviewed: ${uncoverable.join(', ')} — a line there exceeds the read limit.`, ); } - // Bare dimension names share the whiffed-agent explanation; an entry that - // brought its own reason (after an em-dash) must not have the whiff - // sentence appended to it — that would misstate why it went unreviewed. - const whiffedDimensions = unreviewed.filter((d) => !d.includes(' — ')); - const explainedDimensions = unreviewed.filter((d) => d.includes(' — ')); + // One disclosure per subject, one sentence per cause — structurally, not by + // reparsing prose. The first cut recovered a subject/reason boundary from + // the rendered text (the last ` — ` segment), and a reason is free-form: + // an invariant label carries a dash for its file, an error interpolation + // can carry anything, and a boundary guessed wrong regroups the entries it + // garbles. Coverage now hands the entries over as `{subject, reason}` + // pairs; only the CALLER\'s entries are prose, and those are never parsed — + // they are matched against known coverage subjects by prefix (exactly how + // the chunk list above dedupes), and rendered verbatim when nothing + // matches. A run that pasted the gate\'s own gap lines into its input + // posted every disclosure twice — 22 clauses for 11 roles on a public PR + // (#7188) — and the coverage-derived text wins the collision: it is the + // evidence-bounded register this body is written in. + const covEntries = coverageEntries; + const callerLeft: string[] = []; + const seenCaller = new Set(); + for (const d of unreviewed) { + if (seenCaller.has(d)) continue; // a caller pasting itself twice + seenCaller.add(d); + const echoesCoverage = covEntries.some( + (e) => d === e.subject || d.startsWith(`${e.subject} — `), + ); + if (!echoesCoverage) callerLeft.push(d); + } + // Bare caller names share the whiffed-agent explanation; an entry that + // brought its own reason (after an em-dash) is rendered verbatim, its own + // line — unparsed, ungrouped, because its structure is not ours to guess. + const whiffedDimensions = callerLeft.filter((d) => !d.includes(' — ')); + const explainedCaller = callerLeft.filter((d) => d.includes(' — ')); if (whiffedDimensions.length > 0) { notReviewedParts.push( `Not reviewed: ${whiffedDimensions.join(', ')} — the agent returned no evidence of its walk twice.`, ); } - for (const d of explainedDimensions) { + for (const d of explainedCaller) { notReviewedParts.push(`Not reviewed: ${d}.`); } + // Same cause, one sentence: forty-three chunks launched with rewritten + // prompts are one failure with forty-three subjects, not forty-three + // paragraphs — a posted body on #7166 was ninety-nine clauses over four + // causes, the six real findings buried beneath. Grouped by the reason + // STRING, so a reason embedding per-subject detail (an unread brief\'s own + // path) differs per entry and keeps its own line. One subject that appears + // under two causes keeps the FIRST — the categories push in precision + // order, and a chunk flagged `rewritten` is also, to the roster, a + // requirement with no verbatim launch; repeating it under the later, vaguer + // cause would tell the author "no agent was launched" about an agent that + // demonstrably ran. + const seenSubjects = new Set(); + const byReason = new Map(); + for (const { subject, reason } of covEntries) { + if (seenSubjects.has(subject)) continue; + seenSubjects.add(subject); + const subjects = byReason.get(reason) ?? []; + subjects.push(subject); + byReason.set(reason, subjects); + } + for (const [reason, subjects] of byReason) { + notReviewedParts.push( + reason + ? `Not reviewed: ${subjects.join(', ')} — ${reason}.` + : `Not reviewed: ${subjects.join(', ')}.`, + ); + } // Clause 5 — blockers the review could neither confirm nor clear. They // survive every event shape: erasing one is how a review approves the @@ -599,7 +675,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { c === 0 && cannotTell.length === 0 && uncoverable.length === 0 && - unreviewed.length === 0 && + unreviewed.length + coverageEntries.length === 0 && // A missing receipt caps the event but was left out of certification, so a // body could open "Reviewed — no blockers." two lines above "nobody read // them." Nothing nobody read can be certified blocker-free. @@ -614,8 +690,14 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // above.) if (suggestionsInline > 0) clauses.push('Suggestions are inline.'); if (suggestionsDiscarded > 0) { + // Self-contained: this lands in the posted body, and "see the terminal + // output" pointed the PR author at a terminal only the operator has — + // eight hours of real bot reviews carried that dead reference on five + // different pull requests. clauses.push( - `${suggestionsDiscarded} Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.`, + `${suggestionsDiscarded} Suggestion-level finding(s) could not be ` + + `anchored to a changed line and were dropped; nothing further to act ` + + `on here.`, ); } diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index fbb96d99b71..eadeb0ec507 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -140,6 +140,17 @@ export interface CoverageFromTranscripts { uncoverableChunks: number[]; /** Chunk ids a working agent actually reviewed. */ coveredChunks: number[]; + /** + * The pre-formed disclosure entries (`rewrittenPrompts`, `missingRoles`, + * `unreadBriefs`), as `{subject, reason}` pairs in push order — for + * `compose-review`, which dedupes caller echoes by subject and groups + * same-reason subjects into one sentence. The prose twins above remain for + * the stderr formatting; REPARSING them was the bug: a reason is free-form + * text (labels carry ` — ` for an invariant's file, error interpolations + * can carry anything), so a subject/reason boundary recovered from rendered + * prose garbles exactly the entries it matters for. + */ + disclosures: Array<{ subject: string; reason: string }>; } /** The plan, as far as coverage needs it. The roster reads more of it — see RosterPlan. */ @@ -285,6 +296,15 @@ export function coverageFromTranscripts( const idleAgents: string[] = []; const unopenedAgents: string[] = []; const rewrittenPrompts: string[] = []; + const disclosures: Array<{ subject: string; reason: string }> = []; + // The one source for both registers: the structural entry feeds the posted + // body (compose-review), and the returned prose feeds the stderr arrays — + // maintained as a pair, an edit to one and not the other would silently + // diverge what the operator reads from what the author was told. + const disclose = (subject: string, reason: string): string => { + disclosures.push({ subject, reason }); + return `${subject} — ${reason}`; + }; const covered = new Set(); const uncoverable = new Set(); @@ -392,15 +412,21 @@ export function coverageFromTranscripts( rewrittenThisRecord = true; if (!nothingBuiltAtAll && !superseded(rec, chunk)) { rewrittenPrompts.push( - `${name} — ran on a prompt the run wrote itself (none was built for ` + - `this chunk), so the brief with its method and rules never reached it`, + disclose( + name, + 'ran on a prompt the run wrote itself (none was built for this ' + + 'chunk), so the brief with its method and rules never reached it', + ), ); } } else if (!wasDeliveredVerbatim(rec.launchPrompt, b)) { rewrittenThisRecord = true; if (!superseded(rec, chunk)) { rewrittenPrompts.push( - `${name} — launched with a prompt that is not the one the CLI built`, + disclose( + name, + 'launched with a prompt that is not the one the CLI built', + ), ); } } @@ -483,11 +509,14 @@ export function coverageFromTranscripts( // Phrased to read under the `Not reviewed: ` prefix `compose-review` renders it // with, which is where a PR author meets it. missingRoles.push( - `every dimension — none of the ${roster.length} required agents is on ` + - `record as launched with a prompt this skill built, so this diff was ` + - `reviewed, if at all, from prompts the run wrote for itself: no record ` + - `shows the severity bar, the finding format or this project's own rules ` + - `reaching an agent`, + disclose( + 'every dimension', + `none of the ${roster.length} required agents is on record as ` + + `launched with a prompt this skill built, so this diff was ` + + `reviewed, if at all, from prompts the run wrote for itself: no ` + + `record shows the severity bar, the finding format or this ` + + `project's own rules reaching an agent`, + ), ); } @@ -547,9 +576,11 @@ export function coverageFromTranscripts( if (b === undefined) { if (!nobodyBuiltAnything) { missingRoles.push( - `${roleLabel(req)} — no record shows its brief reaching an agent, so ` + - `this dimension was reviewed, if at all, from a prompt the run ` + - `wrote for itself`, + disclose( + roleLabel(req), + 'no record shows its brief reaching an agent, so this dimension ' + + 'was reviewed, if at all, from a prompt the run wrote for itself', + ), ); } missingRoleSelectors.push(selectorOf(req)); @@ -562,12 +593,15 @@ export function coverageFromTranscripts( // shortage of transcripts, not an artifact of claim order. const anyMatch = candidatesOf[buildableIdx].length > 0; missingRoles.push( - anyMatch - ? `${roleLabel(req)} — its prompt reached only an agent already ` + - `credited with another block; one agent was given several blocks, ` + - `and one transcript cannot certify two dimensions` - : `${roleLabel(req)} — its prompt was built, but no agent on record ` + - `was launched with it`, + disclose( + roleLabel(req), + anyMatch + ? 'its prompt reached only an agent already credited with ' + + 'another block; one agent was given several blocks, and one ' + + 'transcript cannot certify two dimensions' + : 'its prompt was built, but no agent on record was launched ' + + 'with it', + ), ); missingRoleSelectors.push(selectorOf(req)); continue; @@ -597,8 +631,11 @@ export function coverageFromTranscripts( ); if (!opened) { unreadBriefs.push( - `${roleLabel(req)} — never opened its brief (${brief}), so it reviewed ` + - 'without the instructions it was launched to follow', + disclose( + roleLabel(req), + `never opened its brief (${brief}), so it reviewed without the ` + + 'instructions it was launched to follow', + ), ); } } @@ -628,6 +665,7 @@ export function coverageFromTranscripts( rewrittenPrompts, missingRoles, missingRoleSelectors, + disclosures, unreadBriefs, missingChunks, uncoverableChunks: [...uncoverable].sort((a, b) => a - b),