mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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:
parent
8cc5354f51
commit
f216e43055
2 changed files with 33 additions and 3 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue