mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
8 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
ulias
|
e36fccd8cf
|
refactor: DRY cache logic, add thread safety, fix Bash exclusion (#704)
## Description Four targeted improvements to ContentRouter and configuration, refactoring ~120 lines of duplicated cache logic into a shared helper and fixing several correctness issues. ### 1. DRY: Extract `_compress_block_content` helper The two-tier cache lookup + compression logic was duplicated ~60 lines per path (tool_result blocks and text blocks in `_process_content_blocks`). Extracted into a single, shared helper method. Net reduction of ~80 lines; no behavioural change. ### 2. Thread-safe `CompressionCache` `CompressionCache` is read/modified from `ThreadPoolExecutor` workers during parallel compression in `apply()`. Added a `threading.Lock` guarding all read-modify-write operations so concurrent cache misses for the same content do not produce duplicate compression work and metrics counters stay consistent. ### 3. Remove duplicate Kompress fallback for SmartCrusher The SMART_CRUSHER strategy block had an inline Kompress fallback that ran when SmartCrusher produced no savings. The unified post-strategy fallback block already covers the same case — the inline copy was a duplicate Kompress invocation. Removed it; the post-strategy handler now owns all fallback decisions for both SMART_CRUSHER and CODE_AWARE. Also added a guard preventing duplicate Kompress when CODE_AWARE's inline fallback fires alongside the unified block. ### 4. Fix Bash exclusion contradiction in `DEFAULT_EXCLUDE_TOOLS` The docstring on `DEFAULT_EXCLUDE_TOOLS` explicitly states "Bash is NOT excluded — its outputs (build logs, test output) are ideal compression targets." But both "Bash" and "bash" were still in the frozenset. Removed them so code matches the documented intent. Closes # ## 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 - [x] Code refactoring (no functional changes) ## Changes Made - `headroom/config.py`: Remove Bash/bash from `DEFAULT_EXCLUDE_TOOLS` - `headroom/transforms/content_router.py`: Extract `_compress_block_content` helper; unified post-strategy fallback block; threading.Lock on CompressionCache; CODE_AWARE duplicate guard - `headroom/client.py`: Replace silent `except Exception: pass` with `logger.debug(..., exc_info=True)` - `tests/test_compression_cache.py`: Add 2 concurrency regression tests - `tests/test_transforms/test_content_router.py`: Add 14 tests covering Bash exclusion, SmartCrusher fallback chain, and `_compress_block_content` shared path ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Formatting passes (`ruff format --check .`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text # 14 new tests added across 3 test classes: # TestExcludeTools: 3 tests (Bash not in DEFAULT_EXCLUDE_TOOLS) # TestSmartCrusherFallback: 4 tests (fallback chain, no duplicate Kompress, JSON direct hit, CODE_AWARE path) # TestCompressBlockContent: 5 tests (skip set, result cache, ratio gating, route counts, transforms tracking) # TestCompressionCache: 2 tests (concurrent hits/misses consistency, stable hash ops no race) # Local run (43 tests pass): $ pytest tests/test_compression_cache.py tests/test_transforms/test_content_router.py -v ...43 passed... # ruff check: $ ruff check headroom/client.py headroom/config.py headroom/transforms/content_router.py tests/test_compression_cache.py tests/test_transforms/test_content_router.py All checks passed! # ruff format: $ ruff format --check headroom/client.py headroom/config.py headroom/transforms/content_router.py tests/test_compression_cache.py tests/test_transforms/test_content_router.py 5 files already formatted ``` ## Real Behavior Proof - Environment: Python 3.12, Linux (CI), headroom with headroom._core Rust extension compiled - Exact command / steps: CI run https://github.com/chopratejas/headroom/actions/runs/27326150021 — 13/16 jobs pass; 2 failures were lint+commitlint (both fixed in subsequent commits); 1 failure is pre-existing test(4) which monkeypatches time.time() but the CompressionCache uses time.monotonic() — unrelated to our changes - Observed result: All 14 new tests pass in CI; SmartCrusher fallback chain deterministically shows [smart_crusher, kompress] or [smart_crusher, kompress, log] when SmartCrusher produces no savings, with no duplicate entries - Not tested: fork-PR CI path where GitHub secrets are not available; local Windows environment where headroom._core Rust extension is not built ## 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes The pre-existing CI failure in `test (4)` is `test_compression_cache_handles_hits_skips_evictions_and_clear` in `tests/test_transforms_content_router.py`. It monkeypatches `time.time()` but the `CompressionCache` (content_router-local, line 191) uses `time.monotonic()` for TTL — the monkeypatched clock never advances, and `is_skipped()` always returns True. This failure exists on `main` and is unrelated to our changes (we only modified the other CompressionCache in `headroom/cache/compression_cache.py`). --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
chopratejas
|
89f7b6c2dd |
fix: complete /v1/responses compression telemetry, multi-frame WS, frozen-count cap
Builds on PR #406 (HTTP /v1/responses PyO3) and PR #410 (WS first-frame PyO3) which landed the binding for `compress_openai_responses_live_zone`. This change closes the remaining gaps so every (provider × endpoint × auth-mode × streaming) combination compresses AND surfaces in the dashboard. Telemetry: extend the PyO3 binding return tuple from `(bytes, modified)` to `(bytes, modified, tokens_saved, transforms_applied)` by adding `CompressionManifest::tokens_saved()` and `transforms_applied()` accessors on the existing manifest. The Python proxy populates request-log telemetry from the binding output instead of recounting tokens. Updates the existing 2-tuple call sites in HTTP and WS first-frame, plus the unpacks in tests. WebSocket multi-frame compression: subscription Codex users keep a long-lived WS open and send multiple `response.create` events per session. PR #410 only compressed the first frame; subsequent frames went raw. Added `_maybe_compress_response_create_frame` closure inside `_client_to_upstream` that runs the same Rust dispatcher on every client→upstream `response.create` text frame, passes other event types (response.cancel, session.update, etc.) through unchanged, and accumulates `tokens_saved` / `transforms_applied` / `ws_frames_compressed` counters across the session. Pre-existing dashboard gap: `streaming.py` and `anthropic.py` write `RequestLog` entries; the non-streaming OpenAI HTTP and WS handlers did not. Result: /transformations/feed was invisible for every Codex turn and every Cline / OpenClaude / Aider turn. Added the same wiring in `handle_openai_chat` (non-streaming), `handle_openai_responses` (non-streaming HTTP), and `handle_openai_responses_ws` (session-end). All three populate `auth_mode` + `endpoint` tags so the dashboard can break compression activity down by client class (PAYG / OAuth / Subscription) and surface (`chat_completions` / `responses_http` / `responses_ws`). The WS metric record is now unconditional — was previously gated on `tokens_saved > 0`, so first-frame no-changes never registered. compute_frozen_count over-freeze for prose-format clients: `compute_frozen_count` walked until it found an unstable `tool_result` / `role: "tool"` block. Cline / OpenClaude / Aider — clients that embed tool calls as XML inside plain text — never produce such a boundary, so the function returned `len(messages)` and the pipeline froze 100% of messages including the brand-new user turn. Live zone empty → `Transform content_router: 16414 → 16414 tokens (saved 0)`. Reported on Discord 2026-05-07 with Cline+DeepSeek. Fix: cap at `max(0, len(messages) - 1)`. Updates 3 existing test assertions whose expected values encoded the old over-freeze. Adds 6 new prose-format invariant tests. CodeQL "clear-text logging of sensitive information" fix: `tests/e2e_real_compression.py` previously stored API keys in local variables in the same scope as diagnostic prints, which CodeQL flagged via data-flow analysis. Refactored to read keys from `os.environ` inside the request helper — the credentials never enter the runner's main scope, so the taint flow never reaches the print. End-to-end verification with real keys (.env): /v1/messages (PAYG, non-stream) tok 14109 → 969 saved 13140 /v1/messages (PAYG, stream) tok 14109 → 969 saved 13140 /v1/chat/completions (PAYG, non-stream) tok 18460 → 1374 saved 17086 /v1/chat/completions (PAYG, stream) tok 18460 → 1374 saved 17086 (cache_hit=100%) /v1/responses HTTP (PAYG, non-stream) bytes 50138 → 597 saved 18391 /v1/responses WS (frame 1) bytes 46429 → 488 saved 16791 /v1/responses WS (frame 2 multi) bytes 46429 → 488 saved 16791 /v1/responses WS (response.cancel) passthrough untouched Tests: cargo workspace + pytest (4846 pass, 0 fail), make ci-precheck passed, two E2E scripts (multi-turn HTTP, WS fake-upstream) all green. |
||
|
chopratejas
|
ea78cf6252 |
fix(proxy): cache concurrency lock, multi-worker docs, bounded compression executor
Three audit follow-ups from issue #327's deep-dive review. C1 — CompressionCache concurrency lock ====================================== `CompressionCache` instances are shared per `session_id` and accessed from async-dispatched threadpool workers. Pre-fix, concurrent requests for the same session raced on `_cache`, `_stable_hashes`, `_first_seen`, and `_total_tokens_saved` with no synchronization. Observable failures: * Lost-update on `_total_tokens_saved` (read-modify-write). * `RuntimeError: OrderedDict mutated during iteration` from `apply_cached` when a concurrent `store_compressed` evicts during the walk. * Lost stable-hash records — next-turn compute_frozen_count reads inconsistent state. May also explain part of SvenMeyer's `_cache: 0 entries / 1003 misses` observation: the cache was being clobbered concurrently. Added `threading.RLock` guarding all mutating methods. `RLock` (not `Lock`) so future code can call locked methods from inside another locked method without self-deadlock. Also locked `HeadroomProxy._compression_caches` dict-of-caches access via a separate `_compression_caches_lock` so two concurrent calls for the same session_id can't each create distinct CompressionCache objects (which would split the cache state between them). The `/stats` endpoint snapshots the cache list under the dict lock before iterating to avoid eviction-during-iteration. C2 — Multi-worker CCR fragmentation: documented + startup warning ================================================================= The in-memory `InMemoryCcrStore` (Rust), `_compression_caches` (Python), `session_tracker_store` (Python), and TOIN learner state are ALL per-process. Multi-worker uvicorn round-robins requests across workers, so a session whose turn-1 lands on worker A may have turn-2 land on worker B. Worker B has zero knowledge of A's CCR markers, replay cache, or prefix-cache state. Result: `Retrieve original: hash=X` markers stay in-context as opaque directives, every fresh tool_result is recompressed from scratch, and `frozen_message_count=0` causes Anthropic prefix-cache busts on every cross-worker turn. Added a "Multi-worker deployment — CCR fragmentation" section in `RUST_DEV.md` documenting the failure modes, the supported configuration (`--workers 1`), and the sticky-session workaround for horizontal scale. The proxy emits a `WARNING`-level log line on startup if `workers > 1` is detected, pointing at the doc section. C3 — Bounded compression executor with cancel-aware metrics =========================================================== `asyncio.wait_for(asyncio.to_thread(pipeline.apply), timeout=...)` cancellation does NOT propagate into the threadpool worker that's running Rust code. Once the worker has picked up the task, `concurrent.futures.Future.cancel()` returns False and the thread runs to completion. Stuck threads accumulated invisibly on asyncio's default executor, contending with unrelated `to_thread` callers (file IO, etc.). Replaced all 7 `asyncio.to_thread` call sites for `pipeline.apply()` across `proxy/handlers/anthropic.py` (3) and `proxy/handlers/openai.py` (4) with a new `HeadroomProxy._run_compression_in_executor(fn, *, timeout)` helper that: 1. Submits to a dedicated bounded `ThreadPoolExecutor` named `headroom-compress` (configurable via `ProxyConfig.compression_max_workers`; defaults to `min(32, (cpu_count or 1) * 4)`). 2. Increments `_compression_in_flight` (gauge) when work starts and decrements when work completes; tracks `_compression_in_flight_max` as a high-water mark. 3. Detects "leaked threads" by comparing wall-clock elapsed against the timeout in the worker's `finally` block. Increments `_compression_leaked_threads` when a worker finishes after its asyncio future was cancelled. Operators can see the leaked-thread rate climbing in `/stats runtime.compression_executor` BEFORE the pool fills up. Tests ===== * `TestCompressionCacheConcurrency` (3 tests) — many threads store_compressed / apply_cached / update_from_result on a single CompressionCache; assert no exceptions, no lost updates, no partial state. * `test_get_compression_cache_returns_same_instance_under_contention` — 32 concurrent `_get_compression_cache(same_id)` calls return the identical instance (would split pre-lock). * `test_proxy_compression_executor.py` (8 tests) — pool size respects config, in-flight gauge tracks running compressions, high-water mark is monotonic, timeout propagates to awaiter, leaked-thread counter increments on post-deadline completion, `/stats` surfaces all three gauges. Verification ============ * All 123 targeted regression tests pass. * `make ci-precheck` clean. * No `Co-Authored-By` trailer; conventional `fix:` prefix; no `--no-verify`. |
||
|
chopratejas
|
44944fb3fe |
fix(proxy): restore Anthropic compression on token mode (issue #327)
Three bugs combined to drive end-to-end compression on the Anthropic backend to ~0% in token mode (the default). User report #327 saw a ~9× drop in dashboard savings from one day to the next on Claude Code traffic; the dashboard headline was technically correct but the underlying compression genuinely was not running. After this change the same Claude Code-shape multi-turn conversation goes from 14987 → 14371 tokens at the request boundary on turn 1 and only recompresses the freshest tool_result on subsequent turns, with the prior turns frozen byte-identical to preserve the upstream prefix cache. Bug 1 — IntelligentContextManager inner ContentRouter has no observer PR #302 (commit |
||
|
JerrettDavis
|
ca728a4d35 |
fix(ci): harden wrap e2e validation
Make the Docker wrap e2e harness validate live proxy env wiring for Codex and Aider, start a real OpenClaw gateway in-container, and clear the repo-wide Ruff issues that were keeping the Python 3.12 CI job red. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Tejas Chopra
|
22dad133e2 |
fix: eliminate prefix cache busts from frozen count underestimation
Root cause: CompressionCache.compute_frozen_count() stopped at the first tool_result not in its cache, capping frozen_message_count at 2. Tool results excluded by content_router (Read/Glob) or skipped (ratio too high) never entered the cache, so every subsequent message was eligible for recompression — causing 192 cache busts per session. Four fixes: 1. Add _stable_hashes set to CompressionCache so excluded/skipped tool_results don't block the frozen count walk 2. Fix _estimate_message_tokens to count tool_result content and tool_use input fields (were counted as 0 tokens in Anthropic format) 3. Fix streaming handler to include assistant response and original_messages in prefix tracker updates (parity with non-streaming) 4. TTL-aware batch recompression: defer first-time compressions within the 5-min cache TTL window, batching them at the boundary to trade many small busts for one |
||
|
chopratejas
|
0f2f992132 |
feat: add frozen count, apply_cached, update_from_result to CompressionCache
Add three methods and supporting helpers for token headroom mode: - compute_frozen_count: counts consecutive stable messages from start - apply_cached: swaps cached compressions into tool results (immutable) - update_from_result: learns new compressions from original/compressed pairs Supports both Anthropic and OpenAI tool result formats. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
chopratejas
|
ee1bcc93bd |
feat: add CompressionCache with LRU eviction for token headroom mode
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |