fix(agent): handle usage-less empty responses
This commit is contained in:
@@ -4609,6 +4609,11 @@ def run_conversation(
|
||||
"error": "First response truncated due to output length limit"
|
||||
}
|
||||
|
||||
# Count every completed provider attempt, including providers
|
||||
# that omit usage. Token/cost accounting below remains gated on
|
||||
# real usage, but the request itself must stay observable.
|
||||
agent.session_api_calls += 1
|
||||
|
||||
# Track actual token usage from response for context management
|
||||
if hasattr(response, 'usage') and response.usage:
|
||||
canonical_usage = normalize_usage(
|
||||
@@ -4783,7 +4788,6 @@ def run_conversation(
|
||||
agent.session_prompt_tokens += prompt_tokens
|
||||
agent.session_completion_tokens += completion_tokens
|
||||
agent.session_total_tokens += total_tokens
|
||||
agent.session_api_calls += 1
|
||||
agent.session_input_tokens += canonical_usage.input_tokens
|
||||
agent.session_output_tokens += canonical_usage.output_tokens
|
||||
agent.session_cache_read_tokens += canonical_usage.cache_read_tokens
|
||||
@@ -4930,6 +4934,15 @@ def run_conversation(
|
||||
f"{cached:,}/{prompt:,} tokens "
|
||||
f"({hit_pct:.0f}% hit, {written:,} written)"
|
||||
)
|
||||
else:
|
||||
logger.info(
|
||||
"API call #%d: model=%s provider=%s in=? out=? total=? "
|
||||
"latency=%.1fs usage=unavailable",
|
||||
agent.session_api_calls,
|
||||
agent.model,
|
||||
agent.provider or "unknown",
|
||||
api_duration,
|
||||
)
|
||||
|
||||
_retry.has_retried_429 = False # Reset on success
|
||||
# Note: don't clear the retry buffer here — an "API call
|
||||
@@ -8552,6 +8565,7 @@ def run_conversation(
|
||||
agent,
|
||||
finish_reason=finish_reason,
|
||||
response=response,
|
||||
observed_generation=_has_structured,
|
||||
)
|
||||
_empty_retry_budget = (
|
||||
_empty_guard.empty_retry_budget(agent, response)
|
||||
@@ -8619,15 +8633,14 @@ def run_conversation(
|
||||
|
||||
if _truly_empty and _deterministic_empty:
|
||||
logger.warning(
|
||||
"Deterministic empty response detected "
|
||||
"(consecutive zero-output completions, "
|
||||
"model=%s provider=%s finish_reason=%s) — "
|
||||
"Repeated empty response detected "
|
||||
"(model=%s provider=%s finish_reason=%s) — "
|
||||
"skipping remaining retries",
|
||||
agent.model, agent.provider, finish_reason,
|
||||
)
|
||||
agent._buffer_status(
|
||||
"⚠️ Model is deterministically returning empty "
|
||||
"(zero output tokens) — skipping further retries "
|
||||
"⚠️ Model is repeatedly returning empty content — "
|
||||
"skipping further retries "
|
||||
"to avoid repeat charges"
|
||||
)
|
||||
|
||||
@@ -8748,7 +8761,12 @@ def run_conversation(
|
||||
"answer:\n\n" + reasoning_preview
|
||||
)
|
||||
else:
|
||||
final_response = "(empty)"
|
||||
final_response = agent._format_turn_completion_explanation(
|
||||
_turn_exit_reason
|
||||
) or (
|
||||
"⚠️ No reply: the model returned empty content after "
|
||||
"all retries and fallback attempts."
|
||||
)
|
||||
break
|
||||
|
||||
# Reset retry counter/signature on successful content
|
||||
|
||||
@@ -15,15 +15,13 @@ like this).
|
||||
|
||||
Two independent guards, both failing OPEN to today's behaviour:
|
||||
|
||||
1. **Deterministic-empty detection** — two consecutive empty attempts,
|
||||
both with usage present and ``output_tokens == 0``, from the same
|
||||
(model, provider, finish_reason), are treated as deterministic: the
|
||||
same prompt will keep producing the same empty. Remaining retries are
|
||||
skipped and the loop proceeds straight to the fallback chain (a
|
||||
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.
|
||||
1. **Deterministic-empty detection** — two consecutive empty attempts from
|
||||
the same (model, provider, finish_reason) are treated as deterministic
|
||||
when usage proves zero output, or when usage is absent and the assembled
|
||||
responses contain neither content nor reasoning. Remaining retries are
|
||||
skipped and the loop proceeds straight to the fallback chain (a different
|
||||
model may behave differently). Mixed evidence or any generated tokens keep
|
||||
the full retry budget.
|
||||
|
||||
2. **Cost-aware retry budget** — when the estimated input cost of a
|
||||
single empty attempt exceeds the configured threshold (default
|
||||
@@ -78,6 +76,7 @@ class EmptyAttempt:
|
||||
finish_reason: str
|
||||
usage_present: bool
|
||||
zero_output: bool
|
||||
observed_generation: bool
|
||||
|
||||
@property
|
||||
def signature(self) -> tuple:
|
||||
@@ -201,7 +200,13 @@ def _zero_output(agent: Any, response: Any) -> tuple:
|
||||
return (True, (output + reasoning) == 0)
|
||||
|
||||
|
||||
def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> None:
|
||||
def record_empty_attempt(
|
||||
agent: Any,
|
||||
*,
|
||||
finish_reason: str,
|
||||
response: Any,
|
||||
observed_generation: bool = True,
|
||||
) -> None:
|
||||
"""Record one empty completion in the current streak.
|
||||
|
||||
Must be called before ``_empty_content_retries`` is incremented for
|
||||
@@ -222,6 +227,7 @@ def record_empty_attempt(agent: Any, *, finish_reason: str, response: Any) -> No
|
||||
finish_reason=str(finish_reason or ""),
|
||||
usage_present=usage_present,
|
||||
zero_output=zero_output,
|
||||
observed_generation=bool(observed_generation),
|
||||
)
|
||||
)
|
||||
|
||||
@@ -234,10 +240,10 @@ 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 >= 2 consecutive attempts with an identical (model, provider,
|
||||
finish_reason) signature. Usage-backed attempts must all prove zero output.
|
||||
Usage-absent attempts must all have no observed content or reasoning. Mixed
|
||||
evidence fails open so ambiguous transients keep their retries.
|
||||
"""
|
||||
if not guard_enabled(agent):
|
||||
return False
|
||||
@@ -245,10 +251,12 @@ def deterministic_empty(agent: Any) -> bool:
|
||||
if len(attempts) < 2:
|
||||
return False
|
||||
first = attempts[0]
|
||||
return all(
|
||||
a.usage_present and a.zero_output and a.signature == first.signature
|
||||
for a in attempts
|
||||
same_signature = all(a.signature == first.signature for a in attempts)
|
||||
usage_proves_empty = all(a.usage_present and a.zero_output for a in attempts)
|
||||
response_proves_empty = all(
|
||||
not a.usage_present and not a.observed_generation for a in attempts
|
||||
)
|
||||
return same_signature and (usage_proves_empty or response_proves_empty)
|
||||
|
||||
|
||||
def empty_retry_budget(agent: Any, response: Any) -> int:
|
||||
|
||||
@@ -5,7 +5,8 @@ deterministic empties (unsignaled provider refusals with zero output
|
||||
tokens) while never tightening behaviour on ambiguous evidence.
|
||||
|
||||
Fail-open contract under test:
|
||||
- Missing usage -> never deterministic, default budget.
|
||||
- Missing usage + no observed generation -> deterministic after two attempts.
|
||||
- Missing usage + observed reasoning -> never deterministic.
|
||||
- Any generated tokens (output or reasoning) -> never deterministic.
|
||||
- Different model/provider/finish_reason across attempts -> not deterministic.
|
||||
- Guard disabled via config (agent.empty_response_guard.enabled: false) ->
|
||||
@@ -42,11 +43,21 @@ def _response(prompt_tokens=25_900, completion_tokens=0, usage_present=True):
|
||||
return SimpleNamespace(usage=usage)
|
||||
|
||||
|
||||
def _record_streak(agent, responses, finish_reasons=None):
|
||||
def _record_streak(
|
||||
agent, responses, finish_reasons=None, observed_generations=None
|
||||
):
|
||||
"""Record attempts the way the loop does: record, then increment."""
|
||||
finish_reasons = finish_reasons or ["stop"] * len(responses)
|
||||
for resp, reason in zip(responses, finish_reasons):
|
||||
guard.record_empty_attempt(agent, finish_reason=reason, response=resp)
|
||||
observed_generations = observed_generations or [False] * len(responses)
|
||||
for resp, reason, observed_generation in zip(
|
||||
responses, finish_reasons, observed_generations
|
||||
):
|
||||
guard.record_empty_attempt(
|
||||
agent,
|
||||
finish_reason=reason,
|
||||
response=resp,
|
||||
observed_generation=observed_generation,
|
||||
)
|
||||
agent._empty_content_retries += 1
|
||||
|
||||
|
||||
@@ -62,12 +73,21 @@ class TestDeterministicEmpty:
|
||||
_record_streak(agent, [_response()])
|
||||
assert guard.deterministic_empty(agent) is False
|
||||
|
||||
def test_missing_usage_fails_open(self):
|
||||
def test_missing_usage_without_observed_generation_is_deterministic(self):
|
||||
agent = _agent()
|
||||
_record_streak(
|
||||
agent,
|
||||
[_response(usage_present=False), _response(usage_present=False)],
|
||||
)
|
||||
assert guard.deterministic_empty(agent) is True
|
||||
|
||||
def test_missing_usage_with_observed_reasoning_fails_open(self):
|
||||
agent = _agent()
|
||||
_record_streak(
|
||||
agent,
|
||||
[_response(usage_present=False), _response(usage_present=False)],
|
||||
observed_generations=[True, True],
|
||||
)
|
||||
assert guard.deterministic_empty(agent) is False
|
||||
|
||||
def test_mixed_usage_presence_fails_open(self):
|
||||
|
||||
@@ -11,7 +11,7 @@ Invariants pinned here:
|
||||
``_empty_terminal_sentinel`` marker (replay semantics unchanged).
|
||||
- Raw reasoning is NEVER promoted earlier in the ladder — a reasoning-only
|
||||
response still goes through prefill continuation first.
|
||||
- A truly empty exhaustion (no reasoning either) still returns "(empty)".
|
||||
- A truly empty exhaustion returns an actionable no-reply explanation.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -109,11 +109,11 @@ def test_exhausted_reasoning_only_delivers_labeled_excerpt(tmp_path, monkeypatch
|
||||
)
|
||||
|
||||
|
||||
def test_exhausted_truly_empty_keeps_existing_behavior(tmp_path, monkeypatch):
|
||||
"""No reasoning anywhere → behavior unchanged from main: the '(empty)'
|
||||
terminal (possibly rewritten by the downstream turn-completion explainer)
|
||||
is delivered, and no reasoning excerpt appears."""
|
||||
def test_exhausted_truly_empty_delivers_no_reply_explanation(tmp_path, monkeypatch):
|
||||
"""No reasoning anywhere produces an honest terminal explanation even
|
||||
when the downstream completion explainer is disabled."""
|
||||
agent = _build_agent(tmp_path, monkeypatch)
|
||||
monkeypatch.setattr(agent, "_turn_completion_explainer_enabled", lambda: False)
|
||||
monkeypatch.setattr(
|
||||
agent, "_interruptible_api_call",
|
||||
lambda api_kwargs: _truly_empty_response(),
|
||||
@@ -122,9 +122,7 @@ def test_exhausted_truly_empty_keeps_existing_behavior(tmp_path, monkeypatch):
|
||||
result = agent.run_conversation("hello?")
|
||||
|
||||
final = result["final_response"]
|
||||
# Either the raw sentinel (explainer off) or the explainer's rewrite —
|
||||
# never the reasoning-excerpt frame, which requires reasoning to exist.
|
||||
assert final == "(empty)" or final.startswith("⚠️ No reply:")
|
||||
assert final.startswith("⚠️ No reply:")
|
||||
assert "only internal reasoning" not in final
|
||||
|
||||
|
||||
|
||||
@@ -3513,12 +3513,12 @@ class TestRunConversation:
|
||||
assert result["api_calls"] == 6 # 1 original + 2 prefill + 3 retries
|
||||
|
||||
|
||||
def test_truly_empty_response_retries_3_times_then_empty(self, agent):
|
||||
"""Truly empty response (no content, no reasoning) retries 3 times then falls through to (empty)."""
|
||||
def test_truly_empty_response_stops_after_repeated_empty(self, agent):
|
||||
"""Repeated empty responses stop after one retry and return an explanation."""
|
||||
self._setup_agent(agent)
|
||||
agent.base_url = "http://127.0.0.1:1234/v1"
|
||||
empty_resp = _mock_response(content=None, finish_reason="stop")
|
||||
# 4 responses: 1 original + 3 nudge retries, all empty
|
||||
# Extra responses prove the guard stops consuming after repetition.
|
||||
agent.client.chat.completions.create.side_effect = [
|
||||
empty_resp, empty_resp, empty_resp, empty_resp,
|
||||
]
|
||||
@@ -3532,7 +3532,7 @@ class TestRunConversation:
|
||||
# #34452: explanation replaces the bare "(empty)" sentinel.
|
||||
assert result["final_response"] != "(empty)"
|
||||
assert "No reply:" in result["final_response"]
|
||||
assert result["api_calls"] == 4 # 1 original + 3 retries
|
||||
assert result["api_calls"] == 2 # 1 original + 1 retry
|
||||
|
||||
def test_deterministic_empty_stops_retries_early(self, agent):
|
||||
"""NS-503: consecutive zero-output-token empties with identical
|
||||
@@ -3588,10 +3588,11 @@ class TestRunConversation:
|
||||
assert result["completed"] is True
|
||||
assert result["api_calls"] == 4 # legacy: 1 original + 3 retries
|
||||
|
||||
def test_empty_without_usage_keeps_full_retry_budget(self, agent):
|
||||
"""NS-503 fail-open: no usage data means no evidence of a
|
||||
deterministic empty — legacy 3-retry behaviour must be preserved
|
||||
(this is the flaky-provider case retries exist for)."""
|
||||
def test_empty_without_usage_stops_after_one_retry_and_logs_calls(
|
||||
self, agent, caplog
|
||||
):
|
||||
"""Two complete empty responses are enough evidence to stop even when
|
||||
the provider omits usage; both attempts remain observable."""
|
||||
self._setup_agent(agent)
|
||||
agent.base_url = "http://127.0.0.1:1234/v1"
|
||||
empty_resp = _mock_response(content=None, finish_reason="stop")
|
||||
@@ -3600,10 +3601,17 @@ class TestRunConversation:
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
patch.object(
|
||||
agent, "_turn_completion_explainer_enabled", return_value=False
|
||||
),
|
||||
caplog.at_level(logging.INFO, logger="agent.conversation_loop"),
|
||||
):
|
||||
result = agent.run_conversation("answer me")
|
||||
assert result["completed"] is True
|
||||
assert result["api_calls"] == 4 # unchanged: 1 original + 3 retries
|
||||
assert result["api_calls"] == 2
|
||||
assert agent.session_api_calls == 2
|
||||
assert result["final_response"].startswith("⚠️ No reply:")
|
||||
assert caplog.text.count("usage=unavailable") == 2
|
||||
|
||||
def test_truly_empty_response_succeeds_on_nudge(self, agent):
|
||||
"""Model produces content after being nudged for empty response."""
|
||||
|
||||
Reference in New Issue
Block a user