Skip to content

fix: account completion calling turn usage - #58

Merged
fitchmultz merged 5 commits into
fitchmultz:mainfrom
hughcars:fix/completion-turn-accounting
Sep 23, 2026
Merged

fitchmultz merged 5 commits into
fitchmultz:mainfrom
hughcars:fix/completion-turn-accounting

Conversation

@hughcars

@hughcars hughcars commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #57.

Fix

Count the assistant response that calls update_goal before marking the goal complete. Use Pi's persisted current assistant message and the executing tool-call ID; completion and subsequent turn events still charge it only once.

Keep response-token ownership tied to the goal active at turn start. Replacing a goal during a response still tracks elapsed tool time, but does not charge the replacement for the old response. Pausing and resuming the same goal preserves its response tokens. Retire ownership after turn accounting so the aborted-response fallback cannot count it again.

This retains the original contribution, simplifies its lookup to native session types, and adds a real in-memory SDK regression.

Verification

  • npm run verify: 353 tests passed, plus typecheck and platform-smoke harness checks.
  • Real SDK regression: prior turn 410,000 + completion response 100,000 = final report 510,000 of 500,000. Restoring the old zero-token completion path fails with 410,000 !== 510,000.
  • Removing turn ownership makes command/tool replacement tests fail with 120 !== 0; the fix preserves five seconds of elapsed tool time.
  • Same-goal pause/resume regression demonstrated 0 !== 120 before the correction; normal responses and completion calls now retain their tokens, with or without an earlier tool result.
  • Resuming between aborted turn_end and agent_end demonstrated 84 !== 42 before the correction; the fallback now leaves the already-accounted 42 tokens unchanged.
  • Coverage includes duplicate completion, multi-tool messages, reused/empty call IDs, unmatched calls, budget crossing, replacement ownership, pause/resume, aborted-response deduplication, and actual SDK event/persistence ordering.
  • Ponytail review removed an unused reset method. Correctness findings for pause/resume and abort deduplication were fixed; an unrelated pre-existing delay in starting a replacement goal's elapsed-time clock remains outside this token-accounting fix.
  • Final official/fork CI passed on 521e52e: https://github.com/fitchmultz/pi-codex-goal/actions/runs/35812844237. Final independent correctness and ponytail reviews have no remaining findings.

@fitchmultz
fitchmultz merged commit 121843f into fitchmultz:main Sep 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update_goal drops the calling turn’s token usage

2 participants