Skip to content

fix(core): cap done repair loop#292

Merged
Sun-sunshine06 merged 1 commit into
mainfrom
fix/251-done-repair-limit
May 3, 2026
Merged

fix(core): cap done repair loop#292
Sun-sunshine06 merged 1 commit into
mainfrom
fix/251-done-repair-limit

Conversation

@Sun-sunshine06
Copy link
Copy Markdown
Collaborator

Summary

  • enforce a three-error-round budget around the agentic done() repair loop
  • terminate the pi-agent tool loop when the budget is exhausted while preserving the latest artifact with warnings
  • align done/tool prompt copy and add regression coverage for the repair-limit path

Fixes #251

Testing

  • pnpm exec biome check packages/core/src/agent.ts packages/core/src/tools/done.ts packages/core/src/agent.test.ts .changeset/done-repair-limit.md
  • pnpm --dir packages/core exec vitest run src/agent.test.ts
  • pnpm --dir packages/core exec tsc --noEmit
  • pnpm typecheck
  • pnpm test

@github-actions github-actions Bot added docs Documentation area:core packages/core (generation orchestration) labels May 3, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review mode: initial

Findings

  • [Minor] The errorRounds counter resets to 0 on a successful done() call (status "ok"). This means after a successful verification, the agent gets a fresh three-error budget for subsequent done() calls. While not a bug, this may allow the agent to cycle through error rounds and fixes indefinitely if it introduces errors after each fix. Consider whether a per-session (non-resetting) budget is desired, or at least document the design choice in the constant comment. (packages/core/src/agent.ts:389)
  • [Nit] formatDoneRepairLimitText includes 'has_errors' as the first line of the text content returned to the model. The previous done() result's details already indicate status: 'has_errors', so repeating it in the text is redundant. Consider removing that line. (packages/core/src/agent.ts:407)

Summary

This PR caps the done() repair loop at three error rounds, preserving the latest artifact and surfacing warnings when the limit is reached. The changes are clean, well-tested, and align the tool prompt copy with the new limit. The two new tests cover both the direct tool behavior and the full agent termination path. Overall, the implementation direction is sound.

Testing

Good coverage: unit tests for the wrap logic and integration-style test using the executeTool mock. Consider adding a test that verifies errorRounds resets after a successful done() call, to document the expected reset behavior.

Open-CoDesign Bot

@Sun-sunshine06 Sun-sunshine06 merged commit 4e0c40c into main May 3, 2026
6 checks passed
@Sun-sunshine06 Sun-sunshine06 deleted the fix/251-done-repair-limit branch May 3, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core packages/core (generation orchestration) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Model reasoning in loop burning all tokens

1 participant