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,