mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
41dab2d099
## Description
`CCRToolInjector.scan_for_markers()` decides whether a compression
marker is Headroom's own by *shape* alone — any bracket marker carrying
a 24-hex hash counts, per the generic fallback pattern
(`\[.*?compressed.*?hash=([a-f0-9]{24})\]`). Other context tools emit
exactly that shape. Once a foreign hash is scanned,
`has_compressed_content` flips true and the retrieve tool + "Available
hashes" system instruction get injected for a hash this proxy never
stored — the model calls `headroom_retrieve`, gets a guaranteed miss,
and re-does work it already had. Two wasted turns per adopted foreign
hash.
Closes #2836
## Type of Change
- [x] 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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/ccr/tool_injection.py`: added
`CCRToolInjector.verify_ownership()` — filters `detected_hashes` down to
hashes the compression store actually recognizes, via the same
`store.exists()` check the retrieve endpoint itself performs. Added a
small `_HashOwnershipStore` Protocol (structural typing, not a hard
dependency on the concrete `CompressionStore` class) and a
`compression_store` constructor field for dependency injection/testing.
`scan_for_markers()` itself is untouched — kept store-independent (pure
regex) rather than baking the check into the scan loop, since that
approach broke 24 existing tests that correctly test "does this shape
match" in isolation.
- `headroom/proxy/handlers/anthropic.py`,
`headroom/proxy/handlers/openai.py`: call `injector.verify_ownership()`
right after `scan_for_markers()` — the two real per-request call sites.
- `verify_ownership()` is also called inside `process_request()` (the
convenience wrapper `batch.py`'s Google path uses), so that path is
covered without a separate call site edit.
- `tests/test_ccr_tool_injection.py`: new `TestVerifyOwnership` class (6
tests) — the exact issue repro, a real-hash-survives case, mixed
own/foreign hashes, explicit store override, store-exception safety
(must not raise), and no-op-on-empty-hashes.
- `tests/test_proxy_anthropic_cache_stability.py`: 3 pre-existing tests
needed updating for the new (correct) behavior — two `_FakeInjector`
test doubles needed a `verify_ownership()` stub added, and one real
end-to-end test needed a genuine store entry seeded (via
`explicit_hash`) for the hash its hand-typed marker references, instead
of asserting on an unverified shape-only match.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`) — not run locally, will
confirm via CI
- [x] New tests added for new functionality
- [x] Manual testing performed (see Real Behavior Proof)
### Test Output
```text
$ .venv/Scripts/python -m pytest tests/test_ccr_marker_policy.py tests/test_ccr_tool_always_on.py tests/test_ccr_tool_injection.py tests/test_proxy/test_ccr_frozen_prefix_coupling.py tests/test_proxy_anthropic_cache_stability.py tests/test_proxy_handlers_batch.py tests/test_proxy_handler_helpers.py -q
151 passed in 15.87s
$ .venv/Scripts/python -m pytest tests/test_proxy_ccr.py tests/test_proxy_openai_responses_stream_ccr.py tests/test_anthropic_ccr_workspace_unbound.py tests/test_compression_store.py tests/test_no_ccr_lossy.py tests/test_proxy_handlers_batch.py -q
121 passed in 20.73s
$ .venv/Scripts/ruff check . && .venv/Scripts/ruff format --check . # touched files only
All checks passed / already formatted
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.14.5, local venv
- Exact command / steps: ran the issue's exact 3-line repro
(`CCRToolInjector.scan_for_markers()` on the foreign marker text, then
`verify_ownership()`) before and after the fix; separately verified a
genuinely-Headroom-stored hash (via `store.store(...,
explicit_hash=...)`) still survives verification and still drives
injection
- Observed result: before the fix (scan only, no verify step exists yet)
`has_compressed_content` is `True` for the foreign marker — matches the
bug report exactly. After adding `verify_ownership()`: foreign marker →
`detected_hashes == []`, `has_compressed_content is False`; real stored
hash → `detected_hashes == [real_hash]`, `has_compressed_content is
True`.
- Not tested: have not driven this through a live two-context-tool proxy
session (e.g. Headroom alongside another CCR-shaped tool in the same
conversation) — verified at the unit/integration level (the exact repro
plus the real proxy handler call sites via
`test_proxy_anthropic_cache_stability.py`'s end-to-end `TestClient`
tests), not via a live multi-tool session.
## 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 (N/A —
internal CCR safety behavior, no user-facing docs reference the
marker-adoption mechanism)
- [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 (release-please
generates this automatically from commit messages)
## Additional Notes
Design note on why `verify_ownership()` is a separate step rather than
baked into `scan_for_markers()`: my first attempt did exactly that and
broke 24 tests across `test_ccr_tool_injection.py`,
`test_ccr_marker_policy.py`, `test_proxy_anthropic_cache_stability.py`,
and `test_proxy_handler_helpers.py` — all of them legitimately testing
"does the regex detect this marker shape" independent of any store
state. Keeping the scan pure and adding an explicit, separately-testable
verification step kept that test surface intact while still closing the
real gap at the three places that actually decide whether to advertise
the retrieve tool.
|
||
|---|---|---|
| .. | ||
| test_anthropic_buffered_timeout.py | ||
| test_anthropic_ccr_deferred_injection.py | ||
| test_anthropic_ccr_raise.py | ||
| test_anthropic_recount_and_reparse_safety.py | ||
| test_anthropic_streaming_ccr_retrieve.py | ||
| test_anthropic_upstream_header.py | ||
| test_background_compression.py | ||
| test_bedrock_passthrough.py | ||
| test_bedrock_sse_ping.py | ||
| test_cc_switch_reconciler.py | ||
| test_ccr_frozen_prefix_coupling.py | ||
| test_compression_failure_action.py | ||
| test_compression_timeout_config.py | ||
| test_compute_turn_id.py | ||
| test_gemini_savings_profile.py | ||
| test_header_safe_transforms.py | ||
| test_mcp_stats_aggregation.py | ||
| test_model_router.py | ||
| test_model_router_wiring.py | ||
| test_openai_backend_path.py | ||
| test_openai_chat_ccr_injection.py | ||
| test_openai_chat_savings_profile.py | ||
| test_openai_responses_ccr.py | ||
| test_openai_stream_usage_option.py | ||
| test_openai_transport_path_prefix.py | ||
| test_openai_upstream_header.py | ||
| test_phase3_byte_identity.py | ||
| test_request_logger.py | ||
| test_settings_fresh_process_precedence.py | ||
| test_settings_store.py | ||
| test_tool_search_repair_after_turn_hooks.py | ||
| test_transformations_feed.py | ||