fix(agent): keep the empty-retry budget when a streak has no known cost - #89215
fix(agent): keep the empty-retry budget when a streak has no known cost#89215humdrum00001010 wants to merge 1 commit into
Conversation
deterministic_empty() aborted the retry budget after two consecutive
zero-output completions without ever consulting cost, unlike the
cost-aware budget guard in the same module ("Unknown pricing, missing
usage, or included/subscription routes leave the budget untouched").
Both guards exist to prevent repeat charges. On a local/self-hosted
endpoint there is nothing to save, so cutting the budget from 3 to 2 can
only forfeit a turn the next attempt would have recovered.
The guard's premise -- "the same prompt will keep producing the same
empty" -- does not hold there. Captured through a logging proxy, two
byte-identical requests in one turn returned EMPTY then a proper
tool_calls response; measured empty rate was 2 of 6 first-attempts, so
roughly 11% of turns hit two in a row and tripped the guard.
Gate the skip on streak_cost_usd() being known, mirroring the budget
guard. Paid routes are unaffected -- the motivating repeat-billing
incident still short-circuits after the second attempt.
test_deterministic_empty_stops_retries_early asserted this behaviour on
a localhost base_url while its own docstring described "re-billing the
full input". It now pins an estimated cost so it exercises the billed
path it documents, and a companion test covers the unpriced streak.
Fixes NousResearch#89213
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewed by reviewer-e (AI automated review). Correct incentive-aligned fix for #89213: the deterministic-empty skip exists purely to avoid repeat charges, so a streak with |
What does this PR do?
deterministic_empty()aborts the empty-retry budget after two consecutive zero-output completions, but — unlike the cost-aware budget guard in the same module — it never consults cost. This gates the skip on the streak having a known cost, mirroring the rule the module already documents for guard #2:Both guards exist to prevent repeat charges ("the charged ~$2.33 for an empty answer incident class"). On a local/self-hosted endpoint there is nothing to save, so cutting the budget from 3 to 2 can only forfeit a turn the next attempt would have recovered.
Related Issue
Fixes #89213
Why the guard's premise doesn't hold there
The module states the rationale as "the same prompt will keep producing the same empty." I measured that directly. Capturing hermes↔endpoint traffic through a logging proxy, two calls in the same turn were byte-identical requests with different outcomes:
Measured empty rate was 2 of 6 first-attempts (~33%), so ~11% of turns hit two empties in a row and trip the guard. Each is recoverable. The endpoint is not at fault — 10/10 direct
curlcalls returned cleantool_calls.Observed asymmetry on that setup:
Type of Change
Changes Made
agent/empty_response_guard.py—deterministic_empty()returnsFalsewhenstreak_cost_usd()isNone. Reuses the existing per-streak cost accessor, so no new state or config. Docstrings updated to record the fail-open condition.tests/agent/test_empty_response_guard.py— 4 tests: unknown-cost and zero-cost streaks are not deterministic, a known-cost streak still is, and failing open on cost does not widen the retry budget.tests/run_agent/test_run_agent.py— addedtest_unpriced_empty_streak_keeps_full_retry_budget(two empties then a success must reach the third call).test_deterministic_empty_stops_retries_earlyassertedapi_calls == 2withagent.base_url = "http://127.0.0.1:1234/v1"— a localhost endpoint, which is exactly the unpriced case this PR makes fail open, so it broke.Its own docstring describes the behaviour under test as:
but a localhost endpoint bills nothing, so the fixture contradicted the intent it documented. I pinned an estimated cost via
patch("agent.empty_response_guard._estimate_attempt_cost", return_value=Decimal("1.10"))so it exercises the billed path it describes, and left the assertion (api_calls == 2) unchanged.If maintainers intend the skip to apply on unpriced routes too, then this PR's premise is wrong and the right fix is narrower — e.g. requiring 3 consecutive empties instead of 2 when cost is unknown. Happy to switch.
How to Test
298 passed. The 2 bug-targeting guard tests were confirmed to fail without the source change and pass with it.One unrelated pre-existing failure, verified identical on clean
mainat 2d511f5 and deselected in that run:TestAnthropicInterruptHandler::test_interruptible_anthropic_interrupt_never_closes_shared_client—ImportError: The 'anthropic' package is required, an env artifact, not touched by this PR.Checklist
empty_response_guard,deterministic empty,empty response retries local) — nearest is Provider fallback is never re-evaluated inside a turn — one transient 429 pins a long agentic run to the fallback for its entire duration #88595 (provider fallback not re-evaluated within a turn), which is a different stage of the same loop