From 6f393487d1a6b55aac88de78fd3a3657c43f85b2 Mon Sep 17 00:00:00 2001 From: Jory Irving Date: Wed, 2 Sep 2026 13:25:02 -0600 Subject: [PATCH] fix(groomer): warn the model before its exploration rounds run out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The byte-budget path tells the model 'budget exhausted, call submit_findings with what you have' and the model reliably complies. The round limit had no equivalent: the loop simply ended, so a run that explored well but had not volunteered findings was discarded with nothing. That mattered once the byte budget stopped binding. Measured over the first runs on the larger budget, exploration went from 6.7 to 22.2 tool calls while the share of runs reporting files fell from 55% to 17%, with 4 of 6 ending on the silent round ceiling. The tight budget had been supplying deadline pressure by accident; removing it left room to explore and no reason to conclude. Also rename maxToolCalls to maxRounds. It caps model round-trips, not calls — 12 rounds produced 22 calls — matching the max_rounds / max_tool_calls split in pr-reviewer-action. DISPATCH_GROOMER_MAX_TOOL_CALLS stays as a deprecated alias. Claude-Session: https://claude.ai/code/session_01YSuDvZq9ncvyX85Uzx3cQh --- docs/hosted-groomer.md | 6 ++- src/lib/groomer/config.ts | 9 +++-- src/lib/groomer/explore.test.ts | 70 +++++++++++++++++++++++++++++++-- src/lib/groomer/explore.ts | 31 +++++++++++++-- src/lib/groomer/run.test.ts | 2 +- src/lib/groomer/run.ts | 2 +- 6 files changed, 106 insertions(+), 14 deletions(-) diff --git a/docs/hosted-groomer.md b/docs/hosted-groomer.md index c61b2d3f..0aa1635e 100644 --- a/docs/hosted-groomer.md +++ b/docs/hosted-groomer.md @@ -28,7 +28,7 @@ The feature is disabled by default. | `DISPATCH_GROOMER_MAX_FILE_BYTES` | `4096` | Maximum bytes per fetched file snippet. | | `DISPATCH_GROOMER_COMMENT_COOLDOWN_HOURS` | `24` | Suppresses repeated hosted-groomer comments on the same issue. A comment is skipped (and recorded on the run) when a prior run posted a comment within this window, unless `force` is true. | | `DISPATCH_GROOMER_TOOL_LOOP_ENABLED` | `true` | Lets the groomer drive its own repository exploration with tools (`search_code`, `read_file`, `list_directory`, `submit_findings`) instead of one pre-computed context block. | -| `DISPATCH_GROOMER_MAX_TOOL_CALLS` | `12` | Tool calls the exploration loop may make per grooming run. | +| `DISPATCH_GROOMER_MAX_ROUNDS` | `12` | Model round-trips the exploration loop may make. One round can carry several tool calls, so this is not a cap on calls. `DISPATCH_GROOMER_MAX_TOOL_CALLS` is accepted as a deprecated alias. | | `DISPATCH_GROOMER_CONTEXT_MODE` | `medium` | Exploration budget preset: `small`, `medium`, `large`. See below. | | `DISPATCH_GROOMER_MODEL_CONTEXT_TOKENS` | unset | The model's real context window in tokens. When set, the exploration budget is derived from it and `DISPATCH_GROOMER_CONTEXT_MODE` is ignored — recommended for self-hosted models whose windows do not match what a named preset assumes. | | `DISPATCH_GROOMER_EXPLORE_MAX_BYTES` | from mode | Overrides the exploration byte budget. | @@ -54,6 +54,10 @@ own rather than the single-call one. Three ways to size it, most specific first: | `medium` (default) | 24 KB | 8 KB | 150s | | `large` | 96 KB | 24 KB | 300s | +With two rounds left the loop tells the model to submit what it has, so a run +that explores well but never volunteers findings is not discarded empty. The +byte budget carries the same nudge when it runs out. + The default suits a modest self-hosted model. If your model's window is much larger, prefer setting `DISPATCH_GROOMER_MODEL_CONTEXT_TOKENS` over guessing a preset: a starved loop stops mid-investigation and reports fewer files, which diff --git a/src/lib/groomer/config.ts b/src/lib/groomer/config.ts index 09b4b8ae..689261e2 100644 --- a/src/lib/groomer/config.ts +++ b/src/lib/groomer/config.ts @@ -15,7 +15,7 @@ export interface HostedGroomerConfig { commentCooldownHours: number; groomerToken: string | null; toolLoopEnabled: boolean; - maxToolCalls: number; + maxRounds: number; maxSearchResults: number; maxDirEntries: number; exploration: ExplorationBudget; @@ -56,7 +56,7 @@ export function getHostedGroomerConfig(): HostedGroomerConfig { maxContextBytes: 8192, repoContextEnabled: false, toolLoopEnabled: false, - maxToolCalls: 0, + maxRounds: 0, maxSearchResults: 0, maxDirEntries: 0, exploration: { maxTotalBytes: 0, maxFileBytes: 0, timeoutMs: 0, source: "small" }, @@ -97,7 +97,10 @@ export function getHostedGroomerConfig(): HostedGroomerConfig { commentCooldownHours: parseIntEnv(process.env.DISPATCH_GROOMER_COMMENT_COOLDOWN_HOURS, 24), groomerToken: process.env.DISPATCH_GROOMER_TOKEN?.trim() || null, toolLoopEnabled: parseBool(process.env.DISPATCH_GROOMER_TOOL_LOOP_ENABLED, true), - maxToolCalls: parseIntEnv(process.env.DISPATCH_GROOMER_MAX_TOOL_CALLS, 12), + maxRounds: parseIntEnv( + process.env.DISPATCH_GROOMER_MAX_ROUNDS ?? process.env.DISPATCH_GROOMER_MAX_TOOL_CALLS, + 12, + ), maxSearchResults: parseIntEnv(process.env.DISPATCH_GROOMER_MAX_SEARCH_RESULTS, 10), maxDirEntries: parseIntEnv(process.env.DISPATCH_GROOMER_MAX_DIR_ENTRIES, 60), exploration: resolveExplorationBudget(), diff --git a/src/lib/groomer/explore.test.ts b/src/lib/groomer/explore.test.ts index 97473aa1..4af4ea36 100644 --- a/src/lib/groomer/explore.test.ts +++ b/src/lib/groomer/explore.test.ts @@ -8,7 +8,7 @@ const options: ExploreOptions = { repoFullName: "org/repo", prompt: "Issue #899: DATABASE_URL with sslmode=no-verify does not turn TLS on", timeoutMs: 60_000, - maxToolCalls: 12, + maxRounds: 12, maxTotalBytes: 8192, maxSearchResults: 10, maxFileBytes: 4096, @@ -125,7 +125,7 @@ describe("exploreRepository", () => { }); const deps = makeDeps({ readFile: vi.fn().mockResolvedValue("y".repeat(400)) }, fetchImpl); - const result = await exploreRepository({ ...options, maxTotalBytes: 100, maxToolCalls: 4 }, deps); + const result = await exploreRepository({ ...options, maxTotalBytes: 100, maxRounds: 4 }, deps); expect(result.warnings).toContain("repository exploration hit its byte budget"); expect(deps.tools.readFile).toHaveBeenCalledTimes(1); @@ -138,11 +138,11 @@ describe("exploreRepository", () => { }); const deps = makeDeps({ searchCode: vi.fn().mockResolvedValue([{ path: "a.ts" }]) }, fetchImpl); - const result = await exploreRepository({ ...options, maxToolCalls: 3 }, deps); + const result = await exploreRepository({ ...options, maxRounds: 3 }, deps); expect(result.toolCalls).toHaveLength(3); expect(result.warnings).toContain( - "repository exploration hit its tool-call budget without submitting findings", + "repository exploration used all its rounds without submitting findings", ); }); @@ -207,3 +207,65 @@ describe("exploreRepository bounds model-supplied findings", () => { expect(result.files).toEqual(["src/a.ts"]); }); }); + +describe("exploreRepository round-limit warning", () => { + it("tells the model to submit when it is nearly out of rounds", async () => { + const calls: string[][] = []; + const fetchImpl = vi.fn().mockImplementation(async (_url: string, init: RequestInit) => { + const body = JSON.parse(init.body as string); + calls.push(body.messages.map((m: { role: string }) => m.role)); + return { + ok: true, + status: 200, + json: async () => ({ + choices: [ + { message: { content: null, tool_calls: [toolCall("1", "search_code", { query: "x" })] } }, + ], + }), + text: async () => "", + }; + }) as unknown as typeof fetch; + + const deps = makeDeps({ searchCode: vi.fn().mockResolvedValue([{ path: "a.ts" }]) }, fetchImpl); + await exploreRepository({ ...options, maxRounds: 4 }, deps); + + const sent = (fetchImpl as unknown as ReturnType).mock.calls.map((c) => + JSON.parse((c[1] as RequestInit).body as string), + ); + const nudges = sent.flatMap((b) => + b.messages.filter( + (m: { role: string; content?: string }) => + m.role === "user" && typeof m.content === "string" && m.content.includes("round(s) left"), + ), + ); + expect(nudges.length).toBeGreaterThan(0); + expect(nudges[0].content).toContain("submit_findings"); + }); + + it("does not nudge before the final rounds", async () => { + const fetchImpl = fetchReturning({ + content: null, + tool_calls: [toolCall("1", "search_code", { query: "x" })], + }); + const deps = makeDeps({ searchCode: vi.fn().mockResolvedValue([{ path: "a.ts" }]) }, fetchImpl); + await exploreRepository({ ...options, maxRounds: 12 }, deps); + + const first = JSON.parse( + ((fetchImpl as unknown as ReturnType).mock.calls[0][1] as RequestInit) + .body as string, + ); + expect( + first.messages.some( + (m: { content?: string }) => typeof m.content === "string" && m.content.includes("round(s) left"), + ), + ).toBe(false); + }); + + it("does not warn about rounds when the model stopped on its own", async () => { + const deps = makeDeps({}, fetchReturning({ content: "nothing to do", tool_calls: [] })); + const result = await exploreRepository({ ...options, maxRounds: 12 }, deps); + expect(result.warnings).not.toContain( + "repository exploration used all its rounds without submitting findings", + ); + }); +}); diff --git a/src/lib/groomer/explore.ts b/src/lib/groomer/explore.ts index 4d3b4b4b..cbc9e437 100644 --- a/src/lib/groomer/explore.ts +++ b/src/lib/groomer/explore.ts @@ -13,7 +13,9 @@ export interface ExploreOptions { /** The same issue context the final grooming call receives. */ prompt: string; timeoutMs: number; - maxToolCalls: number; + /** Model round-trips the loop may make. One round can carry several tool + * calls, so this is not a cap on calls — see maxToolCalls' doc comment. */ + maxRounds: number; maxTotalBytes: number; maxSearchResults: number; maxFileBytes: number; @@ -64,6 +66,9 @@ Work like this: Call submit_findings once you can name the files a worker would change and state what the issue is asking for in this repository's own terms. Be concrete: real paths you have actually seen, never a guess.`; +/** Rounds remaining at which the model is told to wrap up. */ +export const ROUNDS_REMAINING_WARNING = 2; + const EMPTY: Omit = { findings: "", files: [], @@ -142,6 +147,7 @@ export async function exploreRepository( const sources: string[] = []; let bytes = 0; + let roundsExhausted = false; const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), options.timeoutMs); @@ -151,7 +157,24 @@ export async function exploreRepository( ]; try { - for (let turn = 0; turn < options.maxToolCalls; turn++) { + for (let turn = 0; turn < options.maxRounds; turn++) { + roundsExhausted = turn === options.maxRounds - 1; + // Tell the model when it is running out of rounds. The byte-budget path + // already does this and the model reliably submits when it hears it; the + // round limit used to just end the loop, so a run that explored well but + // did not volunteer findings was discarded with nothing to show. Give it + // the same deadline pressure rather than a silent cut-off. + const roundsLeft = options.maxRounds - turn; + if (roundsLeft <= ROUNDS_REMAINING_WARNING && roundsLeft > 0 && records.length > 0) { + messages.push({ + role: "user", + content: + `You have ${roundsLeft} round(s) left before this investigation ends. ` + + "Call submit_findings now with the files you have already opened, " + + "even if you have not finished exploring.", + }); + } + const response = await deps.fetchImpl(`${options.baseUrl}/chat/completions`, { method: "POST", headers: { @@ -263,8 +286,8 @@ export async function exploreRepository( } } - if (records.length >= options.maxToolCalls) { - warnings.push("repository exploration hit its tool-call budget without submitting findings"); + if (roundsExhausted) { + warnings.push("repository exploration used all its rounds without submitting findings"); } return { diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index d20f5585..074ae3f4 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -129,7 +129,7 @@ const mockConfig: HostedGroomerConfig = { commentCooldownHours: 24, groomerToken: null, toolLoopEnabled: false, - maxToolCalls: 12, + maxRounds: 12, maxSearchResults: 10, maxDirEntries: 60, exploration: { maxTotalBytes: 24576, maxFileBytes: 8192, timeoutMs: 150000, source: "medium" }, diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index cb7211c2..dcaa6990 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -197,7 +197,7 @@ async function executeGroomerRun( repoFullName: candidate.repoFullName, prompt: context, timeoutMs: config.exploration.timeoutMs, - maxToolCalls: config.maxToolCalls, + maxRounds: config.maxRounds, maxTotalBytes: config.exploration.maxTotalBytes, maxSearchResults: config.maxSearchResults, maxFileBytes: config.exploration.maxFileBytes,