fix: Accumulate retry costs instead of overwriting in _LAST_CALLBACK_DATA#519
Merged
gltanaka merged 10 commits intopromptdriven:mainfrom Feb 16, 2026
Merged
fix: Accumulate retry costs instead of overwriting in _LAST_CALLBACK_DATA#519gltanaka merged 10 commits intopromptdriven:mainfrom
gltanaka merged 10 commits intopromptdriven:mainfrom
Conversation
Unit and E2E tests that verify _LAST_CALLBACK_DATA["cost"] accumulates across retries instead of being overwritten. Tests currently fail, confirming the bug. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n _LAST_CALLBACK_DATA Fixes promptdriven#509
…urvive importlib.reload Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
you need to fix the unit tests |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes under-reported LLM spend when llm_invoke() triggers cache-bypass retries by accumulating _LAST_CALLBACK_DATA cost/token totals across retry paths, and adds regression tests for issue #509.
Changes:
- Accumulate cost + token totals across retries (None content, malformed JSON, invalid Python) instead of overwriting
_LAST_CALLBACK_DATA. - Expand malformed-JSON detection to include excessive actual trailing newlines.
- Add unit + “E2E-style” tests covering retry cost accumulation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
pdd/llm_invoke.py |
Saves pre-retry callback totals and adds them back after retry completion; expands malformed-JSON heuristic. |
tests/test_llm_invoke_retry_cost.py |
Adds unit tests asserting cost accumulation across retry scenarios. |
tests/test_e2e_issue_509_retry_cost.py |
Adds higher-level tests simulating retry behavior and verifying accumulated cost. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The CSV had empty run_test_command for JavaScript, TypeScript, and TypeScriptReact, causing 5 CI test failures in test_agentic_langtest, test_get_language, and test_get_test_command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y condition
- Add token accumulation assertions to test that claims to verify tokens
- Remove unused imports (csv, CliRunner) from e2e test
- Fix misleading docstring about CliRunner usage
- Trim 3-call setup to 2-call to match actual retry behavior
- Simplify redundant endswith('}') check in _is_malformed_json_response
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… retry failure All 3 retry paths (None content, malformed JSON, invalid Python) set litellm.cache = None but only restored it on success. If the retry call raised an exception, cache stayed permanently disabled for the process. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve conflict: keep upstream's language_format.csv (was accidentally deleted on this branch). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
llm_invokeretries due to None content, malformed JSON, or invalid Python_LAST_CALLBACK_DATA["cost"]was being overwritten on each retry call, silently losing the original call's costVerified with real API calls
Test plan
tests/test_llm_invoke_retry_cost.py(4 tests)tests/test_e2e_issue_509_retry_cost.py(2 tests)🤖 Generated with Claude Code