Commit graph

2 commits

Author SHA1 Message Date
Abhay Singh
b75999017f
fix(transforms/kompress-remote): keep compress fail-open on malformed 200 (#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>
2026-07-17 12:11:41 -07:00
Tejas Chopra
b6eb7a7613
feat(kompress): optional remote compression endpoint (HEADROOM_KOMPRESS_ENDPOINT) (#2171)
## Description

Adds an **opt-in remote Kompress backend** so the proxy can offload
Kompress ML inference to a hosted `/compress` endpoint instead of
loading the ONNX model in-process.

This lets Headroom run as a lean proxy in a sandbox installed with only
`[proxy]` deps while the model runs elsewhere. The feature is purely
additive: with `HEADROOM_KOMPRESS_ENDPOINT` unset, behavior remains the
existing in-process Kompress path.

## Type of Change

- [x] New feature (non-breaking change that adds functionality)

## Changes Made

- `headroom/transforms/kompress_remote.py`: adds
`RemoteKompressCompressor`, a `KompressCompressor`-compatible HTTP
client that posts to `/compress`, sends optional bearer auth, skips
network for tiny inputs, and fails open on
HTTP/network/malformed-response errors.
- `headroom/transforms/kompress_compressor.py`: extracts
`store_kompress_in_ccr()` so the remote client reuses the same
proxy-local CCR marker/storage policy without importing the ML model.
- `headroom/transforms/content_router.py`: selects the remote compressor
when `HEADROOM_KOMPRESS_ENDPOINT` is set, while `"disabled"` still wins
and the unset path remains local Kompress.
- `tests/test_transforms/test_kompress_remote.py`: covers mocked remote
success, auth/header/request behavior, tiny-input no-call behavior, HTTP
fail-open, malformed-success fail-open, and router env selection.

## Testing

- [x] Unit tests pass (`uv run --extra dev pytest
tests/test_transforms/test_kompress_remote.py -q`)
- [x] Linting passes (`uvx ruff@0.15.17 check
headroom/transforms/kompress_remote.py
headroom/transforms/kompress_compressor.py
headroom/transforms/content_router.py
tests/test_transforms/test_kompress_remote.py`)
- [x] Formatting passes (`uvx ruff@0.15.17 format --check
headroom/transforms/kompress_remote.py
headroom/transforms/kompress_compressor.py
headroom/transforms/content_router.py
tests/test_transforms/test_kompress_remote.py`)
- [x] Type checking passes (`uv run --extra dev mypy
headroom/transforms/kompress_remote.py
headroom/transforms/kompress_compressor.py
headroom/transforms/content_router.py`)
- [x] New tests added for new functionality
- [x] Manual testing performed by the author against a live endpoint

## Real Behavior Proof

- Environment: Windows 11 review worktree, Python 3.13.3 for mocked
tests; author also manually tested against a Modal deployment of
`chopratejas/kompress-v2-base`.
- Exact command / steps: ran the focused mocked endpoint test file plus
lint/format/mypy on the changed modules.
- Observed result: remote success maps endpoint response into
`KompressResult`; short inputs do not call the network; 503 responses
and malformed 200 responses return the original content; router selects
the remote compressor only when the env var is set.
- Not tested: full `pytest` suite; production concurrency/latency under
load; endpoints other than the author's Modal reference deployment.

## 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 — follow-up
README flag section
- [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

- The endpoint/deploy artifact (`modal_serve.py`) lives in the separate
`kompress` repo; this PR is only the client-side flag.
- The endpoint is intentionally stateless for CCR. Original-content
storage and retrieval markers remain proxy-local.
- Design note: this capability is intentionally in OSS as an opt-in
flag. The same flag serves self-hosted endpoints and, later, a hosted
endpoint.

---------

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-13 22:01:11 -07:00