diff --git a/src/telemetry/prometheus/metrics.py b/src/telemetry/prometheus/metrics.py index 01d7be4f..9e779758 100644 --- a/src/telemetry/prometheus/metrics.py +++ b/src/telemetry/prometheus/metrics.py @@ -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, *, diff --git a/src/utils/summarizer.py b/src/utils/summarizer.py index 2abdb0f9..d1bf34c2 100644 --- a/src/utils/summarizer.py +++ b/src/utils/summarizer.py @@ -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 diff --git a/tests/integration/test_token_metrics.py b/tests/integration/test_token_metrics.py index 4e6c1652..bdb7f17d 100644 --- a/tests/integration/test_token_metrics.py +++ b/tests/integration/test_token_metrics.py @@ -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 # ============================================================================= diff --git a/tests/utils/test_summarizer.py b/tests/utils/test_summarizer.py index b842ced7..c040e6f9 100644 --- a/tests/utils/test_summarizer.py +++ b/tests/utils/test_summarizer.py @@ -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: