mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -04:00
5fb449e90b
## Description
`HeadroomContribution.efficiency_pct` can report values above 100%
because its numerator and denominator disagree about cache-read tokens.
```python
def total_saved(self) -> int:
return (self.tokens_saved_compression + self.cli_filtering_saved()
+ self.tokens_saved_cache_reads) # includes cache reads
def raw_without_headroom(self) -> int:
return (self.tokens_submitted + self.tokens_saved_compression
+ self.cli_filtering_saved()) # excludes cache reads
def efficiency_pct(self) -> float:
raw = self.raw_without_headroom()
if raw == 0:
return 0.0
return round(self.total_saved() / raw * 100, 1)
```
`tokens_saved_cache_reads` are input tokens that were *forwarded* to the
provider and served from the prefix cache at a discount, so they already
live inside `tokens_submitted` (the "raw input tokens actually
forwarded"). They are added to the numerator via `total_saved()` but
never to the denominator, so with `tokens_submitted=100` and
`tokens_saved_cache_reads=1000` the method returns `1000.0%`, which the
dashboard renders verbatim. An efficiency percentage should never exceed
100%.
## Fix
Use the existing sibling `compression_saved()` (compression + CLI
filtering, which already excludes cache reads) as the numerator. Then
`efficiency_pct = compression_saved / (tokens_submitted +
compression_saved)`, which is bounded by its own denominator and is the
meaningful quantity here: the fraction of the pre-Headroom input that
compression and CLI filtering actually removed. Cache reads are a
provider-side discount on forwarded tokens, not tokens Headroom removed,
so they don't belong in a removal-efficiency ratio. `total_saved()` is
left unchanged for its other callers (`to_dict`, etc.).
Closes #
## 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/subscription/models.py`: `efficiency_pct` now uses
`compression_saved()` instead of `total_saved()` as the numerator, with
a comment explaining the cache-read inconsistency.
- `tests/test_subscription_contribution.py`: new tests — cache reads
can't push efficiency over 100%, the ratio equals the
compression-removal fraction, and empty input yields 0.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [x] Unit tests pass (`uv run --extra dev pytest
tests/test_subscription_contribution.py -q`)
- [x] Linting passes (`uvx ruff@0.15.17 check
headroom/subscription/models.py tests/test_subscription_contribution.py
headroom/memory/factory.py`)
- [x] Type checking passes (`uvx mypy==1.20.2
headroom/memory/factory.py`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/subscription/models.py tests/test_subscription_contribution.py
All checks passed!
$ python -m py_compile headroom/subscription/models.py tests/test_subscription_contribution.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the ratio with a
dependency-free script that replicates the three methods, and left the
full pytest to CI.
- Exact command / steps: computed `efficiency_pct` under the old
numerator (`total_saved`) and the new numerator (`compression_saved`)
for `tokens_submitted=100, tokens_saved_cache_reads=1000` and for a real
compression case (`submitted=1000, compression=400, cache_reads=300`).
- Observed result: old returns `1000.0%` for the cache-read case
(impossible) and the new returns `0.0%`; for the compression case old
returns `50.0%` (inflated by cache reads) and new returns `28.6%` (= 400
/ 1400), always `<= 100%`. The new tests assert these.
- Not tested: the dashboard render path end to end; full local `pytest`
deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
Merged current `main` to pick up the repository-wide mypy cache-key
annotation fix and current dependency security floors, then verified the
focused regression locally. the change swaps one method call in a pure
dataclass method, verified by the standalone proof and the new tests for
CI.
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
37 lines
1.3 KiB
Python
37 lines
1.3 KiB
Python
"""HeadroomContribution ratio methods."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from headroom.subscription.models import HeadroomContribution
|
|
|
|
|
|
def test_efficiency_pct_never_exceeds_100_from_cache_reads():
|
|
"""Cache reads must not push efficiency above 100%.
|
|
|
|
Cache-read tokens are a provider-side discount on tokens that were still
|
|
forwarded, not tokens Headroom removed. Counting them in the numerator while
|
|
the denominator excludes them let efficiency report impossible values like
|
|
1000%.
|
|
"""
|
|
c = HeadroomContribution(tokens_submitted=100, tokens_saved_cache_reads=1000)
|
|
|
|
assert c.efficiency_pct() <= 100.0
|
|
# Nothing was compressed or filtered, so the removal efficiency is 0.
|
|
assert c.efficiency_pct() == 0.0
|
|
|
|
|
|
def test_efficiency_pct_is_compression_removal_ratio():
|
|
"""Efficiency is compression + CLI filtering over the pre-Headroom input."""
|
|
c = HeadroomContribution(
|
|
tokens_submitted=1000,
|
|
tokens_saved_compression=400,
|
|
tokens_saved_cache_reads=300, # must not affect the ratio
|
|
)
|
|
|
|
# 400 removed out of (1000 forwarded + 400 removed) = 28.6%.
|
|
assert c.efficiency_pct() == 28.6
|
|
assert c.efficiency_pct() <= 100.0
|
|
|
|
|
|
def test_efficiency_pct_zero_when_no_input():
|
|
assert HeadroomContribution().efficiency_pct() == 0.0
|