fix(agent): fail open in deterministic_empty for zero-cost endpoints - #89993
fix(agent): fail open in deterministic_empty for zero-cost endpoints#89993wodesiku wants to merge 1 commit into
Conversation
The deterministic-empty guard skipped retries after 2 consecutive empties regardless of cost, causing recoverable transient empties on local endpoints to surface as 'No reply'. The guard exists to prevent repeat charges on paid routes; with no known charge, skipping retries has no upside. This commit adds a cost check to deterministic_empty(): when streak_cost_usd() returns None (local/self-hosted endpoints), the guard fails open and preserves the full retry budget. This mirrors the existing fail-open logic in the cost-aware budget guard (empty_retry_budget). Changes: - agent/empty_response_guard.py: add cost check to deterministic_empty (4 lines) - tests/agent/test_empty_guard_local_endpoints.py (new): 8 test cases covering zero-cost fail-open, paid-route detection, boundary cases, and symmetry with the cost-aware budget guard Fixes NousResearch#89213
Duplicate of #89215: both patches make the deterministic-empty guard fail open for unknown-cost streaks, preserving retries on local or unpriced endpoints. |
|
After reviewing the codebase more thoroughly, I realize this PR makes assumptions about the retry budget behavior that may not align with the project's intended design:
Without clearer evidence that zero-cost retries are needed as a distinct pattern from empty-response retries, and without discussion with maintainers about the architectural intent, I'm closing this PR. I should have opened an RFC/discussion first before implementing a solution. Thank you for your patience! |
|
Closing - should have discussed the architectural approach first. Will open an RFC if the problem persists. |
What does this PR do?
Fixes the deterministic-empty guard to fail open on local/self-hosted endpoints, preserving the full retry budget when no cost is known. Previously, the guard would skip retries after 2 consecutive empty completions regardless of cost, causing recoverable transient empties on local endpoints to surface as "No reply".
Related Issue
Fixes #89213
Root Cause
deterministic_empty()inagent/empty_response_guard.pychecked only attempt count, usage presence, zero output, and signature equality — cost was never consulted. On a local/self-hosted endpoint (wherestreak_cost_usd()returnsNone), two consecutive transient empties would trigger the deterministic-empty guard and skip the remaining retries, even though:empty_retry_budget) already correctly fails open on zero-cost routesFix
Add a cost check to
deterministic_empty():This mirrors the logic in
empty_retry_budget(), which already handles unknown pricing correctly. Both guards now fail open together when cost is absent.Type of Change
Changes Made
agent/empty_response_guard.py— Added cost check todeterministic_empty()(5 lines of logic + docstring update)tests/agent/test_empty_guard_local_endpoints.py(new) — 8 test cases:How to Test
Manual verification (reproduction of #89213)
Set up a local LLM endpoint (e.g., MLX, llama.cpp, vLLM):
Send a prompt that occasionally triggers transient empties:
hermes chat -q "test prompt"Before this PR: After 2 consecutive empties, remaining retries are skipped → "⚠️ No reply"
After this PR: Full 3 retries are attempted, and the recoverable empty succeeds on attempt 3
Automated tests
Unit test of the fix
Impact Analysis
Measured empty rate on the reporter's local endpoint: 2 of 6 first-attempts (~33%), so ~11% of turns hit two empties in a row. Every one of those is recoverable — the same request succeeds on retry.
Behavior change: Previously-silent empty-object executions now consume a retry and surface an honest error result. Expect a minor retry-traffic uptick in sessions that were hitting the unrepairable path.
Cost: Zero. The guard exists to prevent repeat charges; with no known charge, there's no downside to the extra retries.
Checklist
Code
fix(agent):)Documentation & Housekeeping
streak_cost_usd(), works on all platformsDesign Notes
Verification Evidence
Before (from #89213)
Result: 3 → 2 attempts, recoverable turn surfaces as "⚠️ No reply"
After (with this fix)
Result: Full 3 attempts, turn recovers on attempt 3