diff --git a/headroom/transforms/content_router.py b/headroom/transforms/content_router.py index 660271a1e..92041aa0c 100644 --- a/headroom/transforms/content_router.py +++ b/headroom/transforms/content_router.py @@ -2121,7 +2121,7 @@ class ContentRouter(Transform): else: mixed = is_mixed_content(content) detection = _detect_content(content) - strategy = self._determine_strategy(content) + strategy = self._determine_strategy(content, mixed=mixed, detection=detection) if debug_enabled: _log_router_debug( "content_router_input", @@ -2229,17 +2229,37 @@ class ContentRouter(Transform): except Exception as e: # pragma: no cover - defensive logger.debug("CompressionObserver raised (non-fatal): %s", e) - def _determine_strategy(self, content: str) -> CompressionStrategy: + def _determine_strategy( + self, + content: str, + mixed: bool | None = None, + detection: DetectionResult | None = None, + ) -> CompressionStrategy: """Determine the compression strategy from content analysis. Args: content: Content to analyze. + mixed: Precomputed ``is_mixed_content(content)`` when the caller + already has it. ``compress`` runs it on this exact content one + line before calling here; recomputed only when ``None``. + detection: Precomputed ``_detect_content(content)`` when the caller + already has it. The native Rust/Magika pass is the router's + hottest per-message cost — reuse it instead of a second + identical detection; recomputed only when ``None``. Returns: Selected compression strategy. """ + # Reuse the caller's analysis when supplied — ``compress`` already ran + # both on this exact content, and re-running is_mixed_content/ + # _detect_content on identical bytes is the router's hottest wasted cost. + if mixed is None: + mixed = is_mixed_content(content) + if detection is None: + detection = _detect_content(content) + # 1. Check for mixed content - if is_mixed_content(content): + if mixed: # 2. Verify with the native detector: ``is_mixed_content`` uses # cheap regex heuristics that produce false positives on source # code. Python files with dict/list literals (``{``, ``[`` at @@ -2250,13 +2270,11 @@ class ContentRouter(Transform): # wasting latency on splitting without any compression. # When the native magika detector confidently says SOURCE_CODE, # trust it over the regex heuristics. - detection = _detect_content(content) if detection.content_type == ContentType.SOURCE_CODE and detection.confidence >= 0.8: return self._strategy_from_detection(detection) return CompressionStrategy.MIXED - # 2. Detect content type from content itself - detection = _detect_content(content) + # 2. Not mixed — map the detected type straight to a strategy. return self._strategy_from_detection(detection) def _strategy_from_detection(self, detection: Any) -> CompressionStrategy: diff --git a/tests/test_content_router_detection_dedup.py b/tests/test_content_router_detection_dedup.py new file mode 100644 index 000000000..72ff51f05 --- /dev/null +++ b/tests/test_content_router_detection_dedup.py @@ -0,0 +1,96 @@ +"""Regression guards for the content-detection dedup on the router hot path. + +``ContentRouter.compress`` used to run the native ``_detect_content`` twice on +identical content: once for (default-off) debug logging, then again inside +``_determine_strategy``. The native Rust/Magika pass is the router's hottest +per-message cost, so ``compress`` now runs it once and threads the result into +``_determine_strategy``. These tests pin the call count at one and prove the +threaded result routes identically to the recomputed one. +""" + +from __future__ import annotations + +import pytest + +import headroom.transforms.content_router as content_router_module +from headroom.transforms.content_detector import ContentType, DetectionResult +from headroom.transforms.content_router import ( + CompressionStrategy, + ContentRouter, + ContentRouterConfig, + RouterCompressionResult, +) + + +@pytest.fixture(autouse=True) +def _reset_detect_module_state(monkeypatch: pytest.MonkeyPatch) -> None: + # The native-detector circuit breaker (#575) is process-wide; keep it from + # leaking across tests (mirrors tests/test_transforms_content_router.py). + monkeypatch.setattr(content_router_module, "_detect_native_unhealthy", False) + monkeypatch.setattr(content_router_module, "_detect_backend_warned", False) + monkeypatch.setattr(content_router_module, "_detect_panic_warned", False) + + +def _count_detects(monkeypatch: pytest.MonkeyPatch, result: DetectionResult) -> dict[str, int]: + """Replace the module-level ``_detect_content`` with a deterministic counter. + + Both call sites (``compress`` and ``_determine_strategy``) resolve the name + from the module namespace at call time, so a single patch counts every call. + """ + calls = {"n": 0} + + def _counting(content: str) -> DetectionResult: + calls["n"] += 1 + return result + + monkeypatch.setattr(content_router_module, "_detect_content", _counting) + return calls + + +def test_compress_runs_detection_once(monkeypatch: pytest.MonkeyPatch) -> None: + # Before the dedup this was 2 — compress ran _detect_content for debug + # logging, then _determine_strategy ran it again on identical bytes. The + # fix threads the single result through, so it must now be exactly 1. + router = ContentRouter(ContentRouterConfig(prefer_code_aware_for_code=False)) + calls = _count_detects(monkeypatch, DetectionResult(ContentType.PLAIN_TEXT, 1.0, {})) + monkeypatch.setattr(content_router_module, "is_mixed_content", lambda content: False) + # Stub the downstream compressor — this test isolates detection, not output. + monkeypatch.setattr( + router, + "_compress_pure", + lambda *a, **k: RouterCompressionResult( + compressed="x", original="x", strategy_used=CompressionStrategy.TEXT + ), + ) + + router.compress("a representative plain-text blob that is comfortably non-empty") + + assert calls["n"] == 1 + + +@pytest.mark.parametrize( + ("mixed", "detected"), + [ + (False, ContentType.SOURCE_CODE), + (False, ContentType.JSON_ARRAY), + (False, ContentType.BUILD_OUTPUT), + (False, ContentType.PLAIN_TEXT), + (True, ContentType.SOURCE_CODE), # mixed regex hit, native says code -> trust native + (True, ContentType.PLAIN_TEXT), # genuinely mixed + ], +) +def test_determine_strategy_threaded_matches_recomputed( + monkeypatch: pytest.MonkeyPatch, mixed: bool, detected: ContentType +) -> None: + # Threading precomputed (mixed, detection) must pick the SAME strategy as + # letting _determine_strategy recompute them: the dedup changes cost, not + # routing. + router = ContentRouter(ContentRouterConfig(prefer_code_aware_for_code=False)) + detection = DetectionResult(detected, 1.0, {}) + monkeypatch.setattr(content_router_module, "is_mixed_content", lambda content: mixed) + monkeypatch.setattr(content_router_module, "_detect_content", lambda content: detection) + + recomputed = router._determine_strategy("payload") + threaded = router._determine_strategy("payload", mixed=mixed, detection=detection) + + assert threaded is recomputed diff --git a/tests/test_transforms_content_router.py b/tests/test_transforms_content_router.py index 92dbea334..57f0cf2a1 100644 --- a/tests/test_transforms_content_router.py +++ b/tests/test_transforms_content_router.py @@ -323,10 +323,14 @@ def test_content_router_strategy_and_compress_paths(monkeypatch: pytest.MonkeyPa monkeypatch.setattr(router, "_compress_mixed", lambda *args, **kwargs: mixed_result) monkeypatch.setattr(router, "_compress_pure", lambda *args, **kwargs: pure_result) - monkeypatch.setattr(router, "_determine_strategy", lambda content: CompressionStrategy.MIXED) + monkeypatch.setattr( + router, "_determine_strategy", lambda content, **_kwargs: CompressionStrategy.MIXED + ) assert router.compress("mixed") is mixed_result - monkeypatch.setattr(router, "_determine_strategy", lambda content: CompressionStrategy.TEXT) + monkeypatch.setattr( + router, "_determine_strategy", lambda content, **_kwargs: CompressionStrategy.TEXT + ) assert router.compress("pure") is pure_result assert router.compress(" ").strategy_used is CompressionStrategy.PASSTHROUGH