From 5e0f1a219f4d079a7004ca4db4532cafa0746dc4 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Tue, 14 Jul 2026 09:09:33 +0530 Subject: [PATCH] fix(transforms): guard the lossless diff fold to diff-shaped content only (#2140) ## Description `ContentRouter._lossless_first` tries several lossless folds and keeps the smallest result. Most folds self-verify reversibility, but the `diff` fold is subtractive: it removes `index ..` lines without an inverse check. That can silently delete matching lines from non-diff text, logs, or search output. ## Fix - Skip the `diff` fold unless the detected strategy is `CompressionStrategy.DIFF` or the content is structurally diff-shaped. - Keep genuine diffs folding their `index` bookkeeping. - Add regression coverage for a non-diff `index ...` line and a real diff. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/transforms/content_router.py tests/test_lossless_diff_fold_guard.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/transforms/content_router.py tests/test_lossless_diff_fold_guard.py headroom/memory/factory.py 3 files already formatted git diff --check headroomlabs/main...HEAD # no output uv run --extra dev python -m pytest tests/test_lossless_diff_fold_guard.py -q 2 passed ``` ## Review Readiness - [x] Ready for review - [x] Regression tests added - [x] CHANGELOG updated Co-authored-by: JerrettDavis --- CHANGELOG.md | 1 + headroom/transforms/content_router.py | 12 ++++++++ tests/test_lossless_diff_fold_guard.py | 38 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 tests/test_lossless_diff_fold_guard.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d1292d08b..35c88a141 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,6 +111,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **wrap/doctor:** make the Claude Remote Control gate warning accurate and stop it firing for users who never had the feature ([#1779](https://github.com/headroomlabs-ai/headroom/issues/1779)). Claude Code 2.1.196 added a client-side check that **deterministically** disables first-party Remote Control (`/remote-control` / `/rc`) whenever `ANTHROPIC_BASE_URL` points at a non-`api.anthropic.com` host — which Headroom always does. The old notice hedged ("may hide the Remote Control menu"); it now states the disable as fact, names the `/rc` command, and detects the installed Claude Code version so the wording is exact (`2.1.196` when known, `2.1.196+` when not). The gate is upstream and RC's control-plane talks to `claude.ai` (not the API host), so Headroom cannot restore it — the warning tells you to run Claude without Headroom for RC sessions. The warning is suppressed for auth modes that never had Remote Control (API-key/PAYG via `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`, and Bedrock/Vertex/Foundry cloud IAM) and on Claude Code builds older than 2.1.196 where RC is unaffected by a custom base URL. Both the `headroom wrap claude` launch banner and `headroom doctor` co-report the sibling base-URL gates Headroom *does* restore — on-demand tool loading (#746, automatic) and the 1M context window (#1158, via `--1m`) — and the wrap-side co-report is session-accurate: it says "already restored via --1m" when the flag is in effect and reports tool deferral OFF (not falsely "kept on") when the user chose `--tool-search false`/`ENABLE_TOOL_SEARCH=false`; the `ENABLE_TOOL_SEARCH=...` banner line got the same accuracy fix. `is_custom_anthropic_base_url` now recognizes scheme-less values (`myproxy.local:8080`, `127.0.0.1:8787`) as custom hosts and degrades gracefully on malformed URLs instead of crashing `doctor`. `doctor` resolves the Claude Code version lazily, so runs with no custom base URL never pay the `claude --version` subprocess. No request bytes are touched (cache-safe); this is UX/notice-only. * **install:** default the docker image to `ghcr.io/headroomlabs-ai/headroom:latest` instead of the dead `ghcr.io/chopratejas/headroom:latest`. After the repo moved to the `headroomlabs-ai` org, GHCR did not redirect the old package, so `headroom install` / `headroom init` and the install scripts pulled a frozen `0.27.0` image while current releases publish to the new path ([#1867](https://github.com/headroomlabs-ai/headroom/issues/1867)). * **transforms/content-router:** stop a profile-derived `read_protection_window` kwarg from weakening an explicit `--protect-tool-results` guarantee. `ContentRouter.apply()` computes `read_protection_window` from `protect_recent_reads_fraction`, where `0.0` (the sentinel `--protect-tool-results` sets) means "protect all excluded-tool output regardless of conversation depth" per #1374's documented contract — but the method then unconditionally overwrote that window with a `read_protection_window` kwarg whenever one was present. `proxy_pipeline_kwargs()` supplies that kwarg on every request from the active `AgentSavingsProfile.protect_recent` (the default `coding` profile sets `protect_recent=2`), so in practice only the last 2 messages ever kept read-protection and older excluded-tool output silently fell through to lossy compression. The runtime kwarg may now only narrow the window when `protect_recent_reads_fraction > 0`; it can no longer shrink the "protect everything" guarantee set by `--protect-tool-results`. +* **transforms:** stop the lossless `diff` fold from silently dropping lines out of non-diff content. `ContentRouter._lossless_first` tries every `compact_lossless` fold on all content, but the `diff` kind (`diff_strip_index`) is the only one with no exact-inverse check — it removes any line shaped like `index ..`. Applied to arbitrary text/log/search payloads that happen to contain such a line, that line was deleted with no CCR marker, so it was unrecoverable — a violation of the lossless no-loss contract the method's own docstring promises. The `diff` fold now runs only when the strategy is `DIFF` or the content is diff-shaped (`_looks_like_diff`); genuine diffs still have their `index` bookkeeping folded. * **memory:** include the Ollama server URL in the embedder cache key so a second backend can't get an embedder bound to the wrong server. `_create_embedder` cached by `(backend, model)` only, but the Ollama embedder is constructed with `base_url=config.ollama_base_url`. Two configs in the same process that shared a backend and model but pointed at different Ollama servers (e.g. a per-project storage router) collided on one cache slot, so the second silently reused the first's embedder and embedded against the wrong host. The cache key now also includes `ollama_base_url`. * **tokenizers:** use `o200k_base` for the gpt-4.1 / gpt-4.5 / o4 families in `get_encoding_for_model`. `gpt-4.1*` and `gpt-4.5*` matched the broad `gpt-4` prefix and were encoded with `cl100k_base`, and `o4*` matched no prefix and fell through to the `cl100k_base` default — all three use `o200k_base`, so their token counts were computed with the wrong vocabulary. Added explicit `gpt-4.1`/`gpt-4.5` prefixes ahead of `gpt-4` and an `o4` prefix; `gpt-4` and `gpt-3.5` snapshots still resolve to `cl100k_base`. * **cache/ccr:** stop counting a successful eviction as a retrieval in the compression feedback learner, which inverted the learning signal. When an entry is evicted without ever being retrieved, `CompressionStore` emits a synthetic `retrieval_type="eviction_success"` event to mark that the compression was sufficient (the LLM never needed the original). `CompressionFeedback.record_retrieval` had no branch for it, so — because the type is not `"full"` — it was counted as a *search retrieval*, inflating the tool's `retrieval_rate`/`search_rate`. `get_compression_hints` reads a high retrieval rate as "compressing too aggressively" and backs off, so a compression that actually worked pushed the learner toward *less* compression (a standalone repro scores one successful eviction as a 100% retrieval rate). The event is now recognized and left out of the retrieval counters; the compression is still counted by `record_compression` at store time, so a never-retrieved entry correctly yields a low retrieval rate. Genuine retrievals are unaffected. diff --git a/headroom/transforms/content_router.py b/headroom/transforms/content_router.py index 22f06e520..5e7f26b3e 100644 --- a/headroom/transforms/content_router.py +++ b/headroom/transforms/content_router.py @@ -1800,6 +1800,18 @@ class ContentRouter(Transform): order = ([primary] if primary else []) + [ k for k in ("search", "paths", "log", "diff", "text") if k != primary ] + # The "diff" fold (diff_strip_index) is the one compact_lossless kind that + # is purely subtractive with NO exact-inverse check: it removes any line + # shaped like `index ..`. On non-diff content that happens to + # contain such a line, that line is silently and unrecoverably dropped — + # breaking the lossless contract this method's docstring promises, and + # unmarked in CCR mode. Only fold diffs as diffs. + if ( + "diff" in order + and strategy is not CompressionStrategy.DIFF + and not self._looks_like_diff(content) + ): + order = [k for k in order if k != "diff"] best, best_label = content, None for kind in order: try: diff --git a/tests/test_lossless_diff_fold_guard.py b/tests/test_lossless_diff_fold_guard.py new file mode 100644 index 000000000..4312989f2 --- /dev/null +++ b/tests/test_lossless_diff_fold_guard.py @@ -0,0 +1,38 @@ +"""The lossless `diff` fold is purely subtractive with no inverse check, so it +must only run on diff-shaped content — never on arbitrary text that happens to +contain an `index ..` line.""" + +from __future__ import annotations + +from headroom.transforms.content_router import CompressionStrategy, ContentRouter + + +def _lossless_first(content: str, strategy: CompressionStrategy): + # _lossless_first only uses the (static) _looks_like_diff and a lazy import, + # so a bare instance exercises it without the full router init. + router = object.__new__(ContentRouter) + return router._lossless_first(content, strategy) + + +def test_diff_fold_does_not_drop_index_line_from_non_diff_text(): + content = "Here are the object refs:\nindex 0123abc..def4567\nAll done.\n" + + out, label = _lossless_first(content, CompressionStrategy.PASSTHROUGH) + + # The git-blob-index-shaped line must survive; nothing should fold. + assert "index 0123abc..def4567" in out + assert out == content + assert label is None + + +def test_diff_fold_still_applies_to_real_diffs(): + diff = ( + "diff --git a/x b/x\nindex 1111111..2222222 100644\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-a\n+b\n" + ) + + out, label = _lossless_first(diff, CompressionStrategy.DIFF) + + # A genuine diff still gets its index bookkeeping folded (semantic-lossless + # for `git apply`). + assert "index 1111111..2222222" not in out + assert label == "lossless_diff"