fix(summarizer): reject degenerate max_tokens summaries (#899)
A model that loops until the output cap returns non-empty text, so the existing empty-content check lets it through and it overwrites the last good summary. Discard a summary only when it both hit the output cap and scores below a distinct-4-gram ratio of 0.35: set is_fallback so the previous summary is retained, zero the token counts on that path, and count the rejection via summary_rejections_counter.
This commit is contained in:
parent
0d57df430e
commit
52f4ba99f8
|
|
@ -117,6 +117,12 @@ deriver_tokens_processed_counter = NamespacedCounter(
|
|||
["namespace", "task_type", "token_type", "component"],
|
||||
)
|
||||
|
||||
summary_rejections_counter = NamespacedCounter(
|
||||
"summary_rejections",
|
||||
"Total summaries rejected by validation before persistence",
|
||||
["namespace", "summary_type", "reason"],
|
||||
)
|
||||
|
||||
dialectic_tokens_processed_counter = NamespacedCounter(
|
||||
"dialectic_tokens_processed",
|
||||
"Total tokens processed by the dialectic",
|
||||
|
|
@ -274,6 +280,14 @@ class PrometheusMetrics:
|
|||
except Exception as e:
|
||||
self._handle_metric_error("record_deriver_tokens", e)
|
||||
|
||||
def record_summary_rejection(self, *, summary_type: str, reason: str) -> None:
|
||||
try:
|
||||
summary_rejections_counter.labels(
|
||||
summary_type=summary_type, reason=reason
|
||||
).inc()
|
||||
except Exception as e:
|
||||
self._handle_metric_error("record_summary_rejection", e)
|
||||
|
||||
def record_dialectic_tokens(
|
||||
self,
|
||||
*,
|
||||
|
|
|
|||
|
|
@ -98,6 +98,45 @@ class SummaryType(Enum):
|
|||
LONG = "honcho_chat_summary_long"
|
||||
|
||||
|
||||
# Degeneration guard (#899). A model that loops until the output cap emits non-empty
|
||||
# text, so the empty-check in _create_summary can't catch it. distinct-4 is chosen over
|
||||
# zlib compression ratio because it is length-stable: measured 1.00 on clean prose from
|
||||
# 3.7k-30k chars, while zlib drifts 2.17 -> 2.50 and would cross Whisper's 2.4 threshold
|
||||
# on perfectly good long text. Against this PR's fixtures the #899 degenerate loop scores
|
||||
# 0.003 while heavily templated but legitimate prose that also hits the cap scores 0.467,
|
||||
# so 0.35 leaves the clean case a 1.33x margin and the degenerate case two orders below.
|
||||
_DEGENERATE_NGRAM_N = 4
|
||||
_DEGENERATE_DISTINCT_RATIO = 0.35
|
||||
# Provider-native cap-hit spellings; never normalized by completion_result_to_response.
|
||||
_CAP_FINISH_REASONS = frozenset({"max_tokens", "length"})
|
||||
|
||||
|
||||
def _distinct_ngram_ratio(text: str, n: int = _DEGENERATE_NGRAM_N) -> float:
|
||||
"""Fraction of n-grams in `text` that are unique. 1.0 = no repetition."""
|
||||
words = text.split()
|
||||
if len(words) < n:
|
||||
return 1.0
|
||||
grams = [tuple(words[i : i + n]) for i in range(len(words) - n + 1)]
|
||||
return len(set(grams)) / len(grams)
|
||||
|
||||
|
||||
def _hit_output_cap(finish_reasons: list[str]) -> bool:
|
||||
"""True when any provider reported stopping at the output-token cap."""
|
||||
return any(r.lower() in _CAP_FINISH_REASONS for r in finish_reasons)
|
||||
|
||||
|
||||
def _basic_fallback_summary(
|
||||
message_count: int, last_message_content_preview: str
|
||||
) -> str:
|
||||
"""Deterministic one-liner used when the LLM summary is unusable."""
|
||||
if message_count <= 0:
|
||||
return ""
|
||||
return (
|
||||
f"Conversation with {message_count} messages about "
|
||||
f"{last_message_content_preview}..."
|
||||
)
|
||||
|
||||
|
||||
def short_summary_prompt(
|
||||
formatted_messages: str,
|
||||
output_words: int,
|
||||
|
|
@ -615,21 +654,44 @@ async def _create_summary(
|
|||
response.finish_reasons,
|
||||
)
|
||||
is_fallback = True
|
||||
summary_text = (
|
||||
f"Conversation with {message_count} messages about {last_message_content_preview}..."
|
||||
if message_count > 0
|
||||
else ""
|
||||
summary_text = _basic_fallback_summary(
|
||||
message_count, last_message_content_preview
|
||||
)
|
||||
summary_tokens = estimate_tokens(summary_text) if summary_text else 0
|
||||
llm_input_tokens = 0
|
||||
llm_output_tokens = 0
|
||||
elif _hit_output_cap(response.finish_reasons):
|
||||
repetition_ratio = _distinct_ngram_ratio(summary_text)
|
||||
if repetition_ratio < _DEGENERATE_DISTINCT_RATIO:
|
||||
logger.error(
|
||||
(
|
||||
"Generated %s summary is degenerate: hit the output cap "
|
||||
"(finish_reasons=%s) with a distinct-%d ratio of %.3f < %.2f. "
|
||||
"Discarding; the previous summary is retained."
|
||||
),
|
||||
summary_type.name,
|
||||
response.finish_reasons,
|
||||
_DEGENERATE_NGRAM_N,
|
||||
repetition_ratio,
|
||||
_DEGENERATE_DISTINCT_RATIO,
|
||||
)
|
||||
is_fallback = True
|
||||
summary_text = _basic_fallback_summary(
|
||||
message_count, last_message_content_preview
|
||||
)
|
||||
summary_tokens = estimate_tokens(summary_text) if summary_text else 0
|
||||
llm_input_tokens = 0 # match the documented fallback contract
|
||||
llm_output_tokens = 0
|
||||
if settings.METRICS.ENABLED:
|
||||
prometheus_metrics.record_summary_rejection(
|
||||
summary_type=summary_type.name.lower(),
|
||||
reason="degenerate_repetition",
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Error generating summary!")
|
||||
# Fallback to a basic summary in case of error
|
||||
summary_text = (
|
||||
f"Conversation with {message_count} messages about {last_message_content_preview}..."
|
||||
if message_count > 0
|
||||
else ""
|
||||
summary_text = _basic_fallback_summary(
|
||||
message_count, last_message_content_preview
|
||||
)
|
||||
summary_tokens = 0
|
||||
is_fallback = True
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ from src.schemas import (
|
|||
from src.telemetry.prometheus.metrics import (
|
||||
deriver_tokens_processed_counter,
|
||||
dialectic_tokens_processed_counter,
|
||||
summary_rejections_counter,
|
||||
)
|
||||
from src.utils.representation import ExplicitObservationBase, PromptRepresentation
|
||||
from src.utils.summarizer import (
|
||||
|
|
@ -385,6 +386,76 @@ class TestDeriverIngestionMetrics:
|
|||
assert delta > 0, f"Expected messages input tokens > 0, got {delta}"
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Summary degeneration guard (#899)
|
||||
# =============================================================================
|
||||
|
||||
_DEGENERATE_SUMMARY_TEXT = "Human. Forever. Human. Always. Human value. Always. " * 300
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestSummaryDegenerationGuard:
|
||||
"""Persistence-boundary tests for the #899 degeneration guard."""
|
||||
|
||||
async def test_degenerate_cap_hit_does_not_save_summary(
|
||||
self,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
prometheus_test_setup: PrometheusMetricChecker,
|
||||
):
|
||||
"""Degenerate max_tokens summary must not call _save_summary and must count rejection."""
|
||||
metric_checker = prometheus_test_setup
|
||||
workspace, peer = sample_data
|
||||
session = await create_test_session_with_peer(db_session, workspace, peer)
|
||||
messages = await create_test_messages(
|
||||
db_session, workspace.name, session.name, peer.name, count=5
|
||||
)
|
||||
last_message = messages[-1]
|
||||
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_DEGENERATE_SUMMARY_TEXT,
|
||||
input_tokens=20000,
|
||||
output_tokens=4000,
|
||||
finish_reasons=["max_tokens"],
|
||||
)
|
||||
|
||||
rejection_labels = {
|
||||
"namespace": "test",
|
||||
"summary_type": "long",
|
||||
"reason": "degenerate_repetition",
|
||||
}
|
||||
before = metric_checker.capture(summary_rejections_counter, rejection_labels)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new=AsyncMock(return_value=mock_response),
|
||||
),
|
||||
patch(
|
||||
"src.utils.summarizer._save_summary",
|
||||
new=AsyncMock(),
|
||||
) as mock_save,
|
||||
):
|
||||
await _create_and_save_summary(
|
||||
workspace_name=workspace.name,
|
||||
session_name=session.name,
|
||||
message_id=last_message.id,
|
||||
message_seq_in_session=last_message.seq_in_session,
|
||||
summary_type=SummaryType.LONG,
|
||||
message_public_id=last_message.public_id,
|
||||
configuration=create_test_configuration(),
|
||||
)
|
||||
|
||||
mock_save.assert_not_awaited()
|
||||
metric_checker.assert_delta(
|
||||
summary_rejections_counter,
|
||||
rejection_labels,
|
||||
before,
|
||||
1,
|
||||
"Summary rejection counter",
|
||||
)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Deriver Summary Metrics Tests
|
||||
# =============================================================================
|
||||
|
|
|
|||
|
|
@ -27,6 +27,49 @@ _MESSAGE_PUBLIC_ID = "msg_abc123"
|
|||
_LAST_MESSAGE_ID = 42
|
||||
_LAST_MESSAGE_CONTENT_PREVIEW = "hello there how are you"
|
||||
_MESSAGE_COUNT = 5
|
||||
# Degenerate long-summary loop from #899 — non-empty, highly repetitive.
|
||||
_DEGENERATE_TEXT = "Human. Forever. Human. Always. Human value. Always. " * 300
|
||||
# Clean prose that can still hit the output cap (must not be rejected).
|
||||
# Distinct-4 ratio must stay well above 0.35 — do not build this by repeating
|
||||
# a single paragraph (that scores ~0.05 and would false-trigger the guard).
|
||||
_CLEAN_CAP_HIT_TOPICS = [
|
||||
"project planning",
|
||||
"deadline prioritization",
|
||||
"communication preferences",
|
||||
"quarterly goals",
|
||||
"beta launch readiness",
|
||||
"analytics rewrite deferral",
|
||||
"stakeholder alignment",
|
||||
"risk mitigation",
|
||||
"capacity planning",
|
||||
"design review feedback",
|
||||
"API contract changes",
|
||||
"migration sequencing",
|
||||
"observability gaps",
|
||||
"incident response drills",
|
||||
"onboarding materials",
|
||||
"vendor evaluation",
|
||||
"budget reforecast",
|
||||
"security audit findings",
|
||||
"customer interviews",
|
||||
"feature flag rollout",
|
||||
"performance baselines",
|
||||
"dependency upgrades",
|
||||
"test coverage targets",
|
||||
"release checklist",
|
||||
"team rituals",
|
||||
"documentation debt",
|
||||
"support handoff notes",
|
||||
"partner integrations",
|
||||
"data retention policy",
|
||||
"accessibility fixes",
|
||||
]
|
||||
_CLEAN_CAP_HIT_TEXT = " ".join(
|
||||
f"In discussion segment {i + 1}, the participants covered {topic}. "
|
||||
f"They agreed on concrete next steps, owners, and a follow-up date. "
|
||||
f"Open questions around {topic} were parked for the next working session."
|
||||
for i, topic in enumerate(_CLEAN_CAP_HIT_TOPICS)
|
||||
)
|
||||
|
||||
|
||||
async def _call_create_summary(
|
||||
|
|
@ -221,6 +264,107 @@ class TestCreateSummary:
|
|||
assert summary["content"] == ""
|
||||
assert summary["token_count"] == 0
|
||||
|
||||
@pytest.mark.parametrize("finish_reason", ["max_tokens", "length", "MAX_TOKENS"])
|
||||
async def test_degenerate_cap_hit_response_uses_fallback(self, finish_reason: str):
|
||||
"""A summary that loops until the output cap is discarded, not persisted (#899)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_DEGENERATE_TEXT,
|
||||
input_tokens=20000,
|
||||
output_tokens=settings.SUMMARY.MAX_TOKENS_LONG,
|
||||
finish_reasons=[finish_reason],
|
||||
)
|
||||
with patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, in_tok, out_tok = await _call_create_summary(
|
||||
SummaryType.LONG
|
||||
)
|
||||
|
||||
assert is_fallback is True
|
||||
assert "Human. Forever." not in summary["content"]
|
||||
assert (in_tok, out_tok) == (0, 0)
|
||||
|
||||
async def test_degenerate_cap_hit_short_summary_uses_fallback(self):
|
||||
"""Shared guard covers SHORT as well as LONG (#899)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_DEGENERATE_TEXT,
|
||||
input_tokens=5000,
|
||||
output_tokens=settings.SUMMARY.MAX_TOKENS_SHORT,
|
||||
finish_reasons=["max_tokens"],
|
||||
)
|
||||
with patch(
|
||||
"src.utils.summarizer.create_short_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, in_tok, out_tok = await _call_create_summary(
|
||||
SummaryType.SHORT
|
||||
)
|
||||
|
||||
assert is_fallback is True
|
||||
assert "Human. Forever." not in summary["content"]
|
||||
assert (in_tok, out_tok) == (0, 0)
|
||||
|
||||
async def test_degenerate_stop_finish_keeps_content(self):
|
||||
"""Cap-hit conjunct is required — stop + degenerate text is not rejected (#899)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_DEGENERATE_TEXT,
|
||||
input_tokens=20000,
|
||||
output_tokens=500,
|
||||
finish_reasons=["stop"],
|
||||
)
|
||||
with patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, _, _ = await _call_create_summary(SummaryType.LONG)
|
||||
|
||||
assert is_fallback is False
|
||||
assert "Human. Forever." in summary["content"]
|
||||
|
||||
async def test_degenerate_empty_finish_reasons_keeps_content(self):
|
||||
"""Empty finish_reasons must not reject (#899)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_DEGENERATE_TEXT,
|
||||
input_tokens=20000,
|
||||
output_tokens=500,
|
||||
finish_reasons=[],
|
||||
)
|
||||
with patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, _, _ = await _call_create_summary(SummaryType.LONG)
|
||||
|
||||
assert is_fallback is False
|
||||
assert "Human. Forever." in summary["content"]
|
||||
|
||||
async def test_clean_prose_cap_hit_keeps_content(self):
|
||||
"""Dense valid summary that hits the cap is still valid (#899)."""
|
||||
mock_response = HonchoLLMCallResponse(
|
||||
content=_CLEAN_CAP_HIT_TEXT,
|
||||
input_tokens=20000,
|
||||
output_tokens=settings.SUMMARY.MAX_TOKENS_LONG,
|
||||
finish_reasons=["max_tokens"],
|
||||
)
|
||||
with patch(
|
||||
"src.utils.summarizer.create_long_summary",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
):
|
||||
summary, is_fallback, in_tok, out_tok = await _call_create_summary(
|
||||
SummaryType.LONG
|
||||
)
|
||||
|
||||
assert is_fallback is False
|
||||
assert "project planning" in summary["content"]
|
||||
assert in_tok == 20000
|
||||
assert out_tok == settings.SUMMARY.MAX_TOKENS_LONG
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestSummaryCallerMigration:
|
||||
|
|
|
|||
Loading…
Reference in New Issue