fix(compressor): abort compression on empty-content provider degradation to prevent context loss (#94448)
When an auxiliary or main summarizer LLM returns an HTTP 200 with an empty or whitespace-only response (e.g., degraded provider/channel), abort compression and preserve the full conversation context rather than falling through to the destructive static-fallback branch that drops the middle window. - Track _last_summary_empty_content_failure across _generate_summary() and compress() - Attempt fallback to the main model when an aux model returns empty content - Abort compression and preserve all messages intact if no valid summary can be generated - Record summary_empty_content_failure in telemetry and log actionable diagnostic guidance - Add comprehensive unit tests in tests/agent/test_context_compressor.py Fixes #94448
This commit is contained in:
+55
-22
@@ -3325,6 +3325,12 @@ class ContextCompressor(ContextEngine):
|
||||
# strictly better than discarding context for a transient blip
|
||||
# (#29559, #25585). Independent of abort_on_summary_failure.
|
||||
self._last_summary_network_failure: bool = False
|
||||
# Set when summary generation ultimately fails due to the provider
|
||||
# returning empty or whitespace content (HTTP 200 null body / degraded proxy
|
||||
# channel). Like network/auth failures, compress() must ABORT and preserve
|
||||
# the session unchanged instead of destroying the middle window for a
|
||||
# deterministic placeholder (#94448). Independent of abort_on_summary_failure.
|
||||
self._last_summary_empty_content_failure: bool = False
|
||||
# retrying on the main model, record the failure so gateway /
|
||||
# CLI callers can still warn the user even though compression
|
||||
# succeeded. Silent recovery would hide the broken config.
|
||||
@@ -5025,7 +5031,11 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
# exists, not that it's an object with ``.content``. Some
|
||||
# OpenAI-compatible proxies / local backends return a dict- or
|
||||
# str-shaped message; coerce defensively instead of crashing.
|
||||
message = response.choices[0].message
|
||||
if isinstance(response, dict):
|
||||
choices = response.get("choices") or [{}]
|
||||
message = choices[0].get("message") if isinstance(choices[0], dict) else getattr(choices[0], "message", None)
|
||||
else:
|
||||
message = response.choices[0].message
|
||||
if isinstance(message, dict):
|
||||
content = message.get("content")
|
||||
else:
|
||||
@@ -5075,6 +5085,7 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
self._last_summary_error = None
|
||||
self._last_summary_auth_failure = False
|
||||
self._last_summary_network_failure = False
|
||||
self._last_summary_empty_content_failure = False
|
||||
return self._with_summary_prefix(summary)
|
||||
except Exception as e:
|
||||
# ``call_llm`` raises ``RuntimeError`` for two very different cases:
|
||||
@@ -5137,6 +5148,11 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
# back to the main model instead of entering a 60-second cooldown.
|
||||
# See issue #18458.
|
||||
_is_streaming_closed = _is_connection_error(e)
|
||||
# Provider returned HTTP 200 with empty or whitespace body (e.g.
|
||||
# degraded proxy channel / upstream provider fault; #94448).
|
||||
_is_empty_content = (
|
||||
isinstance(e, RuntimeError) and "empty content" in _err_str
|
||||
)
|
||||
# Authentication, permission, and exhausted-quota failures are NOT
|
||||
# transient or fixable by retrying the same request. Flag them so
|
||||
# compress() preserves the session instead of rotating into a
|
||||
@@ -5162,13 +5178,15 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
e,
|
||||
)
|
||||
if (
|
||||
(_is_model_not_found or _is_timeout or _is_json_decode or _is_streaming_closed)
|
||||
(_is_model_not_found or _is_timeout or _is_json_decode or _is_streaming_closed or _is_empty_content)
|
||||
and self.summary_model
|
||||
and self.summary_model != self.model
|
||||
and not getattr(self, "_summary_model_fallen_back", False)
|
||||
):
|
||||
if _is_json_decode:
|
||||
_reason = "returned invalid JSON"
|
||||
elif _is_empty_content:
|
||||
_reason = "returned empty content"
|
||||
elif _is_model_not_found:
|
||||
_reason = "unavailable"
|
||||
elif _is_streaming_closed:
|
||||
@@ -5226,7 +5244,7 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
min(self._consecutive_timeout_failures,
|
||||
len(_TIMEOUT_COOLDOWN_LADDER)) - 1
|
||||
]
|
||||
elif _is_json_decode or _is_streaming_closed:
|
||||
elif _is_json_decode or _is_streaming_closed or _is_empty_content:
|
||||
_transient_cooldown = 30
|
||||
else:
|
||||
_transient_cooldown = 60
|
||||
@@ -5235,15 +5253,18 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
err_text = err_text[:217].rstrip() + "..."
|
||||
self._record_compression_failure_cooldown(_transient_cooldown, err_text)
|
||||
self._last_summary_error = err_text
|
||||
# A terminal connection/network failure (we reach this branch only
|
||||
# after any main-model fallback has already been tried or is
|
||||
# unavailable). Flag it so compress() ABORTS and preserves the
|
||||
# session unchanged instead of destroying the middle window for a
|
||||
# placeholder marker — retrying once the network recovers is
|
||||
# strictly better than dropping context (#29559, #25585). Mirrors
|
||||
# the auth-failure carve-out; independent of abort_on_summary_failure.
|
||||
# A terminal connection/network failure or empty-content response
|
||||
# from a degraded provider (we reach this branch only after any
|
||||
# main-model fallback has already been tried or is unavailable).
|
||||
# Flag it so compress() ABORTS and preserves the session unchanged
|
||||
# instead of destroying the middle window for a placeholder
|
||||
# marker — retrying once the provider recovers is strictly better
|
||||
# than dropping context (#29559, #25585, #94448). Mirrors the
|
||||
# auth-failure carve-out; independent of abort_on_summary_failure.
|
||||
if _is_streaming_closed:
|
||||
self._last_summary_network_failure = True
|
||||
elif _is_empty_content:
|
||||
self._last_summary_empty_content_failure = True
|
||||
logger.warning(
|
||||
"Failed to generate context summary: %s. "
|
||||
"Further summary attempts paused for %d seconds.",
|
||||
@@ -7284,10 +7305,10 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
self._last_compress_aborted = False
|
||||
self._last_compress_refused_would_grow = False
|
||||
self._last_compression_made_progress = False
|
||||
# NOTE: do NOT reset _last_summary_auth_failure or
|
||||
# _last_summary_network_failure here. These flags are set by
|
||||
# _generate_summary() on a terminal failure and are already cleared on
|
||||
# a successful summary. Resetting them eagerly defeats the cooldown
|
||||
# NOTE: do NOT reset _last_summary_auth_failure,
|
||||
# _last_summary_network_failure, or _last_summary_empty_content_failure
|
||||
# here. These flags are set by _generate_summary() on a terminal
|
||||
# failure and are already cleared on a successful summary. Resetting them eagerly defeats the cooldown
|
||||
# protection: _generate_summary() returns None from the cooldown
|
||||
# early-return without re-asserting these flags, so the abort guard
|
||||
# below would see False and fall through to the destructive
|
||||
@@ -7642,18 +7663,19 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
# surface a warning.
|
||||
# Default is False (historical behavior).
|
||||
#
|
||||
# EXCEPTION — terminal access/quota AND transient network failures
|
||||
# always abort. Missing credentials, 401/402/403 access failures, and
|
||||
# confirmed non-resetting quota exhaustion cannot be repaired by
|
||||
# retrying the same summary request. A connection/stream-close error
|
||||
# means the network blipped at the compaction moment (#29559). In all
|
||||
# of these cases, rotating into a child session with a placeholder
|
||||
# summary degrades the conversation for zero benefit. Preserve it
|
||||
# unchanged until access is restored or connectivity recovers.
|
||||
# EXCEPTION — terminal access/quota, transient network failures, and
|
||||
# empty-content provider degradation always abort. Missing credentials,
|
||||
# 401/402/403 access failures, confirmed non-resetting quota exhaustion,
|
||||
# and HTTP 200 empty responses from degraded channels cannot be repaired
|
||||
# by immediately generating a static placeholder. In all of these cases,
|
||||
# rotating into a child session with a placeholder summary degrades the
|
||||
# conversation for zero benefit. Preserve it unchanged until access or
|
||||
# provider health is restored (#29559, #25585, #94448).
|
||||
if not summary and not feasibility_skip and (
|
||||
self.abort_on_summary_failure
|
||||
or self._last_summary_auth_failure
|
||||
or self._last_summary_network_failure
|
||||
or self._last_summary_empty_content_failure
|
||||
):
|
||||
n_skipped = compress_end - compress_start
|
||||
self._last_summary_dropped_count = 0 # nothing actually dropped
|
||||
@@ -7663,6 +7685,8 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
telemetry["failure_class"] = "summary_auth_failure"
|
||||
elif self._last_summary_network_failure:
|
||||
telemetry["failure_class"] = "summary_network_failure"
|
||||
elif self._last_summary_empty_content_failure:
|
||||
telemetry["failure_class"] = "summary_empty_content_failure"
|
||||
else:
|
||||
telemetry["failure_class"] = "summary_generation_aborted"
|
||||
# Roll back the self-heal rehydration so this aborted attempt is a
|
||||
@@ -7690,6 +7714,15 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
"recovers, or continue the conversation as-is.",
|
||||
n_skipped,
|
||||
)
|
||||
elif self._last_summary_empty_content_failure:
|
||||
logger.warning(
|
||||
"Summary generation failed (LLM returned empty content) — "
|
||||
"aborting compression. %d message(s) preserved unchanged; "
|
||||
"the session was NOT rotated. This indicates upstream provider "
|
||||
"degradation: retry with /compress once the provider recovers, "
|
||||
"or continue the conversation as-is.",
|
||||
n_skipped,
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
"Summary generation failed — aborting compression "
|
||||
|
||||
@@ -927,7 +927,44 @@ class TestAuthFailureAborts:
|
||||
assert c._last_summary_network_failure is True
|
||||
assert c._last_summary_auth_failure is False
|
||||
|
||||
def test_generate_summary_flags_empty_content_failure(self):
|
||||
"""An empty-content response on the summary call flags
|
||||
_last_summary_empty_content_failure (#94448)."""
|
||||
with patch("agent.context_compressor.get_model_context_length", return_value=100000):
|
||||
c = ContextCompressor(model="test", quiet_mode=True)
|
||||
with patch(
|
||||
"agent.context_compressor.call_llm",
|
||||
return_value={"choices": [{"message": {"content": " "}}]},
|
||||
):
|
||||
result = c._generate_summary(self._msgs())
|
||||
assert result is None
|
||||
assert c._last_summary_empty_content_failure is True
|
||||
assert c._last_summary_auth_failure is False
|
||||
assert c._last_summary_network_failure is False
|
||||
|
||||
def test_empty_content_summary_aborts_compression_and_preserves_messages(self):
|
||||
"""Empty-content response from degraded provider aborts compression and
|
||||
preserves original messages without dropping context (#94448)."""
|
||||
with patch("agent.context_compressor.get_model_context_length", return_value=100000):
|
||||
c = ContextCompressor(
|
||||
model="test",
|
||||
quiet_mode=True,
|
||||
protect_first_n=2,
|
||||
protect_last_n=2,
|
||||
abort_on_summary_failure=False,
|
||||
)
|
||||
msgs = self._msgs(12)
|
||||
with patch(
|
||||
"agent.context_compressor.call_llm",
|
||||
return_value={"choices": [{"message": {"content": ""}}]},
|
||||
):
|
||||
result = c.compress(msgs, current_tokens=999999, force=True)
|
||||
|
||||
assert result == msgs
|
||||
assert c._last_summary_empty_content_failure is True
|
||||
assert c._last_compress_aborted is True
|
||||
assert c._last_summary_fallback_used is False
|
||||
assert c._last_summary_dropped_count == 0
|
||||
|
||||
|
||||
class TestSummaryFallbackToMainModel:
|
||||
@@ -980,6 +1017,35 @@ class TestSummaryFallbackToMainModel:
|
||||
assert c._last_aux_model_failure_error is not None
|
||||
assert "404" in c._last_aux_model_failure_error
|
||||
|
||||
def test_empty_content_falls_back_to_main_and_succeeds(self):
|
||||
"""Aux model returns empty content -> falls back to main model -> succeeds (#94448)."""
|
||||
mock_ok = MagicMock()
|
||||
mock_ok.choices = [MagicMock()]
|
||||
mock_ok.choices[0].message.content = "summary via main model after empty aux"
|
||||
|
||||
with patch("agent.context_compressor.get_model_context_length", return_value=100000):
|
||||
c = ContextCompressor(
|
||||
model="main-model",
|
||||
summary_model_override="flaky-aux-model",
|
||||
quiet_mode=True,
|
||||
)
|
||||
|
||||
with patch(
|
||||
"agent.context_compressor.call_llm",
|
||||
side_effect=[
|
||||
{"choices": [{"message": {"content": " "}}]},
|
||||
mock_ok,
|
||||
],
|
||||
) as mock_call:
|
||||
result = c._generate_summary(self._msgs())
|
||||
|
||||
assert mock_call.call_count == 2
|
||||
assert mock_call.call_args_list[0].kwargs.get("model") == "flaky-aux-model"
|
||||
assert "model" not in mock_call.call_args_list[1].kwargs
|
||||
assert result is not None
|
||||
assert "summary via main model after empty aux" in result
|
||||
assert c._last_aux_model_failure_model == "flaky-aux-model"
|
||||
assert "empty content" in (c._last_aux_model_failure_error or "").lower()
|
||||
|
||||
def test_no_fallback_when_summary_model_equals_main_model(self):
|
||||
"""If the aux model IS the main model, there's nowhere to fall back
|
||||
|
||||
Reference in New Issue
Block a user