alibaba / alibaba/open-code-review
bug(llmloop): preserve task_done(DONE) completion from the grace round
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 1.8k
- Avg merge
- 2d 4m
- Merged PRs (30d)
- 102
Description
### OpenCodeReview Version
v1.9.8 (latest release). Affected range: v1.9.3 through v1.9.8. The grace round was introduced in da972b2; v1.9.2 and earlier predate it.
### Operating System
macOS (Apple Silicon)
### Installation Method
Built from source
### LLM Provider
Other OpenAI-compatible endpoint
The deterministic reproduction uses a scripted client, so the defect is not provider-specific.
### Bug Description
When a file exhausts its tool-call budget, `RunPerFile` makes a final grace-round LLM request limited to `code_comment` and `task_done`.
If the model calls `task_done(DONE)` in that round, the tool call is parsed and executed, but its completion state is discarded. `RunPerFile` returns `completed=false`, so scan/review callers record the file as incomplete rather than persisting a reusable `review_item_done` checkpoint. A later `ocr resume` then re-runs a file that the model explicitly marked complete.
### Steps to Reproduce
1. Configure a test template with `MaxToolRequestTimes=1`.
2. Return a valid context-tool call in the first LLM response so the budget is exhausted.
3. Return `task_done` with `{"state":"DONE"}` in the grace-round response.
4. Observe that `RunPerFile` returns `(completed=false, StopMaxRounds, nil)`.
5. Run the same flow through scan mode, finalize the session, then load it with `LoadResumeState`.
6. Observe that no completed checkpoint is available and a resumed scan invokes the LLM again for the unchanged file.
This can be reproduced deterministically with a scripted LLM client; no API key is required.
### Expected Behavior
A `task_done(DONE)` executed in the grace round should propagate completion through `RunPerFile`. Scan/review should persist `review_item_done`, and a later resume should reuse the unchanged file without another LLM request.
### Logs / Error Output
```shell
main_task did not complete before stopping
```
### Additional Context
Root cause in v1.9.8:
- `internal/llmloop/loop.go:377-381` invokes `runGraceRound` after `StopMaxRounds` and then unconditionally returns `completed=false`.
- `internal/llmloop/loop.go:432-434` executes grace-round tool calls but discards each returned `TaskCheckpoint`.
- `task_done(DONE)` returns `TaskCheckpoint{Completed: true}`, but that state never reaches scan/review checkpoint persistence.
The grace round still records its LLM response and executes side effects such as `code_comment`; only the terminal completion state is lost.
Contributor guide
Research direction
Start in internal/llmloop/loop.go at the StopMaxRounds path around lines 377-381 and the grace-round tool handling around lines 432-434. Trace the TaskCheckpoint returned by task_done(DONE) through RunPerFile, scan/review checkpoint persistence, and LoadResumeState; done means the completion reaches review_item_done and resume avoids another LLM request for the unchanged file.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- ai
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100