mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tejas Chopra
|
981616c60e
|
feat(transforms): make built-in compressors real Compressor implementations (adapters) (#2391)
## What
Turns each built-in registry entry into a working `Compressor` (the
`compressor_registry` contract): `compress(CompressInput) ->
CompressOutput` delegates to the same underlying built-in method the
content router already invokes in `_apply_strategy_to_content`, reached
through the router's own `_get_*` getter so config flows through
identically. Token counts use the router's `_estimate_tokens`;
`lossless` mirrors the descriptor; `recoverable` is `{}` (built-ins
persist CCR recovery to the store as a side effect, not via their return
value).
Adapted: `smart_crusher, code_aware, search, log, tabular, config, html,
kompress`.
## Behavior change
**None — additive by construction.** Dispatch, the `_get_*` getters,
fallback chains, the reversibility gate, and config are all unchanged.
The router still dispatches built-ins via its existing if/elif and never
routes a request through the registry;
`_resolve_active_external_compressors` filters built-in entries out of
the opt-in external-dispatch path *by type* (the class name
`_BuiltinCompressorEntry` is load-bearing). A default request is
byte-identical: `_active_external_compressors == []`, external dispatch
is an inert guard, and adapters are reachable only via
`compressor_registry.get()/active()`.
## `image` — documented passthrough (not a guess)
`ImageCompressor.compress(messages)` operates on image blocks inside
message dicts, not `str` content, and isn't on the
`_apply_strategy_to_content` path, so there's no faithful `str→str`
delegation. Its adapter is a documented non-raising passthrough rather
than a fabricated one.
## Testing
`tests/test_builtin_compressor_adapters.py` — differential tests
asserting each adapter's output matches the built-in's direct output
(JSON→smart_crusher, CSV→tabular, log lines→log, grep→search,
config→config, Python→code_aware, HTML→html); kompress is mocked (no ML
inference); every registry entry has a working non-raising `compress`.
Updated the obsolete guard test in `test_compressor_selection.py`.
Offline suite: 72 passed; ruff + mypy clean. (Broad
content-router/compression suite deferred to CI — it needs HF-Hub/ONNX
model loads.)
This is PR-A of the adapter phase (built-ins become Compressor
implementations); flipping the router's dispatch to registry-resolved is
the follow-up. Builds on #2370/#2371/#2373/#2388.
|
||
|
Zhenjia ZHOU
|
a84b28af0e
|
fix(proxy): warn when --compressor selection matches no built-in (#2385)
## Description A `--compressor` selection that matches no built-in name (e.g. `smart_krusher`, a typo of `smart_crusher`) silently disables **all** built-in compression: the proxy starts healthy, the dashboard shows ~0 savings, and nothing explains why. The all-off *semantics* is deliberate and stays untouched — `test_only_external_name_disables_all_builtins` pins the opt-in "exactly these" contract, and external/registry names are a legitimate input class. What's missing is any **signal**: a typo and an external compressor name are indistinguishable at this seam, and the registry's own unregistered-name warning (`CompressorRegistry.select`) never runs on this path. Fix: `_apply_compressor_selection` now logs one warning when the selection contains unmatched names — - **nothing matched** (the typo case): says plainly that every built-in compressor is now disabled and lists the valid names + `*`; - **mixed**: names the unmatched entries as assumed registry names. Selection results are byte-identical before/after. Fixes #2384. ## Type of Change - [x] Bug fix (observability for a silent misconfiguration; no behavior change) ## Changes Made - `headroom/proxy/server.py`: `_apply_compressor_selection` computes the unmatched set and emits one `headroom.proxy` warning (two phrasings: nothing-matched vs mixed); docstring updated. - `tests/test_compressor_selection.py`: 3 new tests — typo-only selection warns (and flags stay all-off, pinning the unchanged contract), mixed selection warns only about the unmatched name, matched/wildcard selections stay warning-free. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff` + `mypy`, CI-pinned settings) - [x] Wrote the failing tests first, then the warning ### Test Output ```text $ .venv/bin/python -m pytest tests/test_compressor_selection.py -q 26 passed # Before the fix the two new warning tests fail (no log records emitted). $ ruff check headroom/proxy/server.py tests/test_compressor_selection.py # All checks passed! $ ruff format --check <both> # formatted $ mypy headroom/proxy/server.py --ignore-missing-imports # Success: no issues ``` ## Real Behavior Proof - Environment: macOS (Darwin), Python in a uv venv, branch `fix/compressor-selection-warn` off `main` (`56c7d4a5`). - Exact command / steps: configured stdlib logging at WARNING and called `_apply_compressor_selection(ContentRouterConfig(), {"smart_krusher"})` — the exact typo scenario from #2384. - Observed result: `WARNING headroom.proxy: compressor selection smart_krusher matches no built-in compressor — every built-in compressor is now disabled. If this is a typo, valid names are: code_aware, config, html, image, kompress, log, search, smart_crusher, tabular (or '*' for all).` with `enable_smart_crusher = False` (contract unchanged). Before the fix the same call produced zero log output. - Not tested: a full `headroom proxy --compressor smart_krusher` process launch; the seam is exercised directly and the proxy wires it unconditionally. ## 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 — N/A (docstring updated) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md — N/A ## Additional Notes - Deliberately warn-only: erroring here would break legitimate external/registry selections and could brick startup on a stale `HEADROOM_COMPRESSORS` settings value. If you'd rather hard-fail just the CLI-typed path, happy to follow up. |
||
|
Tejas Chopra
|
56c7d4a59e
|
feat(proxy): select built-in compressors via --compressor + registry inventory (#2373)
## What - Adds an opt-in `--compressor` / `HEADROOM_COMPRESSORS` selection that narrows the active built-in compressors, mapped onto the existing `ContentRouterConfig` `enable_*` flags at the proxy config seam. Recognized names: `smart_crusher, kompress, code_aware, search, log, tabular, config, html, image`; `"*"` selects all. - Builds a name-addressable compressor registry in `ContentRouter`: a metadata-only descriptor per built-in plus opt-in discovery of `headroom.compressor` entry points (the seam added in #2370). ## Why Built-in compressors were only reachable through a hardcoded if/elif; there was no supported way to select a subset (7 of the `enable_*` flags had no external surface) or to see the built-ins as a name-addressable set alongside third-party ones. ## Behavior change **None by default.** `--compressor` unset (the default) leaves every `enable_*` flag at its dataclass default, so the request path is byte-identical to today. The registry is inventory-only — built-ins are still constructed and dispatched by the existing if/elif; `_BuiltinCompressorEntry.compress` deliberately raises (never called), and registry construction is fail-open. Routing an external compressor *through* the pipeline is a deliberate follow-up. ## How - `server.py`: `BUILTIN_COMPRESSOR_FLAGS` map + `_apply_compressor_selection(router_config, compressors)` (no-op when `None`/empty; runs before the `disable_kompress` override so that stays authoritative). - `models.py`: `ProxyConfig.compressors: set[str] | None = None`. - `cli/proxy.py`: `--compressor` (repeatable, comma-split, `HEADROOM_COMPRESSORS`), mirroring `--proxy-extension`. - `content_router.py`: built-in descriptors + `_build_compressor_registry()` (register built-ins, then fail-open `discover()`), exposed as `self.compressor_registry`. Dispatch unchanged. ## Testing `tests/test_compressor_selection.py` — 23 tests: selection mapping (None/empty/whitespace = byte-identical defaults, single/multi/wildcard, external-only disables built-ins, unrecognized ignored), `ProxyConfig` field, registry inventory (descriptors cover the 9 names, valid cost tiers, router exposes registry, inventory doesn't auto-activate, built-in `compress` guard, discovery merges external, fail-open on discovery error). Local: 23 passed; ruff + mypy clean on changed files. Full suite runs in CI. Stacks conceptually on #2370 (registry seam); rebased onto `main` after that merged. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |