diff --git a/agent/empty_response_guard.py b/agent/empty_response_guard.py index fbde4b58b04b7..c2fccadab0a2c 100644 --- a/agent/empty_response_guard.py +++ b/agent/empty_response_guard.py @@ -23,7 +23,9 @@ different model may behave differently). Attempts with missing usage or ``output_tokens > 0`` (model generated *something* — think-block stripping, whitespace, flaky decoding) never classify as deterministic - and keep the full retry budget. + and keep the full retry budget. Streaks with no known cost keep it too: + the skip exists to avoid repeat charges, so with nothing to save it can + only forfeit a recoverable turn. 2. **Cost-aware retry budget** — when the estimated input cost of a single empty attempt exceeds the configured threshold (default @@ -234,16 +236,25 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No def deterministic_empty(agent: Any) -> bool: """True when the current streak looks deterministic. - Requires >= 2 consecutive attempts, ALL with usage present, zero - output tokens, and an identical (model, provider, finish_reason) - signature. Any attempt with missing usage or non-zero output keeps - this False (fail open — transients deserve their retries). + Requires a streak with a known cost plus >= 2 consecutive attempts, + ALL with usage present, zero output tokens, and an identical (model, + provider, finish_reason) signature. Any attempt with missing usage or + non-zero output — or a streak with no known cost — keeps this False + (fail open — transients deserve their retries). """ if not guard_enabled(agent): return False attempts = getattr(agent, _ATTEMPTS_ATTR, None) or [] if len(attempts) < 2: return False + # Skipping retries only ever pays for itself by avoiding repeat charges + # (see the module docstring). When the streak carries no known cost — + # local/self-hosted endpoints, unpriced models, included routes — there is + # nothing to save, so discarding the remaining budget can only cost the + # user a turn that the next attempt would have recovered. Fail open, the + # same way the cost-aware budget guard already does on unknown pricing. + if streak_cost_usd(agent) is None: + return False first = attempts[0] return all( a.usage_present and a.zero_output and a.signature == first.signature diff --git a/tests/agent/test_empty_response_guard.py b/tests/agent/test_empty_response_guard.py index 3ec1a0c437b78..1677c61bc1e39 100644 --- a/tests/agent/test_empty_response_guard.py +++ b/tests/agent/test_empty_response_guard.py @@ -158,6 +158,54 @@ def _reasoning_only_response(): assert guard.deterministic_empty(agent) is False +class TestDeterministicEmptyRequiresKnownCost: + """#89213 — the skip exists to avoid repeat charges, so a streak with no + known cost must keep its full retry budget. On a local/self-hosted endpoint + the empties are transient: the same request succeeds on the next attempt. + """ + + def test_unknown_cost_streak_is_not_deterministic(self, monkeypatch): + monkeypatch.setattr(guard, "_estimate_attempt_cost", lambda a, r: None) + agent = _agent( + model="RavenX-CyberAgent-Qwen3.6-35B-A3B-mlx", + provider="custom", + base_url="http://localhost:8000/v1", + ) + _record_streak(agent, [_response(), _response()]) + # Signature/zero-output evidence is identical to the paid-route case… + assert guard.streak_cost_usd(agent) is None + # …but with nothing to save, the remaining retries must survive. + assert guard.deterministic_empty(agent) is False + + def test_zero_cost_streak_is_not_deterministic(self, monkeypatch): + """A priced-at-zero route is as free as an unpriced one.""" + monkeypatch.setattr( + guard, "_estimate_attempt_cost", lambda a, r: Decimal("0") + ) + agent = _agent() + _record_streak(agent, [_response(), _response()]) + assert guard.deterministic_empty(agent) is False + + def test_known_cost_streak_still_deterministic(self, monkeypatch): + """The motivating incident (paid route, repeat billing) is unchanged.""" + monkeypatch.setattr( + guard, "_estimate_attempt_cost", lambda a, r: Decimal("1.10") + ) + agent = _agent() + _record_streak(agent, [_response(), _response()]) + assert guard.streak_cost_usd(agent) == Decimal("2.20") + assert guard.deterministic_empty(agent) is True + + def test_unknown_cost_does_not_widen_the_budget(self, monkeypatch): + """Failing open on cost must not also inflate the retry budget.""" + monkeypatch.setattr(guard, "_estimate_attempt_cost", lambda a, r: None) + agent = _agent(provider="custom") + _record_streak(agent, [_response(), _response()]) + assert guard.empty_retry_budget(agent, _response()) == ( + guard.DEFAULT_EMPTY_RETRY_BUDGET + ) + + class TestEmptyRetryBudget: def test_default_budget_when_cost_unknown(self, monkeypatch): monkeypatch.setattr(guard, "_estimate_attempt_cost", lambda a, r: None) diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index ddd8702fb7223..8f2c005f3c7a2 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -19,6 +19,8 @@ from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch +from decimal import Decimal + import pytest from agent.codex_responses_adapter import _normalize_codex_response @@ -3268,7 +3270,12 @@ def test_deterministic_empty_stops_retries_early(self, agent): """NS-503: consecutive zero-output-token empties with identical model/provider/finish_reason are deterministic (unsignaled refusal) — the loop must stop re-billing the full input after the second - attempt instead of burning the whole retry budget.""" + attempt instead of burning the whole retry budget. + + Pinned to a *priced* streak: the skip exists to avoid repeat + charges, and since #89213 a streak with no known cost keeps its + full retry budget instead (see the companion test below). + """ self._setup_agent(agent) agent.base_url = "http://127.0.0.1:1234/v1" zero_usage = { @@ -3285,6 +3292,10 @@ def test_deterministic_empty_stops_retries_early(self, agent): patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), + patch( + "agent.empty_response_guard._estimate_attempt_cost", + return_value=Decimal("1.10"), + ), ): result = agent.run_conversation("answer me") assert result["completed"] is True @@ -3293,6 +3304,43 @@ def test_deterministic_empty_stops_retries_early(self, agent): # proves determinism, remaining retries are skipped. assert result["api_calls"] == 2 + def test_unpriced_empty_streak_keeps_full_retry_budget(self, agent): + """#89213: the same zero-output streak on a route with no known + cost (local/self-hosted endpoint) must NOT be treated as + deterministic — there are no repeat charges to avoid, and the + empties are recoverable on a later attempt.""" + self._setup_agent(agent) + agent.base_url = "http://127.0.0.1:1234/v1" + zero_usage = { + "prompt_tokens": 25_900, + "completion_tokens": 0, + "total_tokens": 25_900, + } + empty_resp = _mock_response( + content=None, finish_reason="stop", usage=zero_usage + ) + recovered = _mock_response(content="recovered", finish_reason="stop") + # Two empties then a success: the guard must not cut the retry + # that recovers the turn. + agent.client.chat.completions.create.side_effect = [ + empty_resp, + empty_resp, + recovered, + ] + with ( + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + patch( + "agent.empty_response_guard._estimate_attempt_cost", + return_value=None, + ), + ): + result = agent.run_conversation("answer me") + assert result["completed"] is True + assert result["final_response"] == "recovered" + assert result["api_calls"] == 3 + def test_guard_disabled_via_config_restores_legacy_retries(self, agent): """NS-503: agent.empty_response_guard.enabled: false in config.yaml (resolved to _empty_guard_enabled at init) restores the legacy