Skip to content
46 changes: 46 additions & 0 deletions packages/cli/src/commands/review/agent-prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3665,3 +3665,49 @@ describe('per-chunk retirement — cold territories stop costing a round', () =>
expect(msg).toContain('CONVERGED');
});
});

describe('the tool budget in the briefs', () => {
const budgetPlan = {
...PLAN,
budget: {
inlineAngles: 4,
sweep: true,
specialistCap: 2,
verifyShard: 8,
agentToolBudget: 42,
},
} as never;

it('bakes the soft ceiling into finder and chunk briefs', () => {
expect(buildChunkAgentPrompt(budgetPlan, 13)).toContain(
'About **42 tool calls**',
);
for (const role of ['1a', '2', '6b', 'reverse-audit'] as const) {
expect(buildRoleBrief(budgetPlan, role)).toContain(
'About **42 tool calls**',
);
}
});

it('never budgets the verifier or Build & Test', () => {
// The verifier's per-finding re-trace is the one walk that must not stop
// early (verifyShard governs its load); Agent 7 runs deterministic
// commands.
for (const role of ['verify', '7'] as const) {
expect(buildRoleBrief(budgetPlan, role)).not.toContain('Tool budget');
}
});

it('a plan without the field falls back to no ceiling — more coverage, never less', () => {
expect(buildChunkAgentPrompt(PLAN as never, 13)).not.toContain(
'Tool budget',
);
expect(buildRoleBrief(PLAN as never, '1a')).not.toContain('Tool budget');
});

it('restates the recall rule so the ceiling cannot read as a reporting cap', () => {
expect(buildRoleBrief(budgetPlan, '1a')).toContain(
'never suppresses a finding',
);
});
});
42 changes: 42 additions & 0 deletions packages/cli/src/commands/review/agent-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,40 @@ function chunkFrom(
return { diffPath, chunk, total: chunks.length };
}

/**
* The soft tool-call ceiling for finder/auditor briefs (see
* lib/budget.ts, `agentToolBudget`). Empty when the plan predates the
* field — an old plan fails toward more coverage, exactly like the
* pre-budget fallback the skill documents — and empty for the roles whose
* load is governed elsewhere: the verifier (verifyShard) and Build & Test
* (deterministic commands).
*
* The wording is deliberate on two points. "Stop exploring" is aimed at the
* measured pathology — the slowest agent of a wave is reliably one that
* kept walking the tree past any recall gain (two runs of the same
* 14-agent wave: 11.7 vs 41 minutes) — and the recall rule is restated
* inline because a budget that reads as a reporting cap would suppress
* exactly the low-confidence candidates the pipeline's later stages exist
* to judge.
*/
function toolBudgetBlock(report: PlanReport): string[] {
const budget = report.budget?.agentToolBudget;
if (typeof budget !== 'number' || budget <= 0) return [];
return [
'',
'## Tool budget',
'',
`About **${budget} tool calls** for this whole review — reads, greps, shell, ` +
'everything — as a soft ceiling. At the ceiling: stop exploring, write your ' +
Comment thread
wenshao marked this conversation as resolved.
Outdated
'findings from the evidence already in hand, and name what you did not get ' +
'to in your return (`did not get to: <the check>`) — a disclosed gap is ' +
'actionable; a silent crawl only slows the review. The budget never ' +
Comment thread
wenshao marked this conversation as resolved.
Outdated
'suppresses a finding: a candidate you can already name goes in your ' +
'return regardless (at `Confidence: low` if the budget stopped you before ' +
'verifying it), exactly as the recall rule requires.',
];
}

/**
* The launch prompt for the agent that owns `chunk`.
*
Expand Down Expand Up @@ -385,6 +419,8 @@ export function buildChunkAgentPrompt(
parts.push('', ...repositoryContextBlock(repositoryContext));
}

parts.push(...toolBudgetBlock(report));
Comment thread
wenshao marked this conversation as resolved.
Outdated

// Deliberately NOT included: a sentence for the agent to recite when it finds
// nothing. Every real launch handed the agent its own receipt text — `If you
// find no issues, say "No issues found — reviewed chunk 13 (...)"` — and an
Expand Down Expand Up @@ -830,6 +866,12 @@ export function buildRoleBrief(
}

parts.push('## Your dimension', '', brief.brief);
// Not the verifier (verifyShard governs its load, and its per-finding
// re-tracing is the one walk that must not stop early) and not Build & Test
// (deterministic commands, already self-budgeted).
if (role !== 'verify' && role !== '7') {
parts.push(...toolBudgetBlock(report));
}
Comment thread
wenshao marked this conversation as resolved.
Outdated
Comment thread
wenshao marked this conversation as resolved.
Outdated
const repositoryContext = repositoryContextOf(report);
if (role === '7') {
if (repositoryContext) {
Expand Down
30 changes: 30 additions & 0 deletions packages/cli/src/commands/review/lib/budget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,33 @@ describe('reviewBudget — garbled input fails toward the cheap end, never throw
}
});
});

describe('reviewBudget — the agent tool budget', () => {
it('floors at 30 on a small diff', () => {
expect(
reviewBudget({ srcDiffLines: 40, diffLines: 60 }).agentToolBudget,
).toBe(32);
expect(
reviewBudget({ srcDiffLines: 0, diffLines: 0 }).agentToolBudget,
).toBe(30);
});

it('earns a call per twenty effective lines', () => {
expect(
reviewBudget({ srcDiffLines: 300, diffLines: 400 }).agentToolBudget,
).toBe(45);
});

it('caps at 60 — a wanderer must not out-earn the ceiling', () => {
expect(
reviewBudget({ srcDiffLines: 5000, diffLines: 6000 }).agentToolBudget,
).toBe(60);
});

it('a large all-prose diff earns budget at the coarse effective rate', () => {
// effective = max(src, total/8): prose still has lines to walk.
expect(
reviewBudget({ srcDiffLines: 0, diffLines: 3200 }).agentToolBudget,
).toBe(50);
});
});
32 changes: 32 additions & 0 deletions packages/cli/src/commands/review/lib/budget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,22 @@ export interface ReviewBudget {
* its list, which is a fact about the verifier and not about the diff.
*/
verifyShard: number;
/**
* Soft tool-call ceiling baked into every finder/auditor brief — not the
* verifier, whose load `verifyShard` already governs, and not Build & Test,
Comment thread
wenshao marked this conversation as resolved.
* whose calls are deterministic commands.
*
* A fan-out wave's wall clock is its slowest agent, and the slowest agent
* is reliably a wanderer: two measured runs of the SAME 14-agent wave took
* 11.7 and 41 minutes, the difference being individual agents spending
* 40-100 model calls exploring the tree, while healthy agents on
* comparable diffs settle in the 25-45 range. The ceiling is SOFT: the
* brief tells the agent to stop exploring at the budget, write its
* findings from the evidence in hand, and disclose what it did not get to
* — a disclosed gap feeds the whiff and receipt machinery; an undisclosed
* crawl only feeds the wall clock.
*/
agentToolBudget: number;
}

/**
Expand All @@ -99,6 +115,17 @@ export const MIN_INLINE_ANGLES = 3;
export const MAX_INLINE_ANGLES = 6;
export const VERIFY_SHARD = 8;

/**
* The floor is what a small diff's walk legitimately needs (brief + chunk
* reads + a handful of enclosing-function reads and greps); the ceiling sits
* above every healthy per-agent count measured on real reviews (25-45) and
* below the wandering pathology (40-100+). One extra call per twenty
* effective lines lets a larger territory earn a longer walk.
*/
export const MIN_AGENT_TOOL_BUDGET = 30;
export const MAX_AGENT_TOOL_BUDGET = 60;
const LINES_PER_TOOL_CALL = 20;

/**
* The review budget for a plan.
*
Expand Down Expand Up @@ -133,6 +160,11 @@ export function reviewBudget(input: BudgetInput): ReviewBudget {
sweep: effective >= SWEEP_FLOOR,
specialistCap: src >= SPECIALIST_FLOOR ? 2 : 0,
verifyShard: VERIFY_SHARD,
agentToolBudget: clamp(
MIN_AGENT_TOOL_BUDGET + Math.floor(effective / LINES_PER_TOOL_CALL),
Comment thread
wenshao marked this conversation as resolved.
MIN_AGENT_TOOL_BUDGET,
MAX_AGENT_TOOL_BUDGET,
),
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/skills/bundled/review/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -932,3 +932,7 @@ Every one of six measured CI reviews (2026-08-05/06) spent 1.5–3 minutes at St
### The one-command-per-turn tail

Measured across the same six CI reviews: the post-verdict bookkeeping — Markdown report, cost-ledger, save-artifact, `record_artifact`, the incremental-cache write, cleanup — ran one command per model turn, 4–6 minutes of wall clock after the review's outcome was already decided (and, on posting runs, already on the PR), stretching past 7 minutes when the qwen-home fumbling above joined it. Every command in the tail is cheap; the turns are not — the same arithmetic that batches the Step 1 setup calls, unapplied to the other end of the run.

### The forty-one minute wave

Two measured runs of the same 14-agent Step 3A fan-out, on diffs of comparable size, took 11.7 and 41 minutes — and the wave's wall clock is its slowest agent, so the whole review inherited the difference. The slow wave's tail was not review depth: individual agents spent 40-100 model calls exploring the tree (the pattern a sibling PR had already named as the next optimization target), while healthy agents on the same class of diff settle at 25-45 calls with indistinguishable findings. The budget that answers this is soft on purpose: a hard cap would convert the pathology into silent truncation, so the brief tells the agent to stop exploring at the ceiling, file what it holds, and disclose the checks it did not get to — the disclosure lands in the same receipt machinery that already judges whiffs.
2 changes: 1 addition & 1 deletion packages/core/src/skills/bundled/review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ Read from it:
- `diffLines`, `diffChars`, and `srcDiffLines` / `testDiffLines` / `docsDiffLines` / `generatedDiffLines`
- `chunks[]` — contiguous, non-overlapping line ranges tiling the whole diff. Each entry has `id`, `startLine`, `endLine` (1-based, inclusive), `lines`, `chars`, an `oversized` flag, and `files[]` naming the source files and new-side line ranges it covers. A chunk with `oversized: true` may exceed what one `read_file` call returns.
- `files[]` — per-file `kind` (`source` / `test` / `generated`), `hunks[]` new-side ranges (Step 7 validates comment anchors against these), `addedRanges[]` and `diffRange` (present only on `heavy` files — the exact lines the PR wrote, and where that file's own diff lives, so an invariant agent can see what was deleted), change counts, and the `heavy` flag
- `budget` — how much walking the **size-elastic** parts of this run owe, derived from `srcDiffLines` the same way the topology gate is, and recorded here rather than passed as a flag so every reader sees one number. `inlineAngles` and `sweep` scope Step 3C's low pass; `specialistCap` is the Agent 8 ceiling (**0** below 80 source lines — "one domain dominates the diff" is a judgement, and a judgement made about forty lines finds a dominant domain every time, because forty lines are usually all one thing); `verifyShard` is Step 4's findings-per-verifier. **It never scales a dimension away** — which agents a review owes is the roster's answer and the roster reads `effort`, so a size input cannot become a back door into shrinking coverage. Nothing here is yours to override: a budget the caller can inflate is a budget that gets inflated. **A plan with no `budget` field** (written by an older CLI — the version-skew this skill has already measured once) falls back to the pre-budget flat behaviour, which errs toward more coverage, never less: walk all six angles, run the sweep, cap Agent 8 at 2, shard verification at 8.
- `budget` — how much walking the **size-elastic** parts of this run owe, derived from `srcDiffLines` the same way the topology gate is, and recorded here rather than passed as a flag so every reader sees one number. `inlineAngles` and `sweep` scope Step 3C's low pass; `specialistCap` is the Agent 8 ceiling (**0** below 80 source lines — "one domain dominates the diff" is a judgement, and a judgement made about forty lines finds a dominant domain every time, because forty lines are usually all one thing); `verifyShard` is Step 4's findings-per-verifier; `agentToolBudget` is the soft tool-call ceiling `agent-prompt` bakes into every finder and auditor brief — not the verifier's, not Agent 7's — because a wave's wall clock is its slowest agent and the slowest agent is reliably one that kept exploring past any recall gain: the same 14-agent fan-out has measured 11.7 and 41 minutes on comparable diffs, the difference being individual agents spending 40-100 calls walking the tree (measured; DESIGN.md — The forty-one minute wave). The ceiling is soft and the briefs restate the recall rule beside it: at the budget an agent stops **exploring**, never reporting — findings in hand are filed, open checks are disclosed as `did not get to`. **It never scales a dimension away** — which agents a review owes is the roster's answer and the roster reads `effort`, so a size input cannot become a back door into shrinking coverage. Nothing here is yours to override: a budget the caller can inflate is a budget that gets inflated. **A plan with no `budget` field** (written by an older CLI — the version-skew this skill has already measured once) falls back to the pre-budget flat behaviour, which errs toward more coverage, never less: walk all six angles, run the sweep, cap Agent 8 at 2, shard verification at 8.

A chunk is read with `read_file(file_path=diffPathAbsolute, offset=startLine - 1, limit=endLine - startLine + 1)` — `offset` is 0-based.

Expand Down
Loading