mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description
`RemoteKompressCompressor` (the opt-in `HEADROOM_KOMPRESS_ENDPOINT`
remote compression client) documents a fail-open contract in its own
docstring:
> Fails OPEN: any network/HTTP error returns the content verbatim so a
flaky endpoint degrades compression rather than breaking the proxy.
But only the network call and the `compressed` field check actually run
inside the fail-open guard. The metadata coercions run **after** the
`except`, outside it:
```python
try:
resp = self._client.post(...)
resp.raise_for_status()
data = resp.json()
compressed = data["compressed"]
if not isinstance(compressed, str):
raise TypeError("...")
except Exception as e: # fail OPEN
logger.warning("Remote Kompress failed (%s); passing through", e)
return self._passthrough(content, n_words)
result = KompressResult(
compressed=compressed,
original=content,
original_tokens=int(data.get("original_tokens", n_words)),
compressed_tokens=int(data.get("compressed_tokens", len(compressed.split()))),
compression_ratio=float(data.get("compression_ratio", 1.0)), # <-- outside the guard
model_used=str(data.get("model_used", self.config.model_id)),
)
```
So a hosted `/compress` endpoint that returns a 200 with a valid
`compressed` string but a malformed metadata field escapes the guard and
raises out of `compress`, breaking the proxy request instead of passing
through. The most realistic trigger is an explicit JSON `null`:
`data.get("compression_ratio", 1.0)` returns `None` for a **present**
key (the default only applies to a missing key), and `float(None)`
raises `TypeError`. A non-numeric string like `"original_tokens":
"lots"` raises `ValueError` the same way. Since the whole point of the
flag is to support arbitrary self-hosted endpoints, a slightly-off but
well-meaning endpoint (sending `null` for a field it could not compute)
takes down the request path this class exists to protect.
## Fix
Move the response parsing (the `KompressResult` construction with its
`int`/`float`/`str` coercions) inside the fail-open `try`, so any
malformed field degrades to verbatim passthrough like every other
bad-response case:
```python
try:
...
compressed = data["compressed"]
if not isinstance(compressed, str):
raise TypeError("...")
result = KompressResult(
compressed=compressed,
original=content,
original_tokens=int(data.get("original_tokens", n_words)),
compressed_tokens=int(data.get("compressed_tokens", len(compressed.split()))),
compression_ratio=float(data.get("compression_ratio", 1.0)),
model_used=str(data.get("model_used", self.config.model_id)),
)
except Exception as e: # fail OPEN
logger.warning("Remote Kompress failed (%s); passing through", e)
return self._passthrough(content, n_words)
```
No behavior change on a well-formed response; only the malformed-200
path changes (raise to passthrough).
## 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/kompress_remote.py`: move the `KompressResult`
construction and its field coercions inside the fail-open `try`.
- `tests/test_transforms/test_kompress_remote.py`: add
`test_remote_kompress_null_numeric_field_fails_open` (explicit JSON
`null`) and `test_remote_kompress_non_numeric_field_fails_open`
(non-numeric string), both asserting verbatim passthrough.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] 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
$ uvx ruff@0.15.17 check headroom/transforms/kompress_remote.py tests/test_transforms/test_kompress_remote.py
All checks passed!
$ uvx ruff@0.15.17 format --check headroom/transforms/kompress_remote.py tests/test_transforms/test_kompress_remote.py
2 files already formatted
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/transforms/kompress_remote.py
Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx
mypy@1.20.2`. A full `pytest` OOMs this box (ML-stack import), so I
reproduced the control flow with a dependency-free script and left the
full pytest to CI.
- Exact command / steps: modeled the OLD (coercions outside the `try`)
and NEW (inside the `try`) parsing against a 200 body `{"compressed":
"short result", "compression_ratio": null}` and against a well-formed
body.
- Observed result: OLD raised `TypeError` on the null field (proxy
request breaks); NEW returned passthrough; a well-formed body still
compressed under NEW. The added tests assert both malformed cases
(`null` and non-numeric string) return the original content with
`compression_ratio == 1.0`.
- Not tested: a live remote Kompress endpoint; the added tests drive
`RemoteKompressCompressor` through an `httpx.MockTransport`, matching
the existing test harness in this file.
## 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
The "unit tests pass locally" box is unchecked because a local pytest
run imports the ML stack and OOMs this box; the added tests reuse the
existing `httpx.MockTransport` harness in `test_kompress_remote.py` and
run under the normal CI pytest job, and the behavior is corroborated by
the standalone proof above.
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
154 lines
5 KiB
Python
154 lines
5 KiB
Python
import httpx
|
|
|
|
from headroom.transforms.content_router import ContentRouter, ContentRouterConfig
|
|
from headroom.transforms.kompress_compressor import KompressConfig
|
|
from headroom.transforms.kompress_remote import RemoteKompressCompressor
|
|
|
|
|
|
def _long_text() -> str:
|
|
return " ".join(f"word{i}" for i in range(20))
|
|
|
|
|
|
def _compressor(transport: httpx.BaseTransport) -> RemoteKompressCompressor:
|
|
compressor = RemoteKompressCompressor(
|
|
"https://kompress.example",
|
|
token="secret",
|
|
config=KompressConfig(enable_ccr=False),
|
|
)
|
|
compressor._client = httpx.Client(transport=transport)
|
|
return compressor
|
|
|
|
|
|
def test_remote_kompress_posts_content_and_returns_result() -> None:
|
|
seen: dict[str, object] = {}
|
|
|
|
def handler(request: httpx.Request) -> httpx.Response:
|
|
seen["url"] = str(request.url)
|
|
seen["authorization"] = request.headers.get("authorization")
|
|
seen["json"] = request.read().decode()
|
|
return httpx.Response(
|
|
200,
|
|
json={
|
|
"compressed": "short result",
|
|
"original_tokens": 20,
|
|
"compressed_tokens": 2,
|
|
"compression_ratio": 0.1,
|
|
"model_used": "remote-model",
|
|
},
|
|
)
|
|
|
|
compressor = _compressor(httpx.MockTransport(handler))
|
|
try:
|
|
result = compressor.compress(_long_text(), target_ratio=0.3)
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert seen["url"] == "https://kompress.example/compress"
|
|
assert seen["authorization"] == "Bearer secret"
|
|
assert '"target_ratio":0.3' in str(seen["json"]).replace(" ", "")
|
|
assert result.compressed == "short result"
|
|
assert result.original_tokens == 20
|
|
assert result.compressed_tokens == 2
|
|
assert result.compression_ratio == 0.1
|
|
assert result.model_used == "remote-model"
|
|
|
|
|
|
def test_remote_kompress_short_input_skips_network() -> None:
|
|
called = False
|
|
|
|
def handler(request: httpx.Request) -> httpx.Response:
|
|
nonlocal called
|
|
called = True
|
|
return httpx.Response(200, json={"compressed": "unused"})
|
|
|
|
compressor = _compressor(httpx.MockTransport(handler))
|
|
try:
|
|
result = compressor.compress("too short")
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert called is False
|
|
assert result.compressed == "too short"
|
|
assert result.compression_ratio == 1.0
|
|
|
|
|
|
def test_remote_kompress_http_error_fails_open() -> None:
|
|
content = _long_text()
|
|
compressor = _compressor(httpx.MockTransport(lambda request: httpx.Response(503)))
|
|
try:
|
|
result = compressor.compress(content)
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert result.compressed == content
|
|
assert result.compression_ratio == 1.0
|
|
|
|
|
|
def test_remote_kompress_malformed_success_fails_open() -> None:
|
|
content = _long_text()
|
|
compressor = _compressor(httpx.MockTransport(lambda request: httpx.Response(200, json={})))
|
|
try:
|
|
result = compressor.compress(content)
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert result.compressed == content
|
|
assert result.compression_ratio == 1.0
|
|
|
|
|
|
def test_remote_kompress_null_numeric_field_fails_open() -> None:
|
|
# A 200 response with a valid 'compressed' but a malformed numeric field
|
|
# (here an explicit JSON null) must still fail open, not raise. data.get
|
|
# returns None for a present key, so float(None) would blow up if the
|
|
# coercions were outside the fail-open guard.
|
|
content = _long_text()
|
|
|
|
def handler(request: httpx.Request) -> httpx.Response:
|
|
return httpx.Response(
|
|
200,
|
|
json={"compressed": "short result", "compression_ratio": None},
|
|
)
|
|
|
|
compressor = _compressor(httpx.MockTransport(handler))
|
|
try:
|
|
result = compressor.compress(content)
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert result.compressed == content
|
|
assert result.compression_ratio == 1.0
|
|
|
|
|
|
def test_remote_kompress_non_numeric_field_fails_open() -> None:
|
|
# A non-numeric string in a numeric field is also a malformed response.
|
|
content = _long_text()
|
|
|
|
def handler(request: httpx.Request) -> httpx.Response:
|
|
return httpx.Response(
|
|
200,
|
|
json={"compressed": "short result", "original_tokens": "lots"},
|
|
)
|
|
|
|
compressor = _compressor(httpx.MockTransport(handler))
|
|
try:
|
|
result = compressor.compress(content)
|
|
finally:
|
|
compressor.close()
|
|
|
|
assert result.compressed == content
|
|
assert result.compression_ratio == 1.0
|
|
|
|
|
|
def test_content_router_selects_remote_kompress_from_env(monkeypatch) -> None:
|
|
monkeypatch.setenv("HEADROOM_KOMPRESS_ENDPOINT", "https://kompress.example")
|
|
monkeypatch.setenv("HEADROOM_KOMPRESS_ENDPOINT_TOKEN", "secret")
|
|
|
|
router = ContentRouter(ContentRouterConfig(ccr_inject_marker=False))
|
|
compressor = router._get_kompress()
|
|
try:
|
|
assert isinstance(compressor, RemoteKompressCompressor)
|
|
assert compressor.config == KompressConfig(enable_ccr=False)
|
|
assert compressor._url == "https://kompress.example/compress"
|
|
assert compressor._headers["authorization"] == "Bearer secret"
|
|
finally:
|
|
compressor.close()
|