Skip to content

Commit b759990

Browse files
fix(transforms/kompress-remote): keep compress fail-open on malformed 200 (headroomlabs-ai#2320)
## 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>
1 parent 44a174f commit b759990

2 files changed

Lines changed: 56 additions & 9 deletions

File tree

headroom/transforms/kompress_remote.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,23 @@ def compress(
102102
compressed = data["compressed"]
103103
if not isinstance(compressed, str):
104104
raise TypeError("remote Kompress response field 'compressed' must be a string")
105+
# Coerce the numeric/string metadata fields inside the fail-open guard.
106+
# A 200 response with a malformed field (e.g. a non-numeric string, or
107+
# an explicit JSON null: data.get returns None for a present key, and
108+
# float(None)/int(None) raise) would otherwise escape uncaught and break
109+
# the proxy request, defeating the fail-open contract this class promises.
110+
result = KompressResult(
111+
compressed=compressed,
112+
original=content,
113+
original_tokens=int(data.get("original_tokens", n_words)),
114+
compressed_tokens=int(data.get("compressed_tokens", len(compressed.split()))),
115+
compression_ratio=float(data.get("compression_ratio", 1.0)),
116+
model_used=str(data.get("model_used", self.config.model_id)),
117+
)
105118
except Exception as e: # fail OPEN — never break the proxy on a bad endpoint
106119
logger.warning("Remote Kompress failed (%s); passing through", e)
107120
return self._passthrough(content, n_words)
108121

109-
result = KompressResult(
110-
compressed=compressed,
111-
original=content,
112-
original_tokens=int(data.get("original_tokens", n_words)),
113-
compressed_tokens=int(data.get("compressed_tokens", len(compressed.split()))),
114-
compression_ratio=float(data.get("compression_ratio", 1.0)),
115-
model_used=str(data.get("model_used", self.config.model_id)),
116-
)
117-
118122
# CCR stays PROXY-LOCAL: endpoint is stateless (enable_ccr=False), so we
119123
# store the mapping + append the retrieval marker here — same policy and
120124
# marker format as KompressCompressor.compress.

tests/test_transforms/test_kompress_remote.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,49 @@ def test_remote_kompress_malformed_success_fails_open() -> None:
9696
assert result.compression_ratio == 1.0
9797

9898

99+
def test_remote_kompress_null_numeric_field_fails_open() -> None:
100+
# A 200 response with a valid 'compressed' but a malformed numeric field
101+
# (here an explicit JSON null) must still fail open, not raise. data.get
102+
# returns None for a present key, so float(None) would blow up if the
103+
# coercions were outside the fail-open guard.
104+
content = _long_text()
105+
106+
def handler(request: httpx.Request) -> httpx.Response:
107+
return httpx.Response(
108+
200,
109+
json={"compressed": "short result", "compression_ratio": None},
110+
)
111+
112+
compressor = _compressor(httpx.MockTransport(handler))
113+
try:
114+
result = compressor.compress(content)
115+
finally:
116+
compressor.close()
117+
118+
assert result.compressed == content
119+
assert result.compression_ratio == 1.0
120+
121+
122+
def test_remote_kompress_non_numeric_field_fails_open() -> None:
123+
# A non-numeric string in a numeric field is also a malformed response.
124+
content = _long_text()
125+
126+
def handler(request: httpx.Request) -> httpx.Response:
127+
return httpx.Response(
128+
200,
129+
json={"compressed": "short result", "original_tokens": "lots"},
130+
)
131+
132+
compressor = _compressor(httpx.MockTransport(handler))
133+
try:
134+
result = compressor.compress(content)
135+
finally:
136+
compressor.close()
137+
138+
assert result.compressed == content
139+
assert result.compression_ratio == 1.0
140+
141+
99142
def test_content_router_selects_remote_kompress_from_env(monkeypatch) -> None:
100143
monkeypatch.setenv("HEADROOM_KOMPRESS_ENDPOINT", "https://kompress.example")
101144
monkeypatch.setenv("HEADROOM_KOMPRESS_ENDPOINT_TOKEN", "secret")

0 commit comments

Comments
 (0)