akadmin/headroom
1
0
Fork 0
mirror of https://github.com/headroomlabs-ai/headroom.git synced 2026-08-27 14:17:10 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions
2630 commits 410 branches 169 tags 142 MiB
Commit graph

3 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
TenderDeve
18e1c3c9ba
fix(compression): report source-line span in CCR compression marker (#2597) 
## Description

The compression marker read `[N items compressed to M. Retrieve more:
hash=...]`, where `items` counts whitespace-split **words**, not lines.
So five lines of tool output could show as `[122 items compressed to
27...]`. A reader can't map "items" to lines and can't tell "this line
was compressed away" from "this line was never in the output" — absence
reads as evidence of absence, which per the report led to a materially
wrong conclusion.

Closes #2586

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- Annotate the marker with the source line count — `[N items compressed
to M (from L source lines). Retrieve more: hash=...]` — at both marker
sites: `KompressCompressor.compress` / `compress_batch`
(`kompress_compressor.py`) and the remote path (`kompress_remote.py`).
- The machine-parsed `Retrieve more: hash=` token is left byte-for-byte
unchanged, so CCR detection/retrieval is unaffected.

Scope note: I intentionally kept the existing `items compressed to`
phrasing rather than reword the unit, to avoid churning the marker
format that's referenced across ~12 test fixtures and the `config.py`
template. This is the minimal honesty fix; happy to go further (e.g.
line-unit counts or unifying with the `config.py` template) if you'd
prefer — see the issue thread where I asked about wording.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality

### Test Output

```text
$ uv run pytest tests/test_compression_units.py tests/test_compression_batches.py \
    tests/test_ccr_marker_policy.py tests/test_ccr_tool_injection.py tests/test_session_probes.py -q
92 passed

$ uv run pytest tests/test_ccr_marker_policy.py -q
8 passed   # incl. new test_source_line_span_marker_is_still_detected

$ uv run ruff check headroom/transforms/kompress_compressor.py headroom/transforms/kompress_remote.py tests/test_ccr_marker_policy.py
All checks passed!
```

## Real Behavior Proof

- Environment: headroom @ main, Python 3.14, uv
- Exact command / steps: added a marker in the new enriched format and
ran it through the CCR marker detector.
- Observed result: the retrieval hash is still detected from `[122 items
compressed to 27 (from 5 source lines). Retrieve more: hash=...]`;
existing compression/CCR suites unchanged.
- Not tested: mypy not run locally; the full model-backed compress()
marker path isn't unit-exercised (needs a real backend), so the new test
targets the parser boundary instead.

## 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`

## Additional Notes

Wording is adjustable per the issue discussion. The `config.py` marker
template (a different code path with `Omitted`/`Expires` fields) is left
untouched to keep this focused on the Kompress marker the report hit.
 2026-07-27 06:44:04 -07:00
JD Davis
ec3c3cd234
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>
 2026-07-12 12:16:48 -04:00