diff --git a/.github/pr-images/issue-1696-error-protection-fix.svg b/.github/pr-images/issue-1696-error-protection-fix.svg new file mode 100644 index 000000000..c3d9915fd --- /dev/null +++ b/.github/pr-images/issue-1696-error-protection-fix.svg @@ -0,0 +1,78 @@ + + + Headroom: Error-Output Protection False-Positive Fix (#1696) + + + + + 1. THE GATE + content_router.py routes every message/content-block through + content_has_strong_error_indicators() before deciding whether + to compress it or protect it (pass through untouched). + Rule: if the text contains 2+ DISTINCT keywords from + [error, fail, exception, traceback, fatal, panic, crash] + the block is protected — assumed to be a real failure trace. + + + + + + 2. THE BUG + Passing build/test output mentions BOTH words without failing: + tsc: "Found 0 errors" + jest: "0 failures, 42 passed" + → 2 distinct keyword hits ("error" + "fail") on a CLEAN run + → wrongly protected, forever, from compression. + + + + + + 3. OBSERVED IMPACT (issue #1696 stats.json) + In a long multi-turn JS/TS coding session (KiloCode via headroom proxy), "router:protected:error_output" fired on + nearly every request. Combined with tool-output exclusion and small-block skips, only a sliver of tokens were ever + eligible for compression. + + 0.3% + actual + + 60-95% + expected + + + + + + 4. THE FIX + Strip common zero-result phrases before the keyword scan: + "0 error(s)", "no error(s)", "0 failing", + "0 failure(s)", "no failure(s)" + headroom/transforms/error_detection.py + content_has_strong_error_indicators() + → clean tool output no longer trips protection. + + + + + + 5. REAL FAILURES STILL PROTECTED + Traceback (most recent call last): + ... + ValueError: fatal error during load + "traceback" + "fatal" (+ "error") — 2+ distinct hits + outside a stripped zero-result phrase + → still protected, correctly. + + + + + + 6. TEST COVERAGE ADDED (tests/test_error_detection.py — none existed before) + ✓ real error/traceback text is still flagged ✓ single keyword mention is not flagged + ✓ passing tsc summary ("Found 0 errors") is not flagged ✓ passing eslint summary is not flagged + ✓ "0 errors" text does not mask a real second indicator elsewhere in the same blob + All 5 new tests + full existing content_router suite (86 tests total) pass. + + + Fixes GitHub issue #1696 · headroomlabs-ai/headroom + diff --git a/headroom/transforms/error_detection.py b/headroom/transforms/error_detection.py index 8b6a6cac6..48aa1b4a3 100644 --- a/headroom/transforms/error_detection.py +++ b/headroom/transforms/error_detection.py @@ -145,6 +145,17 @@ def content_has_error_indicators(text: str) -> bool: return bool(_rust_content_has_error_indicators(text)) +# Success-summary phrases from common build/test/lint tools that legitimately +# pair two indicator keywords (`error` + `fail`) while reporting a PASS, e.g. +# tsc's "Found 0 errors", jest's "0 failing" / "0 failures", eslint's +# "0 problems (0 errors, 0 warnings)". Stripped before the keyword scan below +# so a clean JS/TS toolchain run doesn't get permanently protected from +# compression for the rest of a long coding session (issue #1696). +_ZERO_RESULT_PATTERN = re.compile( + r"\b(?:0|no)\s+(?:error|errors|failing|failure|failures)\b", re.IGNORECASE +) + + def content_has_strong_error_indicators(text: str) -> bool: """Stricter triage for compression-protection gates. @@ -162,7 +173,7 @@ def content_has_strong_error_indicators(text: str) -> bool: safe — downstream compressors (LogCompressor) still preserve error lines. """ - lowered = text.lower() + lowered = _ZERO_RESULT_PATTERN.sub(" ", text.lower()) hits = 0 for keyword in ERROR_INDICATOR_KEYWORDS: if keyword in lowered: diff --git a/tests/test_error_detection.py b/tests/test_error_detection.py new file mode 100644 index 000000000..eeb20651f --- /dev/null +++ b/tests/test_error_detection.py @@ -0,0 +1,38 @@ +"""Regression tests for the error/importance detection triage helpers.""" + +from __future__ import annotations + +from headroom.transforms.error_detection import content_has_strong_error_indicators + + +def test_real_error_output_is_detected() -> None: + text = "Traceback (most recent call last):\n ...\nValueError: fatal error during load" + assert content_has_strong_error_indicators(text) + + +def test_single_keyword_mention_is_not_flagged() -> None: + # Only one distinct indicator keyword ("error") — should not trip the + # two-keyword threshold. + text = 'Wrote error_handler.py with an "errors": [] field.' + assert not content_has_strong_error_indicators(text) + + +def test_tsc_passing_summary_is_not_flagged() -> None: + # Regression for issue #1696: a clean `tsc` run mentions both "error" + # and (via "0 failures" in a paired test run) "fail" while reporting + # success. Previously this tripped the two-keyword heuristic and got + # the message permanently protected from compression. + text = "Found 0 errors. Watching for file changes.\nTests: 0 failures, 42 passed" + assert not content_has_strong_error_indicators(text) + + +def test_eslint_passing_summary_is_not_flagged() -> None: + text = "0 problems (0 errors, 0 warnings)\nno failing tests" + assert not content_has_strong_error_indicators(text) + + +def test_zero_result_phrase_does_not_mask_a_real_second_error() -> None: + # "0 errors" is stripped, but a genuine second distinct indicator + # elsewhere in the same blob must still trigger protection. + text = "0 errors from linter, but the build crashed with a fatal signal" + assert content_has_strong_error_indicators(text)