diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index 6a56af410cd..36bc911beab 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -28,7 +28,11 @@ import { } from './lib/deadline.js'; import { getGhHost, setGhHost } from './lib/gh.js'; import { BRIEFS } from './lib/agent-briefs.js'; -import { LEDGER_MAX_ROUND, parseLedger } from './lib/ledger.js'; +import { + LEDGER_MAX_ROUND, + LEDGER_MAX_VOLUME, + parseLedger, +} from './lib/ledger.js'; import { countInlineFindings } from './lib/inline-counts.js'; import { composeReview, @@ -8750,6 +8754,22 @@ describe('convergence telemetry — volume, carried in the marker', () => { expect(parseLedger(r.body)?.posted).toBe(3); }); + it('clamps the count at its origin, so every surface agrees', () => { + // The terminal line, the marker and the artifact must never disagree + // about one round's count. This is the defensive over-cap case: the + // reader is applied where the number is derived, not only where it is + // written, so no surface can see the raw value. + const r = composeReview({ + planPath: plan(), + modelId: 'm', + criticalsInline: 0, + suggestionsInline: LEDGER_MAX_VOLUME + 5, + draftedComments: drafts(LEDGER_MAX_VOLUME + 5), + }); + expect(r.postedInline).toBe(LEDGER_MAX_VOLUME); + expect(parseLedger(r.body)?.posted).toBe(LEDGER_MAX_VOLUME); + }); + it('counts the POST-enforcement set — what submit actually sends', () => { // The floor moves two of the three out of the posting set, so the // recorded volume is one. Counting the drafts would record a number no diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index d78879fbd3b..3b4a1f649dd 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -1543,8 +1543,14 @@ function composeReviewBody( ): ComposeReviewResult { // The posting set this body describes — `input` here is already the // post-enforcement one, so the count needs no second derivation and - // cannot disagree with the marker's. - const postedInline = (input.draftedComments ?? []).length; + // cannot disagree with the marker's. Clamped AT THE ORIGIN through the + // shared reader: every other site that reads a volume applies it, and the + // one that did not was this count on its way to the terminal line, which + // in the defensive over-cap case would have printed the raw number beside + // a marker recording the capped one — the two-outputs-disagree failure + // the shared reader's own docstring exists to prevent. `?? 0` is + // unreachable for an array length; it keeps the type honest. + const postedInline = volumeOf((input.draftedComments ?? []).length) ?? 0; const criticalsInline = toCount(input.criticalsInline, 'criticalsInline'); const suggestionsInline = toCount( input.suggestionsInline, diff --git a/packages/cli/src/commands/review/save-artifact.test.ts b/packages/cli/src/commands/review/save-artifact.test.ts index c4c5d1ab9ab..eeff2f41695 100644 --- a/packages/cli/src/commands/review/save-artifact.test.ts +++ b/packages/cli/src/commands/review/save-artifact.test.ts @@ -69,9 +69,17 @@ const verdict = { deferredCount: 2, // Non-empty for the same reason — absent defaults to []. floorEnforced: [1], - // Non-zero for the same reason — absent defaults to 0. + // Non-zero on purpose too, but for the OPPOSITE reason to its siblings: + // this field's absence is preserved, not defaulted, so the fixture value + // proves passthrough against a validator that would otherwise omit the + // field entirely. postedInline: 3, - // Also non-default on purpose, for the same reason. + // Also non-default on purpose — and on the DEFAULTING side, with + // `deferredCount` and `floorEnforced`: an absent `bodyTrim` reads as an + // untrimmed one and the field is always emitted. Spelled out rather than + // said as "the same reason", which would now resolve against the + // preserved-absence block above it and teach the opposite of what + // `save-artifact.ts` does. bodyTrim: { sections: 2, deferralList: true, fold: true, truncated: true }, lowSignal: { agents: 4, srcDiffLines: 120 }, verdictLine: 'Verdict: Comment — Request changes was downgraded',