mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
8 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
chopratejas
|
00902b8fea |
fix: B7 — CCR hardening: persistent backends + always-on tool
P2-25, P2-26: CCR (Compress-Cache-Retrieve) used an in-memory store
that fragmented across uvicorn workers and was wiped on restart, and
the `headroom_retrieve` tool was registered/unregistered per-request
based on whether the latest body happened to contain compression
markers — every flip busted the prompt cache. Both are sticky
side-channels: once a session has done CCR, the tool list bytes and
the retrieval store must stay stable. This PR fixes both.
Rust:
* Split `ccr.rs` into `ccr/` with `backends/` submodule
(`in_memory.rs`, `sqlite.rs`, `redis.rs` cfg-gated).
* `SqliteCcrStore` (production default): WAL mode, prepared upsert,
lazy TTL purge on read, persistent across worker restarts and
shareable across workers on the same host via SQLite file locking.
* `RedisCcrStore` (cfg-gated behind `feature = "redis"`): SETEX with
startup PING smoke-test, no key-prefix collision risk, no sticky
session required at the LB.
* `CcrBackendConfig::{InMemory, Sqlite, Redis}` + `from_config(...)`
factory — every init failure surfaces (no silent fallback per
`feedback_no_silent_fallbacks.md`).
* `ccr::compute_key` (BLAKE3 → first 24 hex chars) and
`ccr::marker_for("HASH") -> "<<ccr:HASH>>"` centralize the hash +
marker format; one definition for the live-zone dispatcher and the
Python regex (`headroom/ccr/tool_injection.py:211`).
* `compress_anthropic_live_zone_with_ccr` accepts
`Option<&dyn CcrStore>`. When wired, every accepted compression
puts the original bytes into the backend and appends `<<ccr:HASH>>`
to the compressed string. The token-validation gate runs on the
marker-augmented string so the `compressed_tokens >=
original_tokens` rejection stays honest.
Python:
* `SessionCcrTracker` + `apply_session_sticky_ccr_tool` mirror the
PR-A7 `SessionToolTracker` / `apply_session_sticky_memory_tools`
pattern: once a session has done CCR, every subsequent request
injects the recorded golden tool-definition bytes. Tool list bytes
are byte-stable across turns (snapshot test pins them).
* `headroom/ccr/tool_injection.py::inject_tool_definition` accepts a
new `session_has_done_ccr` kwarg per the PR-B7 spec change at line
302-328. The legacy per-request path stays intact for callers that
don't yet thread a session id (e.g. Google handler).
* Anthropic + OpenAI handlers route their CCR tool-list updates
through `apply_session_sticky_ccr_tool`, keyed off the existing
`session_tracker_store.compute_session_id(...)` plumbing.
Backend selection model: `CcrBackendConfig::Sqlite { path }` is the
production default — single host, persistent, multi-worker safe with
sticky session. `CcrBackendConfig::Redis { url }` is the multi-host
scale-out option — no stickiness needed. `InMemory` is for tests
and single-worker dev only. RUST_DEV.md "Multi-worker deployment —
CCR fragmentation" rewritten around this matrix.
Tests:
* `crates/headroom-core/tests/ccr_backends.rs` — 7 tests covering
SQLite round-trip, TTL purge, proxy-restart survival, cross-backend
byte-equal keys, `from_config` paths, and the no-redis-feature
loud-failure check (+ 2 redis tests gated behind the feature).
* `crates/headroom-core/tests/live_zone_ccr.rs` — confirms
`<<ccr:HASH>>` marker injection, store population, and
no-marker-when-no-store invariants end-to-end.
* `tests/test_ccr_tool_always_on.py` — 12 tests pinning the
always-on behaviour, session/provider isolation, LRU bound, no-
session-id fallback, and (per-acceptance-criterion) the byte-stable
tool-definition snapshot.
Per-PR-B7 plan: REALIGNMENT/04-phase-B-live-zone.md.
|
||
|
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
|
378d8a0f05 |
fix(rust): audit cleanup — DiffCompressor CCR leak, CCR TOCTOU race, clippy debt, dep dedup
Closes findings from the post-Phase-3g audit. Five surgical fixes plus telemetry-discoverability docs. PyO3 0.22 → 0.24 security upgrade is its own PR (issue #335). 1. DiffCompressor cache_key persistence (production bug) --------------------------------------------------------- Pre-fix: `RustDiffCompressor.compress()` minted a `cache_key`, embedded `[... hash=abc123]` in the wire marker, and returned without storing the original anywhere. Python ContentRouter then returned the compressed text with a dangling marker — every retrieval tool call from the LLM 404'd. Sibling compressors (LogCompressor, SearchCompressor) already had the right pattern: Rust mints the key, Python's `_persist_to_python_ccr` writes the original to the production `CompressionStore`. DiffCompressor was the asymmetric one. Fix: - Rust: add `DiffCompressor::compress_with_store(content, context, Option<&dyn CcrStore>)` mirroring siblings. Calls `store.put` when a key is minted; legacy `compress()` and `compress_with_stats()` delegate with `None` for parity. - Python: add `_persist_to_python_ccr` helper to `headroom/transforms/diff_compressor.py.compress()` mirroring `log_compressor.py` and `search_compressor.py`. - Pipeline `DiffOffload`: switch to `compress_with_store(Some(store))` and drop the post-hoc double-store hack that papered over this bug at the orchestrator boundary. 2. CCR store TOCTOU race in `get()` ----------------------------------- `InMemoryCcrStore::get()` checked TTL under a read lock, dropped the lock, then called `remove()`. Between drop and remove a concurrent `put()` of the same hash with fresh data could land — and our `remove` would then wipe that fresh entry. Under multi-worker proxy load this manifested as "I just stored it; why is it gone?" Fix: use `DashMap::remove_if`. Predicate runs under the shard write lock so check-and-remove is atomic. New regression test exercises a tight contention loop between writer and reader on the same key. 3. Pre-existing clippy debt in smart_crusher -------------------------------------------- - 3× `field_reassign_with_default` in `crusher.rs` test setup — switch to struct-update syntax `Config { field: x, ..Default }`. - `hash_array_for_ccr` was `#[cfg(test)]` but unused; deleted with a comment so a future test can reintroduce it as a one-liner. `cargo clippy --workspace --all-targets -- -D warnings` is now clean across the whole workspace; previous CI patches that allowed these warnings can be removed in a follow-up. 4. Tokenizers dependency dedup ------------------------------ `tokenizers 0.21` (direct dep) + `tokenizers 0.22` (transitive via fastembed) compiled twice into the binary. Bumped direct dep to `0.22` to align; API is compatible (verified by full tokenizer test suite). Saves compile time + binary bloat. 5. Telemetry-discoverability doc (no new code) ---------------------------------------------- The audit recommended a per-transform invocation counter to inform the next Python → Rust port. Discovered the infrastructure already exists at `/stats`: - `compressions_by_strategy` — invocation count per strategy - `pipeline_timing` — count + avg/max ms per transform name - `tokens_saved_by_strategy` — savings attribution Added a section to `RUST_DEV.md` showing the `curl + jq` recipes to read this data, with example output highlighting how to spot zero-invocation deferral candidates (e.g. `code_compressor`). Verification: workspace tests 734 + 14 + 5 + 4 + 6 + 5 + 2 + 2 + 3 + 4 + 2 + 2 + 1 = all green; cargo fmt clean; cargo clippy --all-targets clean; Python tests 185 pass; commitlint clean. |
||
|
chopratejas
|
12c2665531 |
feat(rust): signals trait module + KeywordDetector (Phase 3e.1)
Establish `crates/headroom-core/src/signals/` as a top-level module holding cross-cutting detection traits. Phase 3e.1 ports `error_detection.py` to a `LineImportanceDetector` trait + a `Tiered<T>` combinator + a single concrete `KeywordDetector` impl backed by aho-corasick. Three traits at three granularities are sketched (line / blob / item); only line-importance is implemented today. Two bug fixes from the Python source bake into both the Rust impl and the Python regex shim: 1. `ERROR_KEYWORDS` listed `timeout|abort|denied|rejected` but `ERROR_PATTERN` regex omitted them. Lines like `"Connection timeout"` were silently neutral despite the keyword being canonical. Both surfaces now flag them. 2. `SECURITY_KEYWORDS` carried `token`, which false-positived on every reference to LLM tokens (`input_tokens`, `tokens_saved`, ...) in our own product. Dropped from the security set. The Python `error_detection.py` shim now reflects keyword data out of Rust via `keyword_registry_snapshot()` and recompiles the legacy `re.Pattern` objects on the fly. Existing callers (text_compressor, search_compressor, intelligent_context) continue to import the same names with no source changes; caller migration to the trait API happens in their own port PRs. The trait architecture is the seam where a future ML detector slots in without touching `KeywordDetector` or any caller. The canonical extension is documented in `signals/README.md` as a classifier head on the existing `bge-small-en-v1.5` embedder loaded by `relevance::EmbeddingScorer` -- 384-dim -> 4-class softmax, ~1.5 KB head, ~1 ms inference, no extra model file. Two alternatives (distilled tinyBERT in ONNX, logistic regression on lexical features) are kept open in case BGE-head underfits. Per the no-silent-fallbacks rule: only `KeywordDetector` lands as a concrete impl. No NoOp, no MockDetector, no stub-ML -- those will arrive with their real implementations. Phase 3g (Compression Pipeline Formalization, issue #315) is queued as the cross-cutting follow-up that will make lossless-then-lossy- then-CCR ordering an explicit, observable architecture rather than implicit per-compressor logic. Trait shapes there will reuse the signals primitive landed in this PR. |
||
|
chopratejas
|
3f8de4e117 |
fix(smart_crusher): re-land orphaned audit close-out — CCR knob + scorer fail-loud
Re-lands two audit fixes that were marked "merged" on GitHub but never reached main: squash-merging the parent stack changed its commit SHA, which silently dropped the contents of the stacked PRs (#301, #305). Single PR this time — no stacking risk. What lands: 1. **`enable_ccr_marker` Rust gate** — new field on `SmartCrusherConfig` (default `true`). `crush_array` checks it before emitting the `<<ccr:HASH>>` marker text and the CCR store write. PyO3 surface + parity-fixture tolerance updated; recorded fixtures predate the field and inherit the `true` default. 2. **Python shim collapses both flags to the gate** — both `ccr_config.enabled=False` and `ccr_config.inject_retrieval_marker =False` now flip the Rust gate off. Storing a payload nothing in the prompt can reference is pointless, and storing under `enabled=False` would be a surprise side effect the user explicitly opted out of. 3. **Custom `scorer` / `relevance_config` fails loud** — replaces the prior WARNING-and-drop. Silently dropping a user-supplied scorer is a textbook silent fallback. `NotImplementedError` instead. Verified zero production callers pass these args; full plumbing arrives with Stage-3c.2's relevance-crate Python bridge. Tests: - 2 new Rust unit tests in `crusher.rs::tests` - 6 new Python tests in `test_smart_crusher_toin_attachment.py` (3 CCR marker-knob behaviors + 3 scorer fail-loud) - Removed `test_ccr_inject_marker_false_logs_warning` (the WARNING is gone now that the flag is honored) - `make ci-precheck` green; eval suite + observability tests run twice consecutively to verify no TOIN file pollution leaks into the regular+coverage double-run on Python 3.11 RUST_DEV.md audit table reflects both gaps closed. |
||
|
chopratejas
|
049ca9cab2 |
fix(smart_crusher): re-attach TOIN learning loop + audit known regressions
The Stage 3c.1b retirement of the Python SmartCrusher silently disconnected three subsystems. The audit on 2026-04-28 caught them; this commit fixes what's fixable today and labels the rest visibly. ## TOIN learning loop — fixed Before: `ContentRouter._record_to_toin` skipped SmartCrusher on the assumption SmartCrusher recorded its own TOIN events. The retired Python class did. The Rust port doesn't know about TOIN. Net result: the highest-traffic compression strategy stopped fueling the learning loop, silently. Fix: shim's `crush()` and `_smart_crush_content()` now call `toin.record_compression()` after a real compression. Filtered on `strategy != "passthrough"` because the Rust port flips `was_modified=True` from JSON whitespace re-canonicalization. Best effort: TOIN failures are logged at debug level and never break compression. Token estimates use `len(json) // 4` (the rule the retired Python used) because the router doesn't pass a tokenizer down to this layer and re-tokenizing here would dominate the recording cost. 7 tests in `tests/test_smart_crusher_toin_attachment.py`: - crush() records on real compression, doesn't on passthrough - structurally-similar inputs land on the same pattern - _smart_crush_content() records (legacy apply() path) - TOIN errors don't break compression - non-JSON input doesn't record - inject_retrieval_marker=False emits a WARNING ## CCR marker emission knob — labelled, not yet fixed `ccr_config.inject_retrieval_marker=False` is not honored — the Rust port emits `<<ccr:HASH N_rows_offloaded>>` markers in `dropped_summary` unconditionally. Today the production default has the flag True so no one is hitting the gap, but the silent-disconnect was a real regression. Shim now logs a WARNING when callers pass `False` so the mismatch is visible. Fix needs a Rust-side gate; tracked in `RUST_DEV.md`. ## Custom relevance scorer — labelled, not yet fixed `relevance_config` and `scorer` constructor args are accepted for source compatibility but the Rust default `HybridScorer` always runs. Shim was logging this at debug; bumped to WARNING. Tracked. ## RUST_DEV.md New "Known regressions in retired-Python components" section with a table per retired component. The intent is that this section gets updated whenever a regression closes (or a new one is found), so the duplicate-codebase tax stays visible instead of decaying into folklore. |
||
|
chopratejas
|
c2749c0fb6 |
docs(rust): lockfile + RUST_DEV.md for proxy CLI
Cargo.lock: pick up tokio-util added in the WS half-close fix. RUST_DEV.md: document how to run headroom-proxy in passthrough mode (listen + upstream flags, e2e test gate, env vars). |
||
|
chopratejas
|
0414cb70e4 |
feat(rust): scaffold workspace + parity harness (phase-0)
Bootstrap the Rust port of Headroom. Additive only — no existing Python code modified. Ships the workshop, not the widgets. Layout Cargo.toml (workspace) + rust-toolchain.toml crates/headroom-core — transform library, stub only crates/headroom-proxy — axum binary, /healthz only crates/headroom-py — PyO3 cdylib, exposes headroom._core.hello() crates/headroom-parity — Rust-vs-Python oracle harness + parity-run CLI Tooling Makefile: test, test-parity, bench, build-proxy, build-wheel, fmt, lint .github/workflows/rust.yml: test, wheels (linux/mac), audit, parity-nightly deny.toml for cargo-deny Parity corpus tests/parity/recorder.py + scripts/record_fixtures.py 125 recorded fixtures across 5 leaf transforms (ccr, tokenizer, log_compressor, diff_compressor, cache_aligner) Docs RUST_DEV.md — developer setup and workspace reference docs/spec/022-rust-migration.md — migration plan and stage breakdown .gitignore: whitelist scripts/record_fixtures.py; ignore target/ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |