feat(review): let an operator lower the reverse-audit round cap

packages/cli/src/commands/review/capture-local.ts

@@ -31,6 +31,7 @@ import {
   stringifyPlanReport,
   type PlanReport,
 } from './lib/report.js';
+import { operatorReviewSettings } from './lib/review-settings.js';
 
 interface CaptureLocalArgs {
   out: string;
@@ -94,7 +95,7 @@ function runCaptureLocal(args: CaptureLocalArgs): void {
     // No ref to `git show` a pre-change file out of, so per-file line counts and
     // heaviness are unavailable — same as `plan-diff`. Chunk coverage, which is
     // what the topology needs, is not.
-    ...buildPlanReport(plan, null),
+    ...buildPlanReport(plan, null, operatorReviewSettings().reverseAuditRounds),
     untrackedFiles: capture.untracked,
     skippedFiles: capture.skipped,
     ...planEffortField(args.effort),

packages/cli/src/commands/review/fetch-pr.test.ts

@@ -248,6 +248,10 @@ vi.mock('node:child_process', async (importOriginal) => {
 vi.mock('../../utils/stdioHelpers.js', () => ({
   writeStdoutLine: vi.fn(),
   writeStderrLine: producerMocks.writeStderrLine,
+  // The settings fallback announces through the SAFE writer; this mock is a
+  // partial one, so an export it does not list is a load-time failure for
+  // every test in the file.
+  writeStderrLineSafe: producerMocks.writeStderrLine,
 }));
 
 vi.mock('../../services/review-worktree-lease.js', () => ({

packages/cli/src/commands/review/fetch-pr.ts

@@ -54,6 +54,7 @@ import {
   stringifyPlanReport,
 } from './lib/report.js';
 import { resolveMergeBase, type GitProbe } from './lib/merge-base.js';
+import { operatorReviewSettings } from './lib/review-settings.js';
 
 interface PrMetadata {
   headRefName: string;
@@ -430,7 +431,11 @@ async function runFetchPr(args: FetchPrArgs): Promise<void> {
     diffPath,
     diffPathAbsolute,
     prDescriptionHasHan: /\p{Script=Han}/u.test(meta.body ?? ''),
-    ...buildPlanReport(plan, (path) => fileLineCount(fetchedSha, path)),
+    ...buildPlanReport(
+      plan,
+      (path) => fileLineCount(fetchedSha, path),
+      operatorReviewSettings().reverseAuditRounds,
+    ),
     ...planEffortField(args.effort),
   };
 

packages/cli/src/commands/review/lib/budget.test.ts

@@ -13,6 +13,7 @@ import {
   stripBudgetGapLines,
   launchToolBudget,
   reverseAuditRoundCap,
+  cappedRoundTier,
   reviewBudget,
 } from './budget.js';
 
@@ -673,9 +674,12 @@ describe('reverseAuditRoundCap — the one reader of the plan field', () => {
 
   it('reads absent, out-of-band and garbled values as the tier', () => {
     // The range is floored at HUGE_REVERSE_AUDIT_ROUNDS (3) — the smallest
-    // cap the CLI writes — so 1 and 2 read as the tier, not as themselves:
-    // honouring them would force a non-converged round-cap stop where the
-    // full loop would have kept auditing.
+    // cap the CLI writes — so 1 and 2 read as the tier, not as themselves.
+    // Not because either always forces a non-converged stop: an all-dry loop
+    // DOES converge under a cap of two, since the convergence check runs
+    // before the cap gate. One cannot converge at all (it refuses the pair's
+    // second member), and two leaves no round for a loop that reports
+    // anything — so neither buys a cheaper review, only a capped verdict.
     for (const bad of [0, 1, 2, 2.5, '1', null] as unknown[]) {
       expect(
         reverseAuditRoundCap({ ...SMALL, budget: { reverseAuditRounds: bad } }),
@@ -802,3 +806,71 @@ describe('reviewBudget — the budget survives the trip through the plan', () =>
     ).toBe(30);
   });
 });
+
+describe('cappedRoundTier — the operator ceiling may only lower a tier', () => {
+  const SMALL = { srcDiffLines: 100, diffLines: 100 };
+  const LARGE = { srcDiffLines: 600, diffLines: 1000 };
+  const HUGE = { srcDiffLines: 5000, diffLines: 5000 };
+
+  it('lowers each tier to what the operator asked for', () => {
+    expect(cappedRoundTier(SMALL, 4)).toBe(4);
+    expect(cappedRoundTier(SMALL, 3)).toBe(3);
+    expect(cappedRoundTier(LARGE, 3)).toBe(3);
+    expect(cappedRoundTier(SMALL, 9)).toBe(9);
+  });
+
+  it('REFUSES to raise any tier — the asymmetry is the whole knob', () => {
+    // A single operator-chosen count is what tiering removed: it is wrong for
+    // at least one topology, and most wrong for the one whose cap exists to
+    // stop six-hour reviews that post nothing. 20 buys nothing anywhere.
+    expect(cappedRoundTier(HUGE, 5)).toBe(3);
+    expect(cappedRoundTier(HUGE, 20)).toBe(3);
+    expect(cappedRoundTier(LARGE, 10)).toBe(5);
+    expect(cappedRoundTier(SMALL, 20)).toBe(10);
+    // Equal to the tier is not a lowering either — it changes nothing, and
+    // reading it as "honoured" would make a later tier change silently pinned
+    // to a number the operator picked against a different tier.
+    expect(cappedRoundTier(SMALL, 10)).toBe(10);
+  });
+
+  it('refuses a ceiling below the convergence minimum', () => {
+    // Neither one nor two buys a cheaper review, though not for the same
+    // reason: one refuses the convergence pair's second member so the loop can
+    // never reach two dry audits, while two lets an all-dry loop converge (the
+    // convergence check runs before the cap gate) but leaves no round for a
+    // loop that reports anything. Both end in a capped verdict.
+    for (const bad of [0, 1, 2, -3]) {
+      expect(cappedRoundTier(SMALL, bad)).toBe(10);
+      expect(cappedRoundTier(HUGE, bad)).toBe(3);
+    }
+  });
+
+  it('ignores a ceiling that is not a whole number', () => {
+    for (const bad of [3.5, Number.NaN, Number.POSITIVE_INFINITY] as number[]) {
+      expect(cappedRoundTier(SMALL, bad)).toBe(10);
+    }
+    expect(cappedRoundTier(SMALL, undefined)).toBe(10);
+  });
+
+  it('lowers what reviewBudget RECORDS, so every reader sees one number', () => {
+    // The setting has to reach the plan, not the gate: `reverseAuditRoundCap`
+    // clamps a stored value into the tier band, and a lowered value is inside
+    // it, so the reader honours it with no knowledge of the setting at all.
+    const b = reviewBudget({ srcDiffLines: 100, diffLines: 100 }, 4);
+    expect(b.reverseAuditRounds).toBe(4);
+    expect(reverseAuditRoundCap({ ...SMALL, budget: b })).toBe(4);
+    // …and an unset ceiling records the tier, exactly as before this setting.
+    expect(
+      reviewBudget({ srcDiffLines: 100, diffLines: 100 }).reverseAuditRounds,
+    ).toBe(10);
+  });
+
+  it('does not let the ceiling touch any other budget field', () => {
+    const plain = reviewBudget({ srcDiffLines: 900, diffLines: 900 });
+    const capped = reviewBudget({ srcDiffLines: 900, diffLines: 900 }, 3);
+    expect({ ...capped, reverseAuditRounds: 0 }).toEqual({
+      ...plain,
+      reverseAuditRounds: 0,
+    });
+  });
+});

packages/cli/src/commands/review/lib/budget.ts

@@ -257,6 +257,50 @@ export function reverseAuditRoundTier(size: DiffSize): number {
     : SMALL_REVERSE_AUDIT_ROUNDS;
 }
 
+/**
+ * The round cap to record, given the topology tier and what the operator asked
+ * for — **the operator may only lower it.**
+ *
+ * The asymmetry is the whole design of this knob, and it is not timidity about
+ * letting people configure things. Raising is refused because a single
+ * configurable number is precisely what tiering removed: a round is one agent
+ * on a small diff and ~90 minutes on a huge one, so one operator-chosen count
+ * is wrong for at least one topology, and the topology it is most wrong for is
+ * the one whose cap exists to stop six-hour reviews that post nothing.
+ * Lowering carries no such hazard — it can only end the loop sooner.
+ *
+ * The two ways an operator actually means "run it longer" both have direct
+ * expressions elsewhere, and neither is a round count: "I have more wall clock
+ * than the huge tier assumes" is a review deadline, which the admission gate
+ * already prices a round against; "keep going while it is still finding real
+ * defects" is a property of the findings, not of a number chosen in advance.
+ *
+ * Below `HUGE_REVERSE_AUDIT_ROUNDS` is refused too, for the reason
+ * `reverseAuditRoundCap` refuses it in a plan — though not the reason an
+ * earlier draft of this gave. A cap of **one** refuses the convergence pair's
+ * second member, so the loop cannot produce the two dry audits convergence is
+ * defined by and every run stops non-converged. A cap of **two** does let an
+ * all-dry loop converge (the convergence check runs before the cap gate), but
+ * it leaves no round at all for a loop that reports anything, so the first
+ * finding makes the stop non-converged. Either way the purchase is a capped
+ * verdict rather than a cheaper review.
+ */
+export function cappedRoundTier(
+  size: DiffSize,
+  operatorCap: number | undefined,
+): number {
+  const tier = reverseAuditRoundTier(size);
+  if (
+    typeof operatorCap !== 'number' ||
+    !Number.isInteger(operatorCap) ||
+    operatorCap < HUGE_REVERSE_AUDIT_ROUNDS ||
+    operatorCap >= tier
+  ) {
+    return tier;
+  }
+  return operatorCap;
+}
+
 /**
  * A line count this module is willing to size a plan from: a real, finite,
  * non-negative `number`. Everything else — absent, `null`, a numeric string, a
@@ -314,8 +358,19 @@ const LINES_PER_TOOL_CALL = 20;
  * that lands on its floor costs one under-walked small diff. It fails toward the
  * cheap end on purpose — the floors are the *minimum* work, not the maximum, so
  * a garbled input still walks three angles and still verifies.
+ *
+ * `operatorRoundCap` is the standing `review.reverseAuditRounds` setting, read
+ * by the capture command and passed in rather than read here — this module has
+ * no imports, and a budget that loaded settings would make every caller's tests
+ * depend on the machine's own `~/.qwen`. It can only lower the round tier; see
+ * `cappedRoundTier`. Nothing else in the budget is operator-tunable, and that
+ * stays true: the rest of these fields size the work a review owes, and a
+ * caller who can shrink them is a caller who shrinks them.
  */
-export function reviewBudget(input: BudgetInput): ReviewBudget {
+export function reviewBudget(
+  input: BudgetInput,
+  operatorRoundCap?: number,
+): ReviewBudget {
   const src = sane(input.srcDiffLines);
   const total = sane(input.diffLines);
 
@@ -350,7 +405,7 @@ export function reviewBudget(input: BudgetInput): ReviewBudget {
     // tier from it would record the SMALL tier's ten rounds for a plan whose
     // size failed to arrive, where the flat cap recorded five. The tier does
     // its own usability check precisely so this call can hand it the truth.
-    reverseAuditRounds: reverseAuditRoundTier(input),
+    reverseAuditRounds: cappedRoundTier(input, operatorRoundCap),
   };
 }
 
@@ -388,9 +443,9 @@ export function reviewBudget(input: BudgetInput): ReviewBudget {
  *
  * The range stays floored at `HUGE_REVERSE_AUDIT_ROUNDS`, the smallest cap
  * the CLI ever writes. A value of one or two is out of band (a hand-edited
- * plan): honouring it would force a non-converged round-cap stop where the
- * full loop would have kept auditing, so it too falls back to the tier —
- * never less.
+ * plan): one cannot reach convergence at all, and two leaves no round for a
+ * loop that reports anything — see `cappedRoundTier` for why neither buys a
+ * cheaper review. Both fall back to the tier, never less.
  */
 export function reverseAuditRoundCap(plan: unknown): number {
   const tier = reverseAuditRoundTier((plan ?? {}) as DiffSize);

packages/cli/src/commands/review/lib/report.test.ts

@@ -47,10 +47,14 @@ describe('buildPlanReport', () => {
   it('resolves the post-image through the injected dependency', () => {
     const plan = buildDiffPlan(editFile('src/a.ts', 3, 2), 400);
     const asked: string[] = [];
-    const report = buildPlanReport(plan, (p) => {
-      asked.push(p);
-      return 1000;
-    });
+    const report = buildPlanReport(
+      plan,
+      (p) => {
+        asked.push(p);
+        return 1000;
+      },
+      undefined,
+    );
     expect(asked).toEqual(['src/a.ts']);
     expect(report.files[0].fileLines).toBe(1000);
     // pre = post - added + removed
@@ -61,16 +65,20 @@ describe('buildPlanReport', () => {
     // 900 added lines into a file that ends up 1 000 long: it existed at 100
     // lines, so it is not "large enough before" — not heavy.
     const plan = buildDiffPlan(editFile('src/a.ts', 3, 900), 400);
-    expect(buildPlanReport(plan, () => 1000).files[0].heavy).toBe(false);
+    expect(buildPlanReport(plan, () => 1000, undefined).files[0].heavy).toBe(
+      false,
+    );
     // Same change into a file that ends up 6 000 long: it existed at 5 100,
     // and 900 changed lines clears the volume threshold.
-    expect(buildPlanReport(plan, () => 6000).files[0].heavy).toBe(true);
+    expect(buildPlanReport(plan, () => 6000, undefined).files[0].heavy).toBe(
+      true,
+    );
   });
 
   it('treats a null resolver as "no tree to read", so nothing is heavy', () => {
     // `plan-diff` has a bare diff file and no ref. It must not guess.
     const plan = buildDiffPlan(addFile('src/big.ts', 2000), 400);
-    const report = buildPlanReport(plan, null);
+    const report = buildPlanReport(plan, null, undefined);
     expect(report.files[0].fileLines).toBe(0);
     expect(report.files[0].preLines).toBe(0);
     expect(report.files[0].heavy).toBe(false);
@@ -84,19 +92,25 @@ describe('buildPlanReport', () => {
       'Binary files a/logo.png and b/logo.png differ',
     ].join('\n');
     const asked: string[] = [];
-    const report = buildPlanReport(buildDiffPlan(diff, 400), (p) => {
-      asked.push(p);
-      return 500;
-    });
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      (p) => {
+        asked.push(p);
+        return 500;
+      },
+      undefined,
+    );
     expect(asked).toEqual([]);
     expect(report.files[0].binary).toBe(true);
     expect(report.files[0].heavy).toBe(false);
   });
 
   it('emits addedRanges only on heavy files', () => {
     const diff = editFile('src/heavy.ts', 3, 900) + addFile('src/light.ts', 20);
-    const report = buildPlanReport(buildDiffPlan(diff, 400), (p) =>
-      p === 'src/heavy.ts' ? 6000 : 30,
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      (p) => (p === 'src/heavy.ts' ? 6000 : 30),
+      undefined,
     );
     const heavy = report.files.find((f) => f.path === 'src/heavy.ts')!;
     const light = report.files.find((f) => f.path === 'src/light.ts')!;
@@ -121,7 +135,11 @@ describe('buildPlanReport', () => {
       '-gone2',
       '',
     ].join('\n');
-    const report = buildPlanReport(buildDiffPlan(diff, 400), () => 100);
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      () => 100,
+      undefined,
+    );
     expect(report.files[0].hunks).toEqual([{ newStart: 1, newEnd: 2 }]);
   });
 
@@ -130,7 +148,11 @@ describe('buildPlanReport', () => {
     // `clearTimeout()` leaves nothing behind. This range points it at the `-`
     // lines that are the only evidence the call ever existed.
     const diff = editFile('src/heavy.ts', 3, 900);
-    const report = buildPlanReport(buildDiffPlan(diff, 400), () => 6000);
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      () => 6000,
+      undefined,
+    );
     const f = report.files[0];
     expect(f.heavy).toBe(true);
     expect(f.diffRange).toEqual({
@@ -143,6 +165,7 @@ describe('buildPlanReport', () => {
     const report = buildPlanReport(
       buildDiffPlan(addFile('src/a.ts', 20), 400),
       () => 30,
+      undefined,
     );
     expect(report.files[0].heavy).toBe(false);
     expect(report.files[0].diffRange).toBeUndefined();
@@ -155,7 +178,7 @@ describe('buildPlanReport', () => {
       addFile('docs/g.md', 30) +
       addFile('package-lock.json', 40);
     const plan = buildDiffPlan(diff, 400);
-    const report = buildPlanReport(plan, () => 100);
+    const report = buildPlanReport(plan, () => 100, undefined);
     expect(report.srcDiffLines).toBe(plan.srcDiffLines);
     expect(report.testDiffLines).toBe(plan.testDiffLines);
     expect(report.docsDiffLines).toBe(plan.docsDiffLines);
@@ -167,8 +190,10 @@ describe('buildPlanReport', () => {
 describe('stringifyPlanReport', () => {
   it('round-trips: the collapsed text parses back to the same object', () => {
     const diff = editFile('src/heavy.ts', 3, 900) + addFile('src/light.ts', 20);
-    const report = buildPlanReport(buildDiffPlan(diff, 400), (p) =>
-      p === 'src/heavy.ts' ? 6000 : 30,
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      (p) => (p === 'src/heavy.ts' ? 6000 : 30),
+      undefined,
     );
     expect(JSON.parse(stringifyPlanReport(report))).toEqual(report);
   });
@@ -177,6 +202,7 @@ describe('stringifyPlanReport', () => {
     const report = buildPlanReport(
       buildDiffPlan(editFile('src/heavy.ts', 3, 900), 400),
       () => 6000,
+      undefined,
     );
     const text = stringifyPlanReport(report);
     // Not one giant line: `read_file` pages at line boundaries, so a compact
@@ -193,6 +219,7 @@ describe('stringifyPlanReport', () => {
     const report = buildPlanReport(
       buildDiffPlan(editFile('src/heavy.ts', 3, 900), 400),
       () => 6000,
+      undefined,
     );
     const collapsed = stringifyPlanReport(report).length;
     const indented = JSON.stringify(report, null, 2).length + 1;
@@ -211,7 +238,11 @@ describe('stringifyPlanReport', () => {
       '+x',
       '',
     ].join('\n');
-    const report = buildPlanReport(buildDiffPlan(diff, 400), () => 1);
+    const report = buildPlanReport(
+      buildDiffPlan(diff, 400),
+      () => 1,
+      undefined,
+    );
     const parsed = JSON.parse(stringifyPlanReport(report)) as typeof report;
     expect(parsed.files[0].path).toBe(weird);
   });