2 commits

Author	SHA1	Message	Date
Abhay Singh	2cae0f8eaf	fix(proxy/cache): strip cache_control from messages in the semantic cache key (#3086) ... ## Description The proxy semantic response-cache key (`compute_semantic_cache_key`) strips `cache_control` from the response-shaping fields (`system`, `tools`, ...) so that a moved prompt-cache breakpoint does not fragment the key: ```python { "model": model, "messages": messages, # hashed verbatim **{k: strip_cache_control(v) for k, v in key_fields.items()}, # stripped } ``` But `messages` was hashed **verbatim**. Messages are the primary key component, and on the Anthropic path they are the most common place a client (e.g. Claude Code) places and *moves* a `cache_control` breakpoint between turns (on the last user turn / a `tool_result` block). So two otherwise-identical requests that differed only in a message-level breakpoint produced different keys and missed the semantic cache — the exact fragmentation the `strip_cache_control` helper exists to prevent, applied to everything except the field that matters most. The existing tests pin the strip for `system` (`test_cache_control_breakpoint_move_same_key`) and `tools` (`test_tools_cache_control_ignored`), but never covered a message-level breakpoint, so the gap went unnoticed. ## Fix Apply `strip_cache_control` to `messages` as well. `cache_control` is a prompt-caching directive for the upstream provider that never changes the generated completion, so removing the annotation before hashing is sound: message *content* still differentiates the key, and two requests that differ only in a `cache_control` breakpoint now share the cache entry (whose stored response body is identical either way). The proxy carries two in-sync copies of this pure policy (`semantic_cache_key_policy.py`, imported by the runtime `SemanticCache`, and `semantic_cache_key.py`, imported by the policy test); both are updated identically so they do not diverge. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/semantic_cache_key_policy.py` and `headroom/proxy/semantic_cache_key.py`: hash `strip_cache_control(messages)` instead of `messages`, with a docstring explaining why message-level breakpoints must not fragment the key. - `tests/test_proxy_semantic_cache_key.py`: added `test_message_cache_control_breakpoint_move_same_key` (behavioral, through `SemanticCache._compute_key`) and `test_message_content_change_still_distinct_key` (guards that stripping does not collapse genuinely different messages). - `tests/test_proxy_semantic_cache_key_policy.py`: added `test_semantic_cache_key_ignores_moved_message_cache_control` at the pure-policy level. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy`) - [x] New tests added ### Test Output ```text tests/test_proxy_semantic_cache_key.py + tests/test_proxy_semantic_cache_key_policy.py 33 passed # uvx ruff@0.15.22 check -> All checks passed! # uvx mypy@1.20.2 (both policy modules) -> Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.15.22 and mypy 1.20.2 via uvx. - Exact command / steps: reverted the two policy modules and ran the new tests to capture the bug (`python -m pytest tests/test_proxy_semantic_cache_key.py::test_message_cache_control_breakpoint_move_same_key tests/test_proxy_semantic_cache_key_policy.py::test_semantic_cache_key_ignores_moved_message_cache_control` -> both failed with two distinct SHA-256 keys for messages that differ only in a `cache_control` breakpoint); restored the fix; re-ran both key suites (`python -m pytest tests/test_proxy_semantic_cache_key.py tests/test_proxy_semantic_cache_key_policy.py` -> 33 passed); ran the wider `tests/test_cache/` suite and confirmed the only failures (`test_client_integration.py`) reproduce identically on clean `main` and are unrelated to this change; then `uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx mypy@1.20.2` on both modules. - Observed result: before the fix, a request whose last message carries `cache_control: {type: ephemeral}` hashes to a different key than the same request without it; after the fix they hash identically (a cache hit), while messages with different text still hash differently. - Not tested: a live multi-turn proxy session measuring the hit-rate improvement (the key contract is verified directly through `SemanticCache._compute_key` and the pure policy, which is what the runtime calls). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This is the pure semantic-cache key policy behind `SemanticCache`, not a rollout-channel-gated runtime feature. - Minimum rollout channel: N/A (no rollout-managed behavior). - Stable/default behavior changed: yes, as a bug fix. Requests that differ only in a message-level `cache_control` breakpoint now share a semantic-cache key (a hit) instead of missing. No request that differs in message content, model, or any shaping field changes key. Because the cache key changes shape, any entries stored under the old (un-stripped) keys are simply not reused and age out under the existing TTL/LRU — a one-time cold start for the affected entries, never a wrong response. - Kill switch / disable path: the semantic cache itself is already gated by the existing cache-enable configuration; disabling it bypasses this path entirely. - Unsafe override required: no. - Qualification impact: higher semantic-cache hit rate on the Anthropic path where clients move `cache_control` breakpoints between turns; no change to which distinct requests are considered equal beyond ignoring the caching directive. - Rollback path: revert this PR; the key returns to hashing messages verbatim. ## 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md`: it is generated by release-please from my Conventional Commit PR title ## Additional Notes Same class as the `system`/`tools` breakpoint handling already in place (issue #327 kept the strip from fragmenting the key on a hit); this extends it to messages, the primary key component. The two in-sync policy copies are updated together to avoid divergence; consolidating them into one module is left out of scope for this bug fix.	2026-08-17 20:21:17 -07:00
JD Davis	740fb9bc16	refactor(cache): isolate semantic key policy (#1953) ... ## Description Extracts proxy semantic-cache key normalization and hashing into a pure policy module while preserving `SemanticCache._compute_key` for existing callers and tests. This separates deterministic cache-key construction from the async cache adapter and LRU storage concerns. 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.semantic_cache_key` for pure cache-control stripping and semantic cache key construction. - Updated `SemanticCache._compute_key` to delegate to the extracted policy while preserving the local `_strip_cache_control` compatibility alias. - Added direct tests for the extracted semantic-cache key policy. - Kept the LiteLLM callback compatibility shim required for repo-wide type checking on fresh branches. ## 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_proxy_semantic_cache_key_policy.py tests/test_proxy_semantic_cache_key.py tests/test_proxy_semantic_cache_key_integration.py tests/test_proxy_openai_cache_key_integration.py tests/test_litellm_callback.py tests/test_compress_api.py::TestLiteLLMCallback -q 46 passed in 11.83s python -m ruff check . All checks passed! python -m ruff format --check . 1095 files already formatted python -m mypy headroom --ignore-missing-imports Success: no issues found in 409 source files gitleaks protect --staged --no-banner --redact no leaks found ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13, local worktree based on `headroomlabs/main`. - Exact command / steps: ran focused semantic cache key tests, handler cache-key integration tests, LiteLLM callback tests, Ruff lint/format checks, mypy over `headroom`, and staged gitleaks scan. - Observed result: all local checks passed; staged secret scan found no leaks. - Not tested: full CI matrix and deployment flows; those are covered by GitHub Actions. ## 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. GitHub reported existing Dependabot alerts on the default branch during push; this PR does not change dependencies, and the staged secret scan is clean.	2026-07-10 23:49:27 -05:00