fix(mcp): report correct savings_percent in headroom_compress (#1106)

## Description

`headroom_compress` reports `savings_percent` backwards. In
`_compress_content`:

```python
savings_pct = (
    round((1 - result.compression_ratio) * 100, 1) if result.compression_ratio < 1.0 else 0
)
```

`compression_ratio` is already the saved fraction (`CompressResult`:
"0.0 = no savings, 1.0 = 100% removed"), so `1 - compression_ratio`
gives the *retained* percentage instead. A no-op comes back as 100% and
a real 71% reduction as 28.8%. The `else 0` branch also zeroes out a
genuine 100% result.

`_Stats.record_compression` a few lines up already does it the right way
(`1 - output_tokens / input_tokens`), so this is just bringing the
return value in line with that.

Closes # (no existing issue — found while evaluating the tool; can file
one if you'd rather track it)

## 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/mcp_server.py`: derive `savings_percent` from
`output_tokens`/`input_tokens` like `record_compression` does, so
`savings_percent` and `tokens_saved` can't disagree.
- `tests/test_ccr_mcp_server.py`: regression test tying
`savings_percent` to the token counts, including the no-op-isn't-100%
case.

## Testing

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

### Test Output

Ran the relevant checks against the changed code (borrowed the prebuilt
`_core.abi3.so` from the released wheel so the checkout could import the
pipeline):

```text
$ ruff check headroom/ccr/mcp_server.py tests/test_ccr_mcp_server.py
All checks passed!

$ mypy --ignore-missing-imports headroom/ccr/mcp_server.py
Success: no issues found in 1 source file

$ pytest tests/test_ccr_mcp_server.py -q
....                                                                     [100%]
4 passed in 1.71s
```

I ran the ccr test module and lint/type checks on the changed files, not
the whole repo suite (that needs a full Rust build) — CI covers the
rest.

## Real Behavior Proof

- Environment: macOS, Python 3.14, checkout + prebuilt `_core` from
headroom 0.26.0
- Exact command / steps: ran `compress()` on three inputs (a 40-record
JSON array, an incompressible string, repeated prose) and compared the
old `round((1 - compression_ratio) * 100, 1)` against the token-derived
value `(1 - comp/orig) * 100`.
- Observed result: the old expression returns the retained %, so 0%
saved is reported as 100% and a real 71.2% reduction as 28.8%; the new
value matches actual savings in every case:

```text
input        orig   comp   actual    old formula    new formula
array(40)     497    143   71.2%       28.8%          71.2%
noop           14     14    0.0%      100.0%           0.0%
prose         111    111    0.0%      100.0%           0.0%
```

- Not tested: the full repo test suite and the E2E workflows (need a
complete Rust build / maintainer-approved CI); only the ccr test module
and lint/type checks on the changed files were run locally.

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

## Additional Notes

Docs/CHANGELOG left unchecked as N/A — no user-facing doc covers this
field, though I'm happy to add a CHANGELOG line if you want one.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
This commit is contained in:
Kessy Similien 2026-06-22 17:05:47 -10:00 committed by GitHub
parent 8cc5354f51
commit f216e43055
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 33 additions and 3 deletions

View file

@ -391,9 +391,15 @@ class HeadroomMCPServer:
)
self._stats.record_compression(input_tokens, output_tokens, strategy)
savings_pct = (
round((1 - result.compression_ratio) * 100, 1) if result.compression_ratio < 1.0 else 0
)
# Percentage of tokens removed. Derive from the same token counts used
# for ``tokens_saved`` so all three fields agree — this mirrors the
# convention in ``_Stats.record_compression`` above. The previous
# ``(1 - result.compression_ratio)`` inverted the value: since
# ``compression_ratio`` is already the *saved* fraction (see
# ``CompressResult`` in headroom/compress.py — "0.0 = no savings, 1.0 =
# 100% removed"), the old expression reported the *retained* percentage,
# e.g. a no-op (0% saved) was reported as 100%.
savings_pct = round((1 - output_tokens / input_tokens) * 100, 1) if input_tokens > 0 else 0
return {
"compressed": compressed_content,

View file

@ -67,6 +67,30 @@ def test_mcp_retrieves_proxy_stored_content(fresh_store) -> None:
assert result["original_content"] == original
def test_compress_savings_percent_tracks_token_counts(fresh_store) -> None:
"""``savings_percent`` must be the *removed* percentage derived from the
token counts never the retained percentage. Regression for the inversion
where ``(1 - compression_ratio)`` reported a no-op (0% saved) as 100%."""
pytest.importorskip("mcp", reason="MCP SDK required")
server = mcp_server.HeadroomMCPServer(check_proxy=False)
# Repetitive JSON array — the shape the engine actually compresses.
content = json.dumps([{"id": i, "status": "ok", "kind": "run"} for i in range(40)])
result = server._compress_content(content)
orig = result["original_tokens"]
comp = result["compressed_tokens"]
expected = round((1 - comp / orig) * 100, 1) if orig > 0 else 0
# Reported savings agrees with the token fields (and with tokens_saved).
assert result["savings_percent"] == expected
assert 0.0 <= result["savings_percent"] <= 100.0
if result["tokens_saved"] == 0:
assert result["savings_percent"] == 0.0 # not inverted to 100
else:
assert result["savings_percent"] > 0.0
def test_mcp_retrieve_with_nonmatching_query_returns_full_content(fresh_store) -> None:
"""A query that matches no item above the relevance floor must still return
the stored entry (it exists and is unexpired) rather than the "Content not