mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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 <chopratejas@gmail.com>
This commit is contained in:
parent
9c7b9d5a9c
commit
ec3c3cd234
3 changed files with 148 additions and 18 deletions
45
headroom/proxy/ccr_marker_policy.py
Normal file
45
headroom/proxy/ccr_marker_policy.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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(
|
||||
|
|
|
|||
89
tests/test_ccr_marker_policy.py
Normal file
89
tests/test_ccr_marker_policy.py
Normal file
|
|
@ -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)
|
||||
Loading…
Add table
Add a link
Reference in a new issue