Commit graph

2 commits

Author SHA1 Message Date
Dima Solodukha
4e30dde2ac
fix(router): compact JSON evades compression via whitespace token counting (#1857)
## Description

`ContentRouter` counts section tokens with `len(content.split())`. On
compact machine-generated JSON — the default output of
`json.dumps(separators=(",", ":"))`, `JSON.stringify`, and boto3 — there
are no spaces, so a large payload counts as ~1 "token". Every section
compression ratio then computes as ~1.0 and the `min_ratio` acceptance
gate silently rejects the compressor's real output: the router logs
`router:noop` while SmartCrusher separately logs `was_modified=true`.
Compression effectively no-ops on the most common agent payload type
(tool results returning JSON), on every provider.

## 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

- Add `_estimate_tokens(text)` — a size-proportional estimate
(`len(text) // 4`, floored at 1), monotone in content size for any
format.
- Replace the decision-relevant `len(...split())` counts in
`ContentRouter` (section original/compressed token counts feeding the
ratio gates, plus the debug estimates) with `_estimate_tokens(...)`.
- Add `tests/test_content_router_compact_json.py`.

## 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

```text
$ pytest tests/test_content_router_compact_json.py -q
2 passed, 1 warning

$ ruff check headroom/transforms/content_router.py tests/test_content_router_compact_json.py
All checks passed!

$ mypy headroom/transforms/content_router.py --ignore-missing-imports
Success: no issues found in 1 source file
```

## Real Behavior Proof

- Environment: `ContentRouter` invoked directly on a 150-item
ECS-service JSON tool_result, Python 3.13, estimator tokenizer.
- Exact command / steps: run the same payload two ways — compact
(`json.dumps(..., separators=(",", ":"))`) and the identical data with
spaces (`separators=(", ", ": ")`) — through
`ContentRouter(ContentRouterConfig(skip_user_messages=False))`.
- Observed result: before this change, compact JSON saved 0.0%
(`router:noop`) while the identical data with spaces saved 43.3%
(`router:tool_result:smart_crusher`) — same data, same compressor, only
whitespace differed. After this change, compact JSON compresses
equivalently to the spaced form.
- Not tested: no behavior change expected for content that already
tokenizes with whitespace (prose, code); those counts move from
word-count to chars/4 but the ratio comparison is self-consistent (both
sides use the same estimator).

## 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

Scope kept deliberately narrow: only the counts that feed
compression-acceptance decisions are changed. Non-decision `.split()`
uses elsewhere are left alone. Happy to add a CHANGELOG entry if you'd
like one.

---------

Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-07-14 06:54:01 -04:00
Zhenjia ZHOU
6c68ff4e9f
perf(compression): take large cold-start contexts off the synchronous kompress path (#1171) (#1298)
## Description

On a cold-start large context, kompress (ModernBERT ONNX) runs
**synchronously on the request thread** — ~200–300s for ~1M tokens. It
blows the 30s compression budget, leaks a non-preemptible worker, and
cascades (executor saturation → queue timeouts on healthy requests); on
timeout the request is forwarded **uncompressed** after eating 30s. This
adds four layered, **default-off, fail-open** mitigations so the request
path is never blocked on ML compression.

Closes #1171

## Type of Change

- [ ] Bug fix (non-breaking change that fixes an issue)
- [x] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [x] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- **Phase 0 — size gate** (`HEADROOM_KOMPRESS_MAX_TOKENS`, default
50000): route oversized text away from ModernBERT (→ LogCompressor /
TextCrusher / passthrough) at the single `_try_ml_compressor` boundary.
- **Phase 1 — cooperative deadline**
(`HEADROOM_COMPRESSION_DEADLINE_MS`, default 20000): any kompress run
self-terminates at the next chunk boundary past the budget, keeping the
unprocessed tail verbatim.
- **Phase 2 — TextCrusher** (`HEADROOM_TEXT_CRUSHER`): a new **native
Rust** extractive prose compressor in
`crates/headroom-core/src/transforms/text_crusher/`, exposed via PyO3 as
`headroom._core.TextCrusher` with a thin Python wrapper. It **reuses the
shared `crate::relevance::BM25Scorer`** rather than reimplementing BM25,
and ships record/replay parity fixtures (mirroring the SmartCrusher
Rust-core + Python-shim pattern).
- **Phase 3 — off-path compression**
(`HEADROOM_BACKGROUND_COMPRESSION`): forward uncompressed immediately
and compress in a per-process background drain; a byte-identical cache
hit on a later turn means the request never blocks on ML.
- Benchmark (`benchmarks/text_crusher_quality_eval.py`), CHANGELOG
entry, and docstrings documenting the fail-open limits.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`, new modules)
- [x] New tests added for new functionality
- [ ] Manual testing performed (Phase 0/1 gate-fire + deadline observed
on real traffic in earlier iterations; Phase 3 off-path is unit- +
byte-identity-tested, not yet live-validated)

### Test Output

```text
$ pytest tests/test_transforms/ tests/test_cache/ \
    tests/test_proxy/test_background_compression.py tests/test_proxy/test_phase3_byte_identity.py -q
501 passed, 37 skipped in 40.33s

$ cargo test -p headroom-core --lib text_crusher
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 834 filtered out

$ ruff check <changed files>
All checks passed!

$ mypy headroom/proxy/background_compression.py headroom/transforms/text_crusher.py
Success: no issues found in 2 source files
```

New coverage: size-gate incl. the strategy-dispatch funnel (KOMPRESS +
TEXT); partial-run deadline (chunk-0 compressed + chunk-1 verbatim
tail); BackgroundCompressor (dedup / queue-full / fail-open); Phase 3
byte-identity round-trip; TextCrusher unit + parity.

## Real Behavior Proof

- Environment: macOS, local dev — `uv` venv, Rust `_core` built via `uv
pip install -e .`.
- Exact command / steps: the `pytest` / `cargo test` / `ruff` / `mypy`
commands shown under Test Output; quality eval `python
benchmarks/text_crusher_quality_eval.py /tmp/squad_dev.json`.
- Observed result: 501 Python + 3 Rust tests pass; ruff + mypy clean on
changed/new modules. Quality eval: TextCrusher keeps ~94% of buried
SQuAD answers at 30% size vs ~36% truncate/random; self-contained speed
run ~333k words in ~76ms (one O(n) pass) — sub-second where ModernBERT
takes minutes (fast-vs-slow contrast, not a same-input run).
- Not tested: Phase 3 off-path on live traffic; multi-worker
(per-process by design — see Additional Notes).

## 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
- [x] 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
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

- **All four features are off by default and fail-open** — with the env
flags unset the paths are no-ops for realistic inputs; on any error the
request is forwarded (compressed if possible, else verbatim), never
dropped. A full background queue / duplicate key surfaces as
`deferred:dropped`.
- **Known limits (documented in `background_compression.py`):** Phase 3
is per-process, in-memory, and token-mode-only — these are
**lost-savings, never lost-correctness**, and consistent with the
project's existing per-process compression cache + sticky-session
multi-worker model. The startup multi-worker warning now names off-path
background compression.
- Phase 2 reuses the existing BM25 scorer; reuse did not improve
answer-retention over a Python prototype (query-awareness dominates) —
its value is the Rust speed + repo-conventional Rust-core/Python-shim
shape.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-23 10:48:06 -05:00