2 commits

Author	SHA1	Message	Date
nangsontay	08fce29b47	fix(proxy): stop toggling headroom_retrieve in the Anthropic tools array (#2672) ... ## Description `should_inject_ccr_tool` deferred CCR tool injection whenever `frozen_message_count > 0`. Because `tools` is the head of Anthropic's cache key, that dropped a tool which was already inside the provider-cached prefix and invalidated the whole prefix — in both directions (`0 → >0` removes it; `>0 → 0` on proxy restart, `/model` switch, lineage eviction or TTL lapse adds it back). On three days of local proxy logs the turns that flipped injection state carried **44.7% of all cache-write tokens at a 52.0% hit rate**, against 98.1% for non-flipping turns. The log signature is `cache_read` alternating between two values exactly 172 tokens apart — the 464-byte tool definition. This deletes the gate and calls `apply_session_sticky_ccr_tool` directly, which is **what `openai.py` already does** — the two handlers now have the same shape. Net −61 production lines, no new state, no new config flag. Fixes defect 1 of #2671. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] Performance improvement ## Changes Made - `headroom/proxy/ccr_marker_policy.py` — deleted the `should_inject_ccr_tool` gate; `apply_session_sticky_ccr_tool` is now the single decision point. - `headroom/proxy/handlers/anthropic.py` — calls `apply_session_sticky_ccr_tool` directly, matching `openai.py`. - `headroom/proxy/helpers.py` — dropped the now-unused gate plumbing. - `tests/test_proxy_anthropic_cache_stability.py` — new test asserting the forwarded `tools` array is byte-identical across a `frozen 0 → >0` transition. - `tests/test_ccr_marker_policy.py` — removed the three unit tests that pinned the deleted decision (they encoded the defect). - `tests/test_proxy/test_ccr_frozen_prefix_coupling.py` — same unredeemable-marker intent, re-pinned at the sticky helper. - `tests/test_proxy/test_anthropic_ccr_deferred_injection.py` — autouse reset fixture for the process-global `SessionCcrTracker` (separate commit). - Formatting-only follow-up commit applying `ruff format` (pinned 0.15.17) to the two test files above. ### Why deleting the gate is safe `apply_session_sticky_ccr_tool` already holds the correct rule. Its four branches, in order: | # | condition | action | |---|---|---| | 1 | tool already in the incoming tool list (client/MCP pre-registered) | skip; the client's bytes win | | 2 | `session_id is None` (WS / pre-session) | per-turn flag drives it verbatim | | 3 | session has done CCR | always inject the recorded golden bytes | | 4 | fresh session, no compression this turn | **skip** | Branch 4 is the safety property: a session that has never compressed still gets no tool, so removing the gate cannot start injecting into non-CCR conversations. Branch 3 is what the gate was starving. `has_new_ccr_markers` still gates first-time injection, so markers replayed from the previously-forwarded prefix cannot trigger one. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed The three deleted unit tests encoded the defect. Coverage moves to the property that actually matters and was previously untested: **the forwarded `tools` array must be byte-identical across a `frozen 0 → >0` transition.** That test asserts on the forwarded request body rather than on a policy function's return value; unit-testing the old policy in isolation is exactly what let a wrong-but-self-consistent decision pass. Verified failing on `upstream/main` with an assertion on the missing tool (not an `ImportError`, so it fails for the right reason). Full suite: same pre-existing unrelated failures as `upstream/main`, **zero new** (verified by running the whole suite on both revisions and diffing the failure sets). ### Test Output ```text $ uv run pytest tests/test_ccr_marker_policy.py \ tests/test_proxy/test_anthropic_ccr_deferred_injection.py \ tests/test_proxy/test_ccr_frozen_prefix_coupling.py \ tests/test_proxy_anthropic_cache_stability.py -q collected 48 items tests/test_ccr_marker_policy.py ..... [ 10%] tests/test_proxy/test_anthropic_ccr_deferred_injection.py .............. [ 39%] . [ 41%] tests/test_proxy/test_ccr_frozen_prefix_coupling.py .. [ 45%] tests/test_proxy_anthropic_cache_stability.py .......................... [100%] ======================= 48 passed, 2 warnings in 13.59s ======================== $ ruff check . All checks passed! $ ruff format --check . 1349 files already formatted ``` ## Real Behavior Proof - Environment: local macOS proxy serving live Claude Code traffic to the Anthropic API; baseline = 3 days of proxy logs on `upstream/main`, after = 5.5 hours with this change live. - Exact command / steps: ran the proxy with this branch built in, drove normal Claude Code sessions through it (including `/model` switches and proxy restarts, the two events that used to flip injection state), then parsed 235 real turns from the proxy logs with the same parser used for the baseline in #2671. - Observed result: flip turns fell from 177 (44.7% of all cache write) to 2 (1.7%); steady-state write share 1.192% → 0.867%; aggregate hit rate 86.75% → 89.21%; main conversation warm hit rate 98.1% → 97.70% (n=149). The 2 remaining "flips" have `cache_read == 0` — cold starts that the bucketing counts as a state change, not real flips. | metric | baseline | after | |---|---|---| | flip turns | 177, carrying 44.7% of all cache write | **2**, carrying **1.7%** | | main conv, warm | 98.1% | **97.70%** (n=149) | | steady-state write share | 1.192% | **0.867%** | | aggregate | 86.75% | **89.21%** | - Not tested: `mypy headroom` was not run locally for this body; the OpenAI handler path (unchanged by this PR); tracker state loss mid-session (see note below); and defect 2 of #2671 (the sub-call breakpoint), which is untouched and is now 54.9% of remaining cache write — that is why aggregate stays just under 90%. ## 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 - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` ## Additional Notes **Pre-existing and unchanged here:** if the tracker loses state mid-session while the transcript still carries markers, branch 4 returns no tool and those markers are unredeemable. `upstream/main` has no recovery for that; this PR neither creates nor fixes it. See my comment on #2500, which adds a recovery path for the related dangling-reference case. **N/A checklist items:** no documentation changes — this removes an internal policy function with no user-facing surface. `mypy headroom` left unchecked because it was not run for this body; CI covers it. **Merge-order conflict with #2500 (please read before landing either):** this PR *deletes* `should_inject_ccr_tool`, which is the exact function #2500 extends with `transcript_requires_tool`. Whichever lands second needs a semantic rebase, not just a textual one — git will not flag it. If this PR lands first, #2500's recovery path should re-target `apply_session_sticky_ccr_tool` (the sticky helper now owns the decision alone) or the handler call site in `handlers/anthropic.py`. If #2500 lands first, the gate deletion here still applies but the `transcript_requires_tool` override needs to move with it. Happy to do the rebase either way — say which order you prefer.	2026-08-03 16:18:11 -07:00
Lucas Santos	43494ff526	fix(proxy): stop re-compressing headroom_retrieve output and emitting unredeemable markers (#1323) ... ## Description Two related CCR problems that both end in unreadable content. The first one (#1077) is an infinite loop. Any tool output over ~500 bytes gets replaced with a `<<ccr:hash>>` marker, and you call `headroom_retrieve` to get the original back. But the proxy then compresses the *retrieve response too*, so what comes back is a brand new marker. Retrieve that one and you get another marker. The second one (#1006), the proxy makes two independent decisions per request: SmartCrusher compresses, and the `headroom_retrieve` tool gets injected. The injection is deferred when there's a frozen message prefix (`frozen_message_count > 0`), but compression keeps running anyway. So the agent receives `[... compressed to N. Retrieve more: hash=...]` markers with no `headroom_retrieve` tool to redeem them. For #1077, SmartCrusher now skips `headroom_retrieve` results. Before crushing a tool message (OpenAI `role=tool`) or tool-result block (Anthropic `type=tool_result`), it checks whether that tool id maps to the CCR tool, and if so leaves it alone. Retrieved content stays readable. For #1006, compression and injection are no longer decided in isolation. The injection decision is extracted into `should_inject_ccr_tool`, which the Anthropic handler calls: when injection was deferred because of a frozen prefix but compression just emitted new markers, it injects the tool anyway, so a marker is never handed to an agent that can't act on it. The existing session-sticky dedup means sessions that already have the tool don't get it re-injected and don't lose their cache. Closes #1077 Closes #1006 ## 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/transforms/smart_crusher.py`: exempt `headroom_retrieve` results from compression on both the OpenAI `role=tool` and Anthropic `type=tool_result` paths. - `headroom/proxy/helpers.py`: add `should_inject_ccr_tool`, the deferral-plus-override decision the handler used to inline, so the #1006 behaviour is testable at the decision point. - `headroom/proxy/handlers/anthropic.py`: call `should_inject_ccr_tool` to couple injection with compression; rename the misleading `frozen_prefix=` log key to `frozen_message_count=`. - `tests/test_transforms/test_smart_crusher_ccr_retrieve_exemption.py` and `tests/test_proxy/test_ccr_frozen_prefix_coupling.py`: new tests; the frozen-prefix test now drives `should_inject_ccr_tool` so it would fail if the override were removed. ## 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 $ uv run --extra dev python -m pytest tests/test_proxy/test_ccr_frozen_prefix_coupling.py tests/test_transforms/test_smart_crusher_ccr_retrieve_exemption.py -q 5 passed, 1 skipped ruff: All checks passed! mypy: Success: no issues found ``` The SmartCrusher test skips locally because the Rust extension `.so` is built for a different OS, the same skip the existing SmartCrusher tests take locally. It runs in CI where the extension is built. ## Real Behavior Proof - Environment: macOS, Python 3.13, this branch. - Exact command / steps: `uv run --extra dev python -m pytest tests/test_proxy/test_ccr_frozen_prefix_coupling.py tests/test_transforms/test_smart_crusher_ccr_retrieve_exemption.py -q`. The frozen-prefix test calls `should_inject_ccr_tool` (the function the Anthropic handler now uses) with a frozen prefix and freshly emitted markers, then drives `apply_session_sticky_ccr_tool` end to end and asserts `headroom_retrieve` lands in the outbound tools. The exemption test runs a `headroom_retrieve` tool result through SmartCrusher on both the OpenAI and Anthropic shapes. - Observed result: 5 passed, 1 skipped. The retrieve tool is injected even under a frozen prefix once markers exist, and is not injected when no markers were emitted. Removing the handler override flips `should_inject_ccr_tool` and fails the test. - Not tested: a full live proxy session. The behaviours are covered at the decision, transform, and handler-call level by the new tests. ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes This one touches compression gating, so it's worth a careful read on the injection coupling, that's the part where a wrong call would re-introduce data loss. 1. Tool results with no id mapping still compress, marked with `# ponytail:` comments. Only ids we can positively identify as the CCR tool are exempted. 2. The injection coupling keys off `injector.has_compressed_content`, so the tool only shows up when there's actually something to retrieve. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>	2026-06-25 10:11:42 -05:00