feat(llmloop): add grace round after tool-request budget exhausted - #872
Conversation
When RunPerFile exits because MaxToolRequestTimes reaches zero, perform one additional LLM call with only code_comment and task_done available. This gives the model a final chance to submit findings it identified but had not yet reported, preventing loss of review comments on budget stop.
|
🔍 OpenCodeReview found 2 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
- Check ctx.Err() before making the grace round LLM call to avoid wasted API calls when the context is already cancelled. - Pass messages copy to AppendTaskRecord and call rec.SetResponse so the grace round interaction is visible in session/debug logs.
Cover three scenarios: - Grace round fires and collects code_comment on budget exhaustion - Grace round is skipped when context is already cancelled - Grace round is NOT triggered on StopEmptyRounds
| resp, err := r.deps.LLMClient.CompletionsWithCtx(ctx, llm.ChatRequest{ | ||
| Model: r.deps.Model, | ||
| Messages: messages, | ||
| Tools: graceDefs, |
There was a problem hiding this comment.
Restricting the tool list for this final call will likely cost us the whole KV cache prefix.
Placing the tool list in or next to the system prompt is the mainstream practice among current LLMs, precisely so that the prefix cache keeps hitting across a session. Swapping r.deps.MainToolDefs for the two-tool subset rewrites that part of the prefix, so everything behind it is invalidated.
The timing makes it expensive: runGraceRound only fires on StopMaxRounds, i.e. when the conversation has accumulated a full budget's worth of tool_result file contents and is at its longest. We re-read that prompt at full price and write a fresh cache entry that nothing downstream will ever read. The new totalCacheReadTokens/totalCacheWriteTokens counters should show it directly: read ≈ 0, write ≈ the whole prompt.
That said, there's a genuine trade-off here between effectiveness and cost. Narrowing the tool list is a hard constraint, whereas leaving MainToolDefs intact and relying on the prose instruction (optionally filtering resp.ToolCalls() on the response side) is only a soft one — the model could spend its last turn on a read tool. If the temporary override does measurably improve how often the grace round lands a real finding, then this may simply be a price we have to pay.
2 participants
Summary
When the LLM tool-request budget (
MaxToolRequestTimes) is exhausted during file review, the model may have identified issues but not yet submitted them viacode_comment. Previously these findings were silently lost.This PR adds a grace round: after the main loop exits on budget, one final LLM call is made with only
code_commentandtask_doneavailable. The model can either submit pending findings or cleanly exit — preventing false-positive pressure sincetask_doneprovides an opt-out.Changes
runGraceRound: new method onRunnerthat appends a budget-exhausted notice to the conversation and performs a single LLM call with restricted toolsgraceRoundToolDefs: filtersMainToolDefsto onlycode_comment+task_doneStopMaxRounds(not on empty-round or compression stops)Test plan
go build ./...passesgo test ./...all greenMAX_TOOL_REQUEST_TIMESto 3, confirmed grace round fires and comment is collected