From ec3c3cd2345a9aed682eec719370c36bb79e2173 Mon Sep 17 00:00:00 2001 From: JD Davis Date: Sun, 12 Jul 2026 16:16:48 +0000 Subject: [PATCH] refactor(proxy): extract ccr marker policy (#2004) ## Description Extracts CCR marker freshness and retrieval-tool injection decision policy from `headroom.proxy.helpers` into a focused pure module. Existing helper functions remain as compatibility wrappers for current Anthropic/OpenAI handler imports. Closes # ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [x] Code refactoring (no functional changes) ## Changes Made - Added `headroom.proxy.ccr_marker_policy` for new-marker detection and frozen-prefix tool injection decisions. - Kept `helpers.has_new_ccr_markers()` and `helpers.should_inject_ccr_tool()` as compatibility wrappers. - Added direct policy tests for replayed markers, genuinely new markers, missing prior forwards, empty current hashes, and frozen-prefix override behavior. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text python -m pytest tests/test_ccr_marker_policy.py tests/test_proxy/test_ccr_frozen_prefix_coupling.py tests/test_proxy_handler_helpers.py::TestHasNewCcrMarkers 16 passed in 0.91s python -m ruff check . All checks passed! python -m ruff format --check . 1069 files already formatted python -m mypy headroom --ignore-missing-imports Success: no issues found in 410 source files gitleaks protect --staged --no-banner --redact no leaks found ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13 - Exact command / steps: Ran direct CCR marker policy tests, frozen-prefix coupling tests, existing helper marker freshness tests, full ruff, format check, mypy, and staged gitleaks scan. - Observed result: Existing frozen-prefix CCR behavior remains green while the marker freshness and injection decision policy is directly covered. - Not tested: Full repository pytest suite locally; CI covers the broader matrix. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes Documentation and changelog updates are not applicable for this internal refactor. The default-branch Dependabot alerts reported during push are pre-existing and unrelated to this PR. Co-authored-by: Tejas Chopra --- headroom/proxy/ccr_marker_policy.py | 45 +++++++++++++++ headroom/proxy/helpers.py | 32 +++++------ tests/test_ccr_marker_policy.py | 89 +++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 18 deletions(-) create mode 100644 headroom/proxy/ccr_marker_policy.py create mode 100644 tests/test_ccr_marker_policy.py diff --git a/headroom/proxy/ccr_marker_policy.py b/headroom/proxy/ccr_marker_policy.py new file mode 100644 index 000000000..ca1be0c8a --- /dev/null +++ b/headroom/proxy/ccr_marker_policy.py @@ -0,0 +1,45 @@ +"""CCR marker freshness and retrieval-tool injection policy.""" + +from __future__ import annotations + +from typing import Any, Literal + + +def has_new_ccr_markers( + *, + current_detected_hashes: list[str], + previous_forwarded_messages: list[dict[str, Any]] | None, + provider: Literal["anthropic", "openai", "google"], +) -> bool: + """Return whether current CCR hashes contain hashes not previously forwarded.""" + + current = set(current_detected_hashes) + if not current: + return False + if not previous_forwarded_messages: + return True + + from headroom.ccr.tool_injection import CCRToolInjector + + previous = CCRToolInjector( + provider=provider, + inject_tool=False, + inject_system_instructions=False, + ) + previous.scan_for_markers(previous_forwarded_messages) + return bool(current - set(previous.detected_hashes)) + + +def should_inject_ccr_tool( + *, + configured_inject_tool: bool, + frozen_message_count: int, + has_compressed_content: bool, +) -> tuple[bool, bool]: + """Decide whether the CCR retrieval tool must be injected this turn.""" + + inject_tool = configured_inject_tool + if inject_tool and frozen_message_count > 0: + inject_tool = False + is_marker_override = not inject_tool and has_compressed_content + return (inject_tool or is_marker_override), is_marker_override diff --git a/headroom/proxy/helpers.py b/headroom/proxy/helpers.py index f62fcb527..c21b24de8 100644 --- a/headroom/proxy/helpers.py +++ b/headroom/proxy/helpers.py @@ -42,6 +42,12 @@ from headroom.proxy.ccr_golden_policy import ( create_fresh_ccr_tool_definition, replay_golden_ccr_tool_definition, ) +from headroom.proxy.ccr_marker_policy import ( + has_new_ccr_markers as _has_new_ccr_markers, +) +from headroom.proxy.ccr_marker_policy import ( + should_inject_ccr_tool as _should_inject_ccr_tool, +) from headroom.proxy.ccr_session_tracker import SessionCcrTracker as _SessionCcrTracker from headroom.proxy.internal_header_policy import ( INTERNAL_HEADER_PREFIX, @@ -2218,21 +2224,11 @@ def has_new_ccr_markers( Returns True iff ``current_detected_hashes`` contains a hash that is not present in ``previous_forwarded_messages``. """ - current = set(current_detected_hashes) - if not current: - return False - if not previous_forwarded_messages: - # No prior forward → every marker is new (genuine first CCR turn). - return True - from headroom.ccr.tool_injection import CCRToolInjector - - prev = CCRToolInjector( + return _has_new_ccr_markers( + current_detected_hashes=current_detected_hashes, + previous_forwarded_messages=previous_forwarded_messages, provider=provider, - inject_tool=False, - inject_system_instructions=False, ) - prev.scan_for_markers(previous_forwarded_messages) - return bool(current - set(prev.detected_hashes)) def should_inject_ccr_tool( @@ -2257,11 +2253,11 @@ def should_inject_ccr_tool( True only when injection happens *because* of new markers despite a deferral, so the caller can log the override distinctly. """ - inject_tool = configured_inject_tool - if inject_tool and frozen_message_count > 0: - inject_tool = False # defer to preserve cache - is_marker_override = not inject_tool and has_compressed_content - return (inject_tool or is_marker_override), is_marker_override + return _should_inject_ccr_tool( + configured_inject_tool=configured_inject_tool, + frozen_message_count=frozen_message_count, + has_compressed_content=has_compressed_content, + ) def apply_session_sticky_ccr_tool( diff --git a/tests/test_ccr_marker_policy.py b/tests/test_ccr_marker_policy.py new file mode 100644 index 000000000..a9a0e033c --- /dev/null +++ b/tests/test_ccr_marker_policy.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +from headroom.ccr.tool_injection import CCRToolInjector +from headroom.proxy.ccr_marker_policy import has_new_ccr_markers, should_inject_ccr_tool + + +def _hashes(*contents: str) -> list[str]: + injector = CCRToolInjector( + provider="anthropic", + inject_tool=False, + inject_system_instructions=False, + ) + injector.scan_for_markers([{"role": "user", "content": content} for content in contents]) + return injector.detected_hashes + + +def test_has_new_ccr_markers_filters_replayed_forwarded_markers() -> None: + marker = "[100 items compressed to 10. Retrieve more: hash=abc123def456abc123def456]" + + assert ( + has_new_ccr_markers( + current_detected_hashes=_hashes(marker), + previous_forwarded_messages=[{"role": "user", "content": marker}], + provider="anthropic", + ) + is False + ) + + +def test_has_new_ccr_markers_detects_hash_not_seen_in_previous_forward() -> None: + old = "[100 items compressed to 10. Retrieve more: hash=abc123def456abc123def456]" + new = "[50 items compressed to 5. Retrieve more: hash=deadbeefdeadbeefdeadbeef]" + + assert ( + has_new_ccr_markers( + current_detected_hashes=_hashes(old, new), + previous_forwarded_messages=[{"role": "user", "content": old}], + provider="anthropic", + ) + is True + ) + + +def test_has_new_ccr_markers_treats_missing_previous_forward_as_new() -> None: + marker = "[100 items compressed to 10. Retrieve more: hash=abc123def456abc123def456]" + + assert ( + has_new_ccr_markers( + current_detected_hashes=_hashes(marker), + previous_forwarded_messages=None, + provider="anthropic", + ) + is True + ) + + +def test_has_new_ccr_markers_returns_false_without_current_hashes() -> None: + assert ( + has_new_ccr_markers( + current_detected_hashes=[], + previous_forwarded_messages=None, + provider="anthropic", + ) + is False + ) + + +def test_should_inject_ccr_tool_overrides_frozen_prefix_deferral_for_markers() -> None: + assert should_inject_ccr_tool( + configured_inject_tool=True, + frozen_message_count=3, + has_compressed_content=True, + ) == (True, True) + + +def test_should_inject_ccr_tool_defers_frozen_prefix_without_markers() -> None: + assert should_inject_ccr_tool( + configured_inject_tool=True, + frozen_message_count=3, + has_compressed_content=False, + ) == (False, False) + + +def test_should_inject_ccr_tool_injects_configured_tool_without_frozen_prefix() -> None: + assert should_inject_ccr_tool( + configured_inject_tool=True, + frozen_message_count=0, + has_compressed_content=False, + ) == (True, False)