mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
2126 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
GUOHAO LIU
|
3bcef2be37
|
fix(cache): extract tool_result content from list-of-blocks format (#2092)
## Description Closes #2053 Modern Claude Code sends `tool_result` content as a list of typed blocks (`[{"type": "text", "text": "..."}]`) instead of a plain string. `_extract_tool_result_content` and `_swap_tool_result_content` in `compression_cache.py` only handled the plain string case, so every tool_result was skipped before compression — zero savings for all `headroom wrap claude` users. Fix: extract text from list-of-blocks content in `_extract_tool_result_content`, and collapse the list to a single text block in `_swap_tool_result_content` when replacing with compressed content. ## Type of Change - [x] Bug fix (non-breaking) - [ ] New feature (non-breaking) - [ ] Breaking change - [ ] Documentation update ## Changes Made - `headroom/cache/compression_cache.py`: - Added `_extract_text_from_blocks()` helper to extract joined text from Anthropic list-of-blocks format - Updated `_extract_tool_result_content()` to handle list content in both Anthropic tool_result blocks and OpenAI role=tool messages - Updated `_swap_tool_result_content()` to collapse list-of-blocks to a single text block when replacing content (preserves structure, prevents multi-block join mismatch) - `tests/test_token_headroom_mode.py`: Added 12 tests covering Anthropic list-of-blocks, OpenAI list, mixed blocks, empty list, non-mutation, missing-text-block fallback, and non-tool messages ## Testing - [x] Existing tests pass - [x] New tests cover the fix - [x] PBT round-trip property verified (6 properties × 850+ random examples) - [x] Adversarial edge case tests pass (50 cases across 3 functions) ``` tests/test_token_headroom_mode.py ....................... 32 passed (0.35s) tests/test_transforms_content_router.py ................. 37 passed (1.08s) tests/test_backend_bugs.py ............................... 37 passed (4.86s) ``` ## Real Behavior Proof - Environment: headroom main (upstream/main at time of PR), uv-managed Python 3.12 - Exact command / steps: 1. `uv run python -m pytest tests/test_token_headroom_mode.py -x -q --no-header` 2. `uv run python /tmp/pbt_tool_result_content.py` (PBT round-trip, 200 examples each property) 3. `uv run python /tmp/adversarial_lens_security.py` (50 edge case checks) 4. Design scan: grep'd codebase for `isinstance(content, str)` near tool_result — 3 sibling functions (`_to_text`, `_block_text`) already handle list-of-blocks - Observed result: All 106 tests pass. PBT confirmed extract+swap round-trip invariant holds for 850+ random inputs. 50 adversarial edge cases (null/None/nested/unicode/100K chars/1000 blocks) produce no crashes or wrong output. - Not tested: live proxy with real Claude Code traffic (the ContentRouter already handles list content correctly since v0.32.0, confirmed by code audit) ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --------- Co-authored-by: lennney <lennney@users.noreply.github.com> Co-authored-by: JD Davis <mxjerrett@gmail.com> |
||
|
Rod Boev
|
e9000863fc
|
fix(kompress): fail-open wall-clock guard on single-cache-miss compression (#2114)
## Description The single-cache-miss branch in `ContentRouter` ran compression inline on the request path without its own wall-clock guard, so a cooperative stall waited for the full call even when `HEADROOM_COMPRESSION_DEADLINE_MS` was meant to fail open. This change adds a branch-level watchdog that returns `PASSTHROUGH` after the deadline, scoped only to the one-pending-task path and not the native GIL-hold root cause. Closes #2046 ## 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 - Added `_compression_deadline_seconds()` and a watchdog around the single-cache-miss inline compression branch in `ContentRouter`. - Returned the original content with `PASSTHROUGH` and logged a fail-open warning after the configured deadline, while preserving under-deadline and deadline-disabled behavior. - Added focused regressions for timeout, under-deadline output, and disabled-deadline behavior, then kept the wider deadline suite green. - Raised the locked production floors for `click` and `pillow` to clear the current `pip-audit` findings that now fail external PR merge snapshots. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py -q`) - [x] Linting passes (`uv run ruff check headroom/transforms/content_router.py tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text uv run pytest tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py -q ============================= test session starts ============================= platform win32 -- Python 3.12.13, pytest-9.0.3, pluggy-1.6.0 rootdir: D:\Repos\headroom-pr-2046-compression-freeze configfile: pyproject.toml plugins: anyio-4.12.1, langsmith-0.9.3, asyncio-1.3.0, cov-7.0.0 asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function collected 5 items tests\test_content_router_single_item_deadline.py ... [ 60%] tests\test_transforms\test_kompress_deadline.py .. [100%] ============================== 5 passed in 0.42s ============================== uv run ruff check headroom/transforms/content_router.py tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py All checks passed! uv run ruff format headroom/transforms/content_router.py tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py --check 3 files already formatted ``` ## Real Behavior Proof - Environment: Windows, Python 3.12.13, router-level harness with a cooperative slow-compression stub - Exact command / steps: `uv run pytest tests/test_content_router_single_item_deadline.py tests/test_transforms/test_kompress_deadline.py -q`, which forces one frozen prefix and one cache miss, then sleeps past a 10 ms deadline - Observed result: the guarded branch returns the original content through `PASSTHROUGH` at the deadline, while under-deadline and deadline-disabled behavior stay unchanged - Not tested: native GIL-holding freeze ## 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 ## Screenshots (if applicable) Not applicable. ## Additional Notes - Security CI currently flags `click 8.3.1` and `pillow 12.2.0`, so this PR carries the narrow floor bump to `click>=8.3.3` and `pillow>=12.3.0` as a supply-chain unblock for the same final merge snapshot. - `CHANGELOG.md` remains untouched because Headroom generates release notes from conventional commits. - This PR is a Python-side mitigation for the single-cache-miss branch only; the native GIL-hold root cause remains a separate owner. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Ingmar Krusch
|
0ad7dc7c1c
|
fix(backends/litellm): preserve tool_result cache_control, complete streaming cache stats (#2144)
## Description Two gaps remain in Bedrock Converse prompt caching after [#1390](https://github.com/headroomlabs-ai/headroom/pull/1390) (currently open, not yet merged), which preserves `cache_control` on the system prompt and plain text blocks: 1. `_convert_messages_for_litellm` still drops `cache_control` on `tool_result` blocks during Anthropic-to-OpenAI conversion. #1390's own test (`test_tool_result_blocks_unaffected`) documents this as explicitly out of scope. In practice this is the case that matters most: in agent loops the moving cache breakpoint (what Claude Code marks with `cache_control: {type: ephemeral}`) lands on the tail `tool_result` message far more often than on the system prompt, so caching degraded to system-only even with #1390 applied. 2. `stream_message` never requests `stream_options.include_usage`, so LiteLLM/Bedrock never returns a usage chunk over SSE and `cache_read_input_tokens`/`cache_creation_input_tokens` always read 0 downstream, even when the Bedrock prompt cache is genuinely engaged server-side. The `message_start` event emitted before streaming begins is necessarily sent before any usage is known (hardcoded `input_tokens: 0`, no cache fields) — this PR captures the real values from the trailing usage chunk, carries them on the terminal `message_delta.usage`, and updates `StreamingMixin._stream_response_bedrock` to record those fields while preserving the normal Anthropic stream event order. I raised the streaming cache-stats gap directly in the [#1390 comment thread](https://github.com/headroomlabs-ai/headroom/pull/1390#issuecomment-4845691613); another reviewer (`dspv`) independently flagged the tool_result gap in the same thread with measurements matching what I found on this deployment. This PR is the follow-up with the actual diff and tests, scoped to only what #1390 doesn't cover. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/backends/litellm.py`: - `_convert_messages_for_litellm`: the `tool_result` → `role: "tool"` conversion now carries `cache_control` from the source block onto the emitted message when present. - `stream_message`: sets `kwargs["stream_options"] = {"include_usage": True}` before calling `acompletion`; captures `prompt_tokens`/`cache_read_input_tokens`/`cache_creation_input_tokens` from the final usage-bearing chunk in the streaming loop; after the loop, carries them on the terminal `message_delta.usage` when present (omitting cache keys entirely when their value is 0, to match the existing "no cache fields" contract elsewhere in this file). - `tests/test_bedrock_tool_result_cache_and_streaming_stats.py` (new): 8 tests covering `tool_result` cache_control preservation (present, absent, multiple blocks, non-Bedrock provider) and the streaming cache-stats completion (`stream_options` requested, terminal `message_delta.usage` carries real values, no extra `message_start` is emitted, zero-valued cache fields omitted). - `CHANGELOG.md`: added a `### Fixed` entry under `Unreleased`, cross-referencing #1390. ## 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 $ uv run pytest tests/test_backend_bugs.py tests/test_backend_anyllm.py tests/test_bedrock_region.py \ tests/test_bedrock_streaming_input_tokens.py tests/test_backend_streaming_cache_metrics.py \ tests/test_backend_nonstreaming_cache_metrics.py tests/test_litellm_nonstream_cache_usage.py \ tests/test_litellm_callback.py tests/test_bedrock_tool_result_cache_and_streaming_stats.py -q ============================= test session starts ============================== platform darwin -- Python 3.13.13, pytest-9.0.3, pluggy-1.6.0 collected 125 items tests/test_backend_bugs.py ..................................... [ 29%] tests/test_backend_anyllm.py ............... [ 41%] tests/test_bedrock_region.py ........................................... [ 76%] tests/test_bedrock_streaming_input_tokens.py .. [ 77%] tests/test_backend_streaming_cache_metrics.py .... [ 80%] tests/test_backend_nonstreaming_cache_metrics.py .... [ 84%] tests/test_litellm_nonstream_cache_usage.py ..... [ 88%] tests/test_litellm_callback.py ....... [ 93%] tests/test_bedrock_tool_result_cache_and_streaming_stats.py ........ [100%] ======================== 125 passed, 1 warning in 4.80s ======================== $ uv run ruff check headroom/backends/litellm.py tests/test_bedrock_tool_result_cache_and_streaming_stats.py All checks passed! $ uv run mypy headroom/backends/litellm.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - **Environment:** personal fork deployed as a real proxy (macOS launchd service, `headroom install apply`) with `--backend bedrock --mode cache --code-aware --bedrock-profile sso-bedrock`, fronting a live multi-turn Claude Code agent session against Bedrock (us-east-1). - **Exact command / steps:** ran an 8-turn replayed Claude Code agent session (system prompt + tool_result-heavy history, matching Claude Code's real traffic shape) through this deployment, with and without the tool_result cache_control fix applied, and inspected the proxy's PERF log lines and `/stats` output for `cache_hit_pct`/`cache_read`/`cache_write`. - **Observed result:** with only #1390's system-prompt/text-block preservation (no tool_result fix), billed token-equivalents were `input + 1.25*write + 0.1*read` = 292,859 (-17% vs. no caching at all) — caching engaged but only on the system prompt, since the tail tool_result's cache_control was still being dropped. With this PR's tool_result fix added, the same session billed 116,569 token-equivalents (-67%), matching direct-Bedrock parity. Separately, before the streaming-stats fix, the proxy's own PERF log lines showed `cache_read=0 cache_write=0 cache_hit_pct=0` on every request in this session despite the frozen-prefix mechanism confirming caching was active (`frozen_message_count` in the hundreds); after the fix, the same PERF lines report real nonzero cache_read/cache_write values matching the billing evidence above. - **Not tested:** live verification was done under `--mode cache`; the streaming-stats half of this fix is mode-agnostic (it's about what `stream_message` requests from LiteLLM/Bedrock and how it surfaces the result, independent of Headroom's own prefix-freeze bookkeeping), but I have not separately re-verified it live under `--mode token`. ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — backend logic change, no UI surface. ## Additional Notes - "I have made corresponding changes to the documentation" is unchecked: no file in `docs/`, `README.md`, or `CONTRIBUTING.md` documents the Bedrock Converse cache_control/streaming-usage mechanics this PR touches, so there is no existing section to update. - Overlap with #1390: that PR is still open as of this writing. This PR is based on current `upstream/main`, not on #1390's branch, and touches only the `tool_result` branch of `_convert_messages_for_litellm` (a different code path from #1390's text-block/system-prompt branch) plus `stream_message`'s usage handling, which #1390 does not touch at all. If #1390 merges first, this PR should apply cleanly since the two only share the same function, not the same lines. - No linked issue number: found via independent investigation of a personal deployment (measuring real token billing impact), and via participating in the #1390 review thread, not filed as a `headroomlabs-ai/headroom` issue first. --------- Co-authored-by: Ingmar Krusch <ingmar.krusch@gmail.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Tejas Chopra
|
58d3445d23
|
ci: scope native/wheel/dashboard jobs to relevant paths (unstarve the queue) (#2155)
## Problem CI's `changes` paths-filter is too coarse: both `code` and `e2e` include the catch-all `headroom/**`, so **any** Python change fires the full matrix — including the scarce-runner hogs: `build-wheel-windows`, `macos-native-wrapper`, `windows-native-wrapper`, `docker-native-e2e`. macOS runners cap at 5 on the Free plan, so a trivial Python PR burns the scarcest resource. `init-e2e`/`wrap-e2e` trigger on `headroom/**` too. ## Change (CI config only — no product code) - **ci.yml:** split `changes` into `code` / `native` / `dashboard` / `packaging`. Re-gate: | job | before | after | |-----|--------|-------| | `build-wheel-windows` | code | `packaging` | | `test-dashboard-ui` | code | `dashboard` | | `docker-native-e2e` | e2e | `native` | | `windows-native-wrapper` | e2e | `native` | | `macos-native-wrapper` | e2e | `native` | - **init-e2e / wrap-e2e:** scoped off `headroom/**` onto the `cli`/`install`/`providers`/`rtk` subpaths the flows exercise (mirrors the existing `*-native-e2e` workflows). ## Safety - Verified no job `needs:` any of the re-gated jobs, so no dependency chain breaks. `test-dashboard-ui` still gets `build-wheel` (dashboard files are under `headroom/` ⟹ `code` too). - Job-level `if:` skips resolve as **neutral** (not failing), so this is safe even if branch protection is added. - Full matrix still runs on any `.github/workflows/**` change and on push to `main` (post-merge safety net). ## Net A pure-Python logic PR skips ~7-9 jobs incl. all 3 native platform builds — freeing the macOS/Windows caps that were starving the queue. |
||
|
Abhay Singh
|
b0afee85b3
|
fix(memory): size HNSW index_batch resize off the id high-water mark (#2139)
## Description `HNSWVectorIndex.index_batch()` used the live memory map size to decide whether to resize hnswlib before adding new labels. hnswlib does not reclaim capacity slots when labels are removed with `mark_deleted`, so after delete/evict churn the live count can be much lower than the assigned-id high-water mark. That lets a batch add skip resizing and then fail in `add_items` with `number of elements exceeds the specified limit`. ## Fix - Size the batch resize check from `self._next_hnsw_id`, which has already been incremented for the new batch labels. - Match the single-item `index()` path's high-water-mark capacity behavior. - Add a regression test that deletes most entries from a small index and then batch-adds enough new memories to require a resize. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/memory/adapters/hnsw.py tests/test_memory/test_hnsw_batch_capacity.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/memory/adapters/hnsw.py tests/test_memory/test_hnsw_batch_capacity.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_memory/test_hnsw_batch_capacity.py -q 1 passed, 18 warnings ``` ## Review Readiness - [x] Ready for review - [x] Regression test added - [x] CHANGELOG updated Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
iotron
|
5cece7bf58
|
fix(proxy): Strip Codex responses-lite marker from response.create frame body (#1820)
## Description Codex CLI mirrors the `X-OpenAI-Internal-Codex-Responses-Lite` request header into the `response.create` WS frame body under `client_metadata.ws_request_header_x_openai_internal_codex_responses_lite`. The existing fix strips the header itself (both the HTTP fallback path and the WS handshake path) but never touched this frame-body copy, so Headroom still forwarded it to `wss://chatgpt.com/backend-api/codex/responses` unmodified. Upstream rejects `gpt-5.x` models whenever that field is truthy, so every Codex-through-Headroom turn on gpt-5.5/5.4 failed with a 400, even on builds that already contain the header-strip fix. Closes #1523 ## 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 - `headroom/proxy/handlers/openai.py`: add `_strip_codex_lite_metadata()`, which removes `client_metadata.ws_request_header_x_openai_internal_codex_responses_lite` from a `response.create` frame body (checks both the flat-body shape and the `{"response": {...}}` envelope shape Codex uses depending on call path). Fail-safe no-op on non-JSON payloads or when the key is absent. - Wired the helper into both WS-forwarder send sites in the same file: the initial `first_msg_raw` send and the steady-state `_client_to_upstream` relay send. - `tests/test_openai_codex_ws_lifecycle.py`: added `test_ws_first_frame_strips_codex_lite_metadata_mirror`, which sends a `response.create` frame carrying the mirror key through the handler and asserts the frame actually forwarded to the fake upstream has the key removed while sibling `client_metadata` fields (e.g. `thread_id`) survive. ## 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 $ ruff check headroom/proxy/handlers/openai.py tests/test_openai_codex_ws_lifecycle.py All checks passed! $ mypy headroom/proxy/handlers/openai.py Success: no issues found (pre-existing unrelated notes in headroom/proxy/server.py only) $ pytest tests/test_openai_codex_ws_lifecycle.py tests/test_openai_codex_routing.py tests/test_codex_openai_contract_parity.py -q 52 passed, 23 warnings in 1.66s # Proof the new test actually catches the bug (checked out the parent commit's # openai.py, i.e. pre-fix, with the new test present): $ pytest tests/test_openai_codex_ws_lifecycle.py -q -k strips_codex_lite_metadata FAILED tests/test_openai_codex_ws_lifecycle.py::test_ws_first_frame_strips_codex_lite_metadata_mirror AssertionError: assert 'ws_request_header_x_openai_internal_codex_responses_lite' not in {'thread_id': 't_1', 'ws_request_header_x_openai_internal_codex_responses_lite': True} 1 failed, 26 deselected in 0.63s # Restored the fix -> same test passes (see "52 passed" run above). ``` ## Real Behavior Proof - Environment: macOS, Python 3.13, headroom-ai 0.30.0 (pip-installed copy patched identically to this diff), Codex CLI 0.142.5, ChatGPT Plus subscription auth, model `gpt-5.5`. - Exact command / steps: `headroom wrap codex` (proxy on 127.0.0.1:8787) → `codex exec "..."` with `model_provider = "headroom"` in `~/.codex/config.toml`. Also reproduced deterministically via a throwaway debug proxy instance with `HEADROOM_CODEX_WIRE_DEBUG=1` wire capture, isolated from the live account/session. - Observed result: pre-patch, deterministic `400 unsupported_value` / "This model is not supported when using X-OpenAI-Internal-Codex-Responses-Lite" on every single turn (reproduced repeatedly, across a header-only-stripped build). Post-patch (installed copy with this exact diff): clean full completions streamed to `response.completed` with zero error frames via wire capture, and — after restarting the live production proxy to load the patched module — the user confirmed `codex` working normally end-to-end through Headroom on `gpt-5.5` in real day-to-day use, not just the isolated repro. - Not tested: the HTTP (non-WS) fallback path for Codex — that path doesn't carry a `response.create` frame body in the same way, and the existing header-strip logic already covers it; no upstream 400s observed there in this investigation. ## 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 ## Screenshots (if applicable) N/A — backend WS proxy fix, no UI surface. ## Additional Notes - CHANGELOG.md / docs left untouched: this is a narrowly-scoped bugfix to existing (undocumented-at-user-level) WS forwarding internals; happy to add a CHANGELOG entry if maintainers want one. - Full root-cause writeup with wire-capture details is on the issue: https://github.com/headroomlabs-ai/headroom/issues/1523#issuecomment-4887989873 - Did not run the full repo-wide `pytest`/`mypy headroom` (whole package) — scoped to the modified file and the three most relevant existing test modules (`test_openai_codex_ws_lifecycle.py`, `test_openai_codex_routing.py`, `test_codex_openai_contract_parity.py`), all passing. Glad to run the full suite if a maintainer wants that in CI instead. Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
ec97443e66
|
fix(ccr): detect read_lifecycle stale/superseded markers in the injector (#2148)
## Description A stale-read CCR marker is handed to the model with no tool to redeem it, so the original file bytes are silently lost. `read_lifecycle` emits, for a stale/superseded read: ``` [Read content stale: app.py was modified after this read — re-read the file for current content. Retrieve original: hash=<24-hex>] ``` and stores the original-at-read-time bytes in the CCR store under that hash, so `headroom_retrieve` *would* resolve it. But `CCRToolInjector._marker_patterns` never matches this marker — every pattern requires the word "compressed" (`[N type compressed to M. Retrieve more: hash=…]`, `[N type compressed. hash=…]`, the generic `\[.*?compressed.*?hash=…\]`) or the `<<ccr:` form. The stale marker says "stale/modified/superseded" and uses the phrase **`Retrieve original: hash=`**, which no pattern recognizes. Why that's a data-loss bug: on a frozen-prefix turn, `should_inject_ccr_tool` only re-injects `headroom_retrieve` when there's detected compressed content (`injector.has_compressed_content`). Since the stale marker isn't detected, the tool isn't injected, and the model is left a marker advertising `Retrieve original: hash=X` with no tool to redeem it. For a stale read, retrieval is the *only* way to recover the original bytes (re-reading yields current, different content) — so it's silently lost. This is exactly the "unredeemable marker" case the #1006 guard exists to prevent. Both `read_lifecycle` and prefix freezing are on by default, so this is reachable in ordinary long agentic sessions. The sibling `read_maturation` marker has the identical recovery contract and *is* detected — only because its text happens to contain "compressed" and ends in `]`, so the generic pattern catches it. That inconsistency is the tell. ## Fix Add a marker pattern that matches the load-bearing `Retrieve original: hash=<hash>` phrase (12–24 hex), so `read_lifecycle` markers are detected and the retrieve tool is injected. No other pattern or behavior changes. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/ccr/tool_injection.py`: add a `Retrieve original: hash=([a-f0-9]{12,24})` pattern to `CCRToolInjector._marker_patterns`. - `tests/test_ccr_tool_injection.py`: add `test_scan_detects_read_lifecycle_stale_marker` — a stale marker's hash is detected and `has_compressed_content` is true. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/ccr/tool_injection.py tests/test_ccr_tool_injection.py All checks passed! $ python -m py_compile headroom/ccr/tool_injection.py tests/test_ccr_tool_injection.py OK ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the pattern matching with a dependency-free script that runs the four existing patterns plus the new one against a real read_lifecycle marker, and left the full pytest to CI. - Exact command / steps: built the stale marker with a 24-hex hash and ran all four existing `_marker_patterns` and the new pattern against it, plus a normal `compressed` marker as a control. - Observed result: all four existing patterns return no match for the stale marker; the new pattern extracts the hash; the normal `compressed` marker is still matched by the existing pattern (unchanged). The new test asserts the injector detects the stale marker's hash and reports `has_compressed_content`. - Not tested: the full frozen-prefix inject path end to end; full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The "unit tests pass locally" and "type checking" boxes are unchecked because the full suite imports the ML stack, which I can't run in this environment; the change adds one regex to the existing pattern list (its single capture group is picked up by `_scan_text`'s last-group extraction), verified by the standalone proof and the new test. Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
9e38905a7d
|
fix(memory): apply turn_id scope filter even without agent_id (#2130)
## Description `SQLiteMemoryStore._build_query_conditions()` dropped `turn_id` when a query specified `session_id` and `turn_id` without also specifying `agent_id`. That made a single-turn scope return every memory in the session, and `count()` uses the same helper. ## Fix - Apply `agent_id` and `turn_id` as independent narrowing predicates inside the `session_id` branch. - Preserve the existing `agent_id`-only behavior. - Add direct query-condition regression tests for turn-only, agent+turn, and agent-only scopes. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.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_memory/test_query_conditions.py -q 3 passed ``` ## Review Readiness - [x] Ready for review - [x] Regression tests added - [x] CHANGELOG updated Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
5e0f1a219f
|
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 <hex>..<hex>` 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 <mxjerrett@gmail.com> |
||
|
Abhay Singh
|
dbb4e4cf48
|
fix(proxy/anthropic): cache response under the looked-up messages (#327) (#2124)
## Description The Anthropic response cache stores each entry under a different key than it is looked up by whenever the request pipeline rewrites `messages`, so the cache never hits and fills with unreachable entries. The handler snapshots the scalar cache-key fields once, before upstream, specifically to avoid post-mutation key drift: ```python # Snapshot cache-key fields from the request body ONCE here ... The pipeline # may mutate body before the response is cached, so re-reading there would # compute a different key and the cache would never hit (#327). cache_key_fields = {"system": body.get("system"), "tools": body.get("tools"), ...} ... cached = await self.cache.get(messages, model, **cache_key_fields) # get ... await self.cache.set(messages, model, response.content, ..., **cache_key_fields) # set ``` But `messages` — the primary key component (the cache key is a content hash of `{model, messages, **fields}`) — is passed **live** at both sites, and it is reassigned between them by: - the enterprise security scan (`messages, _security_ctx = self.security.scan_request(messages, ...)`), - the `pre_compress` hook (`messages = self.config.hooks.pre_compress(messages, ...)`), - image compression (`messages = await ...compress(messages, ...)`). So when any of those paths fires, `cache.set` stores the response under the *post-mutation* messages while every future `cache.get` computes the key from the *raw inbound* messages. The keys never match: the response cache is effectively write-only and each stored entry is unreachable until it is evicted. When none of the three paths mutates `messages`, the keys coincide and the cache works, which is why this went unnoticed. The scalar fields were snapshotted for exactly this reason (#327); `messages` was the one key component left live. ## Fix Snapshot the lookup messages alongside `cache_key_fields` (before any mutation) and pass that snapshot to `cache.set`, so the entry is stored under the same key it was looked up by. `messages` is only ever reassigned (to new objects) after the snapshot, so the reference stays the raw inbound list. No signature change. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/handlers/anthropic.py`: snapshot `cache_lookup_messages = messages` in the pre-upstream key-snapshot block and use it (not the live `messages`) at `cache.set`. - `tests/test_anthropic_pre_upstream_backpressure.py`: reuse the `_DummyAnthropicHandler` harness with a recording fake cache and a security scanner that rewrites `messages`; assert the messages passed to `cache.set` equal those passed to `cache.get` (and are the raw lookup messages, not the rewrite). - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [x] Unit tests pass (`uv run --extra dev pytest tests/test_anthropic_pre_upstream_backpressure.py::test_response_cache_keys_on_lookup_messages_not_mutated -q`) - [x] Linting passes (`uvx ruff@0.15.17 check headroom/proxy/handlers/anthropic.py tests/test_anthropic_pre_upstream_backpressure.py headroom/memory/factory.py`) - [x] Type checking passes (`uvx mypy==1.20.2 headroom/memory/factory.py`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/proxy/handlers/anthropic.py tests/test_anthropic_pre_upstream_backpressure.py All checks passed! $ python -m py_compile headroom/proxy/handlers/anthropic.py tests/test_anthropic_pre_upstream_backpressure.py OK ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing `headroom` pulls in the torch/transformers stack and running the handler test locally OOM-kills this box, so I verified the key logic with a dependency-free script that replicates the content-hash key from `semantic_cache_key.py`, and left the full pytest (including the new handler test) to CI. - Exact command / steps: computed the cache key for the raw inbound messages (the `get` key), then computed the `set` key from the live (mutated) messages versus the raw snapshot. - Observed result: with the mutated messages the set key differs from the get key (cache never hits); with the raw snapshot the set key equals the get key. The new handler test drives a request through a security scanner that rewrites `messages` and asserts `cache.set` receives the same messages as `cache.get`. - Not tested: a live end-to-end get-hit across two identical requests through the real cache; full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes Merged current `main` to pick up the repository-wide mypy cache-key annotation fix, then verified the focused regression locally. the fix snapshots one variable and swaps it at the set site, verified by the key-computation proof and a new regression test that reuses the file's existing, proven `handle_anthropic_messages` harness (injecting the cache and scanner on the instance, so the shared harness is untouched). Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
JD Davis
|
09be107d06
|
fix(deps): raise transformers security floor
Raise the production transformers floor to a version fixed for CVE-2026-5241 and refresh uv.lock so pip-audit passes. |
||
|
Abhishek Mittal
|
52a024d28c
|
fix(proxy): strip [1m] model suffix before upstream forwarding (#2027)
## Description Scopes the `[1m]` context-window tier suffix sanitizer to Anthropic `/v1/messages` requests only (addresses PR #2027 review feedback). The original patch applied the rewrite to every buffered compressible endpoint, which would have silently mutated OpenAI Chat Completions and OpenAI Responses request model IDs. The `[1m]` marker is an Anthropic/Claude Code compatibility signal emitted by the Headroom CLI; the existing Python parity behavior (`sanitize_anthropic_model_id()`) is Anthropic-specific and must not leak onto OpenAI shapes. Refactors the helper into `compression::sanitize_anthropic_model_id_in_body`, drops the dead `sanitize_model_id` helper in `sse/anthropic.rs`, and adds 8 unit tests + 5 wiremock-backed integration tests that pin the scope. All 420 `headroom-proxy` tests pass; `cargo fmt` and `cargo clippy -D warnings` clean. ## 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 - Move `sanitize_request_model_id` out of `proxy.rs` and into `compression::sanitize_anthropic_model_id_in_body` (Anthropic-specific name; private `trim_anthropic_model_id_suffix` helper for unit-testable pure behavior). - Gate the call site on `CompressibleEndpoint::AnthropicMessages` **after** classification. The OpenAI Chat Completions and OpenAI Responses arms get an explicit no-op match so the sanitizer cannot re-apply to those paths. - Drop the dead `sanitize_model_id` helper in `sse/anthropic.rs` (it was `#[allow(dead_code)]` with no callers). - 8 new unit tests in `compression/mod.rs`: trailing `[1m]` stripped, Claude-style suffix stripped, no-suffix passthrough (byte-equal), non-string model, missing `model` field, non-JSON body, `[1m]` mid-string, and the pure trim helper. - 5 new integration tests in `tests/integration_anthropic_model_sanitize.rs` that boot a real Rust proxy in front of a wiremock upstream. ## Testing - [x] Unit tests pass (`cargo test -p headroom-proxy` → 420 passed, 35 suites) - [x] Linting passes (`cargo clippy -p headroom-proxy --tests --all-features -- -D warnings` clean) - [x] Type checking passes (`cargo check -p headroom-proxy --tests --all-features` clean) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ cargo test -p headroom-proxy --test integration_anthropic_model_sanitize Compiling headroom-proxy v0.x.x Finished `test` profile [unoptimized + debuginfo] target(s) Running tests/integration_anthropic_model_sanitize.rs test anthropic_messages_strips_1m_suffix_glm ... ok test anthropic_messages_strips_1m_suffix_claude ... ok test anthropic_messages_passthrough_when_no_suffix ... ok test openai_chat_completions_passthrough_with_1m_model ... ok test openai_responses_passthrough_with_1m_model ... ok test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ``` ```text $ cargo test -p headroom-proxy test result: ok. 420 passed; 0 failed; 0 ignored; 0 measured; 235 filtered out finished in 10.93s ``` ```text $ cargo clippy -p headroom-proxy --tests --all-features -- -D warnings Finished `dev` profile [unoptimized + debuginfo] target(s) ``` ## Real Behavior Proof - **Environment:** macOS 14.x; `rustc` pinned via `rust-toolchain.toml`; `cargo` 1.x. No network access required (wiremock upstream). - **Exact command / steps:** 1. `cargo test -p headroom-proxy --test integration_anthropic_model_sanitize` — confirms `/v1/messages` strips `glm-5.2[1m]` and `claude-3-7-sonnet[1m]`; confirms `/v1/chat/completions` and `/v1/responses` leave the body byte-equal (SHA-256 asserted). 2. `cargo test -p headroom-proxy` — full suite green (420 passed). 3. `cargo clippy -p headroom-proxy --tests --all-features -- -D warnings` — clean. 4. `cargo fmt -p headroom-proxy --check` — clean. 5. Source inspection of `crates/headroom-proxy/src/proxy.rs` after the change: the call site is now in a `match endpoint` arm that explicitly returns `buffered` for the OpenAI variants, so the sanitizer cannot re-apply to those paths. - **Observed result:** all 5 new integration tests pass, all 420 crate tests pass, clippy and fmt clean. The OpenAI tests assert SHA-256 byte equality on a body whose `model` field ends in `[1m]`; if the sanitizer were to re-leak onto OpenAI shapes these would fail loudly with a length delta. - **Not tested:** a live Anthropic API call (would require real credentials and is not required to prove the byte-level scope fix). The Python proxy's `sanitize_anthropic_model_id()` is the documented parity reference (Python PR #1840, issue #1812). ## 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 (N/A — no user-facing docs change; the Python proxy's `sanitize_anthropic_model_id` is the parity reference cited in code comments) - [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 (project uses git log + PR titles; this PR's title follows the conventional commit shape) ## Screenshots (if applicable) N/A — backend behavior, no UI change. ## Additional Notes - The OpenAI integration tests rely on a JWT-style `Authorization: Bearer` header to classify the request as `AuthMode::OAuth` and short-circuit the PR-E4 `prompt_cache_key` injector. This is the same control variable the existing `integration_chat_completions.rs` tests use to isolate dispatcher byte-fidelity from the E4 hook. Comments in each test explain the relationship. - The dead helper in `sse/anthropic.rs` is removed, so the diff is net negative on LoC for the SSE module. - The Python parity reference is `sanitize_anthropic_model_id()` (Python PR #1840, issue #1812); the function name and the call-site scope are the explicit parity contract. - Branch was rebased onto `upstream/main` (91 commits behind) before force-push to the fork; conflict-free rebase. The original PR commit and the fix are the only two commits on the PR. --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> Co-authored-by: Abhishek Mittal <abhishek.mittal@users.noreply.github.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
thejesh23
|
35701ce809
|
fix(shared_context): don't evict an unrelated entry on an update at capacity (#2136)
Fixes #2135. ## Summary `SharedContext.put` ran `_evict_if_needed` before writing, and the eviction loop only checked `len(self._entries) >= self._max_entries`. When a caller updated a key that was already cached at capacity, the put would not have grown the map — but the loop still dropped the oldest unrelated entry. Same defect class as fixed for `SemanticCache` in #2094: the eviction path must know the incoming key so an update is not treated as an insert. This mirrors that fix over to `SharedContext`. Threads the incoming key through `_evict_if_needed` and skips capacity eviction when it names an entry that already exists. Expired-entry cleanup still runs unconditionally. Issue #2135 has the reproduction and impact writeup. ## Test plan - [x] `uv run pytest tests/test_shared_context.py` — 16 passed (added `test_updating_existing_key_at_capacity_does_not_evict`). - [x] `uv run ruff check headroom/shared_context.py tests/test_shared_context.py` — clean. - [x] `uv run ruff format --check headroom/shared_context.py tests/test_shared_context.py` — already formatted. ## Real behavior proof **Setup:** macOS 25.4 (Darwin arm64), Python 3.12.13, `uv 0.11.28`, this branch (`fix/shared-context-evict-on-update`). **Before the patch (unpatched `main`)** \`\`\` before update: ['a', 'b', 'c'] after update: ['b', 'c'] # <-- 'a' evicted, even though 'c' was an update \`\`\` **After the patch (this branch)** \`\`\` \$ uv run python <<'PY' from headroom.shared_context import SharedContext ctx = SharedContext(ttl=3600, max_entries=3) ctx.put(\"a\", \"x\"*400) ctx.put(\"b\", \"x\"*400) ctx.put(\"c\", \"x\"*400) print(\"before update:\", sorted(ctx.keys())) ctx.put(\"c\", \"y\"*400) # update existing at capacity print(\"after update: \", sorted(ctx.keys())) print(\"c value:\", ctx.get(\"c\", full=True)[:12] + \"...\") PY before update: ['a', 'b', 'c'] after update: ['a', 'b', 'c'] c value: yyyyyyyyyyyy... \`\`\` **Test output** \`\`\` \$ uv run pytest tests/test_shared_context.py -q ................ [100%] 16 passed in 2.17s \`\`\` **What I did NOT test** - Multi-thread test — the fix is inside the existing `self._lock`, so serialization semantics are unchanged; I did not add a concurrent-put stress test. - Interaction with TTL expiry AND capacity in one call — the existing `test_evicts_oldest_at_capacity` and `test_expired_entry_returns_none` still pass, but I did not add a combined case. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
thejesh23
|
ecdcf13f3f
|
fix(compress): don't mutate the caller's CompressConfig via kwargs (#2134)
## Description `compress(messages, config=my_cfg, protect_recent=0, target_ratio=0.2)` used to write those kwarg values onto the caller's `my_cfg` object — so a shared per-agent `CompressConfig` was silently rewritten every time a call passed a single override. The next call that did NOT override that field then saw the previous request's value instead of the original default. Copy the config once at entry with `dataclasses.replace` before applying kwarg overrides (and before the savings-profile pass, which also mutates in place). Existing behavior for callers that pass **only** kwargs, or **only** a config, is unchanged. Issue #2133 has the root-cause walkthrough. Closes #2133 ## 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 - `headroom/compress.py`: copy the incoming `CompressConfig` once at entry with `dataclasses.replace` before applying kwarg overrides, so the caller's object is no longer mutated. The savings-profile branch already did a defensive `replace(cfg)`; that copy is now hoisted up front so both the kwarg and profile paths share the same guarantee. - `tests/test_compress_api.py`: added `test_kwargs_do_not_mutate_caller_config`, which fails on unpatched `main` and passes on this branch, covering the previously broken kwarg leg. - `CHANGELOG.md`: noted the fix. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_compress_api.py -q ................. [100%] 17 passed in 2.88s $ uv run ruff check headroom/compress.py tests/test_compress_api.py All checks passed! $ uv run ruff format --check headroom/compress.py tests/test_compress_api.py 2 files already formatted ``` ## Real Behavior Proof - Environment: macOS 25.4 (Darwin arm64), Python 3.12.13, `uv 0.11.28`, branch `fix/compress-mutates-caller-config`, model `claude-sonnet-4-5-20250929` used for token counting. - Exact command / steps: build `c = CompressConfig(protect_recent=4, target_ratio=0.8)`, call `compress(msgs, model="claude-sonnet-4-5-20250929", config=c, protect_recent=0, target_ratio=0.2)` on a 3000-char user message, then read `c.protect_recent` and `c.target_ratio` back (full snippet run via `uv run python <<'PY' ... PY` — see the code block below). - Observed result: before the patch, `c.protect_recent` became `0` and `c.target_ratio` became `0.2` (caller's config silently rewritten). After the patch, `c.protect_recent` stays `4` and `c.target_ratio` stays `0.8`; caller's config unchanged. `uv run pytest tests/test_compress_api.py` reports 17 passed including the new `test_kwargs_do_not_mutate_caller_config` case. - Not tested: end-to-end proxy path with `savings_profile` set (the pre-fix code already did a defensive `replace(cfg)` on that branch, so the profile leg was safe; this change hoists that copy up front and the added unit test covers the kwarg leg that was broken — I did not spin up the proxy to reconfirm the profile branch end-to-end). No concurrent-caller / threading regression test was added — the fix removes the mutation entirely which sidesteps the race, but there is no explicit multi-thread reproducer. ### Reproducer **Before the patch (unpatched `main`)** ```text before: protect_recent=4, target_ratio=0.8 after : protect_recent=0, target_ratio=0.2 # <-- caller's cfg silently rewritten caller's config MUTATED ``` **After the patch (this branch)** ```text $ uv run python <<'PY' from headroom.compress import compress, CompressConfig c = CompressConfig(protect_recent=4, target_ratio=0.8) print(f"before: protect_recent={c.protect_recent}, target_ratio={c.target_ratio}") msgs = [{"role":"user","content":"x"*3000}] compress(msgs, model="claude-sonnet-4-5-20250929", config=c, protect_recent=0, target_ratio=0.2) print(f"after : protect_recent={c.protect_recent}, target_ratio={c.target_ratio}") print("caller's config", "unchanged" if (c.protect_recent, c.target_ratio) == (4, 0.8) else "MUTATED") PY before: protect_recent=4, target_ratio=0.8 after : protect_recent=4, target_ratio=0.8 caller's config unchanged ``` ## 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: no user-facing doc covers the CompressConfig / kwargs contract; see Additional Notes --> - [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 - **Documentation checklist item** — left unchecked as N/A. The behavior being fixed is internal to `headroom.compress.compress()`; the mutation contract of `CompressConfig` + kwargs is not covered in any user-facing doc (`wiki/compression.md`, `wiki/text-compression.md`, `wiki/image-compression.md`, and `docs/content/docs/shared-context.mdx` document a different / higher-level API surface). The `CHANGELOG.md` entry is the appropriate place for this fix. - **`mypy headroom` checklist item** — left unchecked because I did not run it in this workflow; the change is a two-line refactor within a well-typed function and no signatures moved. - The prior body's `## Summary`, `## Test plan`, and `## Real behavior proof` sections were reorganized into the six template-required headings so the PR-governance check passes. All technical content (root-cause, before/after reproducer, and test output) is preserved above; no code changes were made in this update. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
thejesh23
|
af7385a298
|
fix(paths): reject '.', '..', and NUL as plugin names (#2132)
Fixes #2131. ## Description `plugin_config_dir` / `plugin_workspace_dir` rejected `/` and `\` in the plugin name but accepted `.` and `..`. Since the returned path is `<root> / "plugins" / name`, `plugin_config_dir("..")` resolved to the whole config root and `plugin_workspace_dir("..")` to the whole workspace root: savings ledger, memory DB, license cache, logs, and every other plugin's state. That defeated the sandbox the helper was written to enforce. Both callers are folded onto a shared `_validate_plugin_name` that rejects the empty string, both path separators, `.`, `..`, and NUL. Closes #2131 ## 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 - `headroom/paths.py`: added `_validate_plugin_name` and shared it across `plugin_config_dir` and `plugin_workspace_dir`. - `tests/test_paths.py`: expanded invalid-name coverage for `.`, `..`, and NUL and added a sandbox-escape regression test. - `CHANGELOG.md`: noted the path traversal fix. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_paths.py -q ........................................................................... [100%] 79 passed in 0.14s $ uv run ruff check headroom/paths.py tests/test_paths.py All checks passed! $ uv run ruff format --check headroom/paths.py tests/test_paths.py 2 files already formatted ``` ## Real Behavior Proof - Environment: macOS 25.4 (Darwin arm64), Python 3.12.13, `uv 0.11.28`, branch `fix/plugin-path-traversal`. - Exact command / steps: set `HEADROOM_CONFIG_DIR=/tmp/hc` and `HEADROOM_WORKSPACE_DIR=/tmp/hw`, then call `plugin_config_dir("..")`, `plugin_config_dir(".")`, and `plugin_config_dir("legit-plugin")`. - Observed result: before the patch, `plugin_config_dir("..")` resolved to `/private/tmp/hc`, escaping the plugin sandbox. After the patch, `plugin_config_dir("..")` and `plugin_config_dir(".")` raise `ValueError`; a normal plugin name resolves under `/private/tmp/hc/plugins/legit-plugin`. - Not tested: Windows behavior and plugin-registry integration. The added check is a pure string validation at the path-helper layer. ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A. ## Additional Notes - Documentation is not updated because this is a helper-level sandbox fix rather than a user-facing behavior change; the changelog entry captures it. - `mypy headroom` was not run in the author's workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Ingmar Krusch
|
3c1a5cdb1c
|
fix(backends/litellm): drop oversized tool names before Bedrock Converse (#2129)
## Description The Bedrock Converse API hard-rejects any request containing a tool name over 64 characters (`toolConfig.tools.N.member.toolSpec.name`). Claude Code includes every globally-added claude.ai MCP connector tool in every request it sends, even connectors the user hasn't enabled locally. One org-wide connector with a 65-char tool name is enough to fail every single request routed through this backend's Bedrock path, with no way to remove or disable the connector client-side. Direct Bedrock mode (`CLAUDE_CODE_USE_BEDROCK=1`, bypassing this proxy) is unaffected: it hits Bedrock's native Anthropic-compatible endpoint, which has no such length limit. Only the Converse API, which this LiteLLM-backed `bedrock` provider path uses, enforces it. ## 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 - `headroom/backends/litellm.py`: `send_message` and `stream_message` both filter tools with names over 64 characters out of the payload before converting/forwarding, but only for `self.provider == "bedrock"`. Other providers are untouched. - `tests/test_backend_bugs.py`: new `TestBedrockOversizedToolNameFiltering` covering both `send_message` and `stream_message` — an oversized (65-char) name is dropped on `bedrock`, a name at exactly the 64-char boundary is kept, and non-`bedrock` providers forward oversized names unfiltered (the limit is a Bedrock Converse constraint, not a general one). - `CHANGELOG.md`: added a `### Fixed` entry under `Unreleased`. ## 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 $ uv run pytest tests/test_backend_bugs.py tests/test_backend_anyllm.py -q ============================= test session starts ============================== platform darwin -- Python 3.13.13, pytest-9.0.3, pluggy-1.6.0 collected 57 items tests/test_backend_bugs.py .......................................... [ 73%] tests/test_backend_anyllm.py ............... [100%] ============================== 57 passed in 1.42s ============================== $ uv run ruff check headroom/backends/litellm.py tests/test_backend_bugs.py All checks passed! $ uv run mypy headroom/backends/litellm.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - **Environment:** personal fork deployed as a real proxy (macOS launchd service, `headroom install apply`) with `--backend bedrock --mode token --code-aware --bedrock-profile sso-bedrock`, fronting a live Claude Code session with a globally-added-but-not-locally-enabled claude.ai MCP connector (`TopCounsel`) whose tool name is 65 characters. - **Exact command / steps:** run any Claude Code request through this deployment while the org-wide `TopCounsel` connector is present (it is included in the tool list on every request regardless of local enablement). - **Observed result:** before the fix, every request failed with a LiteLLM `BedrockException`: `1 validation error detected: Value 'mcp__claude_ai_TopCounsel_by_The_L_Suite__complete_authentication' at 'toolConfig.tools.N.member.toolSpec.name' failed to satisfy constraint: Member must have length less than or equal to 64`. After applying the fix (filtering the oversized tool out before the LiteLLM call), the same session proceeds normally with no validation error, confirmed live against this deployment. - **Not tested:** truncating the name instead of dropping it was tried and discarded during investigation — the model echoes the truncated name back in `tool_use` blocks, and Claude Code matches tool calls by the original full name, so truncation breaks routing on the return path. This PR drops the tool entirely rather than truncating, which is why it is not present as an alternative in the diff. ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — backend logic change, no UI surface. ## Additional Notes - "I have made corresponding changes to the documentation" is unchecked: no file in `docs/`, `README.md`, or `CONTRIBUTING.md` documents backend-specific tool-list filtering behavior, so there is no existing section to update. - No linked issue number: this was found via independent investigation of a personal deployment (a live Bedrock validation failure), not filed as a `headroomlabs-ai/headroom` issue first. Checked `gh pr list`/`gh issue list` for existing coverage of "Bedrock Converse 64-char tool name" and found none open or merged. - A native Bedrock Anthropic-compatible endpoint backend (avoiding Converse's tool-name limit entirely) would be the more complete long-term fix, but is out of scope for this PR. --------- Co-authored-by: Ingmar Krusch <ingmar.krusch@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Tejas Chopra
|
2976d49f18
|
fix(proxy): preserve sub-path in X-Headroom-Base-Url custom upstream (#2037) (#2127)
## Description `_resolve_openai_upstream_base` ran the `X-Headroom-Base-Url` value through `_normalize_origin`, which strips the path. A custom OpenAI-compatible upstream served from a sub-path, such as `https://host/api/v1`, was routed to the bare origin and returned `proxy_error` (#2037). This re-attaches the path after origin normalization. This is a clean extraction of the path fix from #2047, which bundled it with an unrelated `supports_websockets = true` to `false` default change across init/wrap/codex. #2047 can be closed in favor of this narrower fix. Closes #2037 ## 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 - `headroom/proxy/handlers/openai.py`: re-attach the request header path component in `_resolve_openai_upstream_base` after origin normalization. - `tests/test_proxy/test_openai_upstream_header.py`: assert sub-paths are preserved and trailing slashes are normalized. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ pytest tests/test_proxy/test_openai_upstream_header.py 5 passed $ ruff check headroom/proxy/handlers/openai.py tests/test_proxy/test_openai_upstream_header.py All checks passed! ``` ## Real Behavior Proof - Environment: local proxy header resolution path, custom OpenAI-compatible upstream configured via `X-Headroom-Base-Url`. - Exact command / steps: resolve `X-Headroom-Base-Url: https://gateway.example/api/v1` through `_resolve_openai_upstream_base` / `_resolve_openai_upstream`. - Observed result: before the fix, the upstream resolved to `https://gateway.example` and lost `/api/v1`, causing the proxy to route to the wrong endpoint. After the fix, it resolves to `https://gateway.example/api/v1`; a trailing slash is normalized away. - Not tested: end-to-end request against a live third-party OpenAI-compatible gateway. The regression is covered at the proxy routing helper layer where the path was dropped. ## 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 ## Screenshots (if applicable) N/A. ## Additional Notes - Documentation is not updated because this fixes the existing header behavior rather than changing a documented user-facing contract. - Changelog is not updated in this PR; the change is scoped to the regression and test. - `mypy headroom` was not run in the author's workflow. |
||
|
牧濑红莉栖（BOT）
|
09d1ef45be
|
fix(proxy): compress Hermes scoped coding-agent passthrough (#1815)
## Description Compress Hermes Studio scoped coding-agent passthrough requests in the generic OpenAI passthrough handler. Hermes can route scoped Claude Code and Codex traffic through Headroom while preserving its own proxy paths; this PR keeps Hermes responsible for scoped proxy authentication/provider adaptation while still applying Headroom compression to supported chat payloads before forwarding. The compression remains narrow-scoped: - Only chat messages with `user` or `assistant` roles are compressed. - Tool, function, reasoning, and system items are preserved byte-stable. - Non-dict items in the Responses `input` array are preserved and spliced back. ## 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 - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Detect `/api/codex-proxy/.../v1/responses` paths and compress supported Responses `input` chat items before forwarding. - Detect `/api/claude-code-proxy/.../v1/messages` paths and compress supported Anthropic `messages` payloads before forwarding. - Preserve bypass, malformed payload, missing-model, tool/function, reasoning/system, and non-dict passthrough behavior. - Add regression coverage in `tests/test_hermes_passthrough_compression.py`. ## Testing - [x] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_hermes_passthrough_compression.py -v test_codex_proxy_preserves_tool_and_function_items PASSED test_codex_proxy_preserves_nondict_items PASSED test_codex_proxy_bypass_header_skips_compression PASSED test_codex_proxy_malformed_input_preserved PASSED test_codex_proxy_compression_applies_to_chat_messages PASSED test_claude_proxy_preserves_tool_use_items PASSED test_claude_proxy_bypass_header_skips_compression PASSED test_claude_proxy_no_model_forwarded_unchanged PASSED test_claude_proxy_compression_applies_to_chat_messages PASSED test_non_hermes_routes_not_affected PASSED ``` ## Real Behavior Proof - Environment: Author-reported local test environment for `headroom/proxy/handlers/openai.py` and `tests/test_hermes_passthrough_compression.py`. - Exact command / steps: `python -m pytest tests/test_hermes_passthrough_compression.py -v`. - Observed result: The 10 Hermes passthrough regression tests passed, covering Codex and Claude scoped proxy routes plus preservation/bypass cases. - Not tested: End-to-end Hermes Studio traffic against a live upstream service is not covered by this PR body evidence. ## 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 - [ ] 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 ## Screenshots (if applicable) N/A. ## Additional Notes Generated with Claude Code. The unchecked checklist items are not required for this narrow proxy-handler test change. --------- Co-authored-by: x1051445024 <你的GitHub注册邮箱> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
GUOHAO LIU
|
8870b6971f
|
fix(deps): bump pillow to 12.3.0 and click to 8.4.2 (#2097)
## Description
Pip-audit found 6 vulnerabilities in 2 packages in the lockfile:
| Package | From | To | Vulns Fixed |
|---------|------|----|-------------|
| click | 8.3.1 | 8.4.2 | PYSEC-2026-2132 |
| pillow | 12.2.0 | 12.3.0 | PYSEC-2026-2253~2257 |
Closes #N/A (no issue filed — security workflow failure)
## Type of Change
- [x] Bug fix (non-breaking)
- [ ] New feature (non-breaking)
- [ ] Breaking change
- [ ] Documentation update
## Changes Made
- `uv.lock`: Upgraded click from 8.3.1 to 8.4.2, pillow from 12.2.0 to
12.3.0
## Testing
- [x] `uv lock --upgrade-package` resolved cleanly
- [x] `ruff check headroom/` passes
- [x] CLI import verified (click 8.4.2 loads correctly)
```
$ uv run python -c "import click; print(click.__version__)"
click: 8.4.2
$ uv tree --depth=1 | grep -E "click|pillow"
click v8.4.2
pillow v12.3.0 (extra: all)
pillow v12.3.0 (extra: image)
```
## Real Behavior Proof
- Environment: headroom main (upstream/main
|
||
|
Tejas Chopra
|
22af75adae
|
fix(memory): annotate _EMBEDDER_CACHE key as 3-tuple (unbreak main lint) (#2153)
## What One-line type fix: `_EMBEDDER_CACHE` is keyed by a 3-tuple `(backend, model, ollama_base_url)` but was still annotated `dict[tuple[str, str], Embedder]`. ## Why `mypy headroom --ignore-missing-imports` (the CI `lint` job) fails on `main` at `factory.py:187`/`:219` because of this mismatch. Since the lint job runs whole-package mypy, **every open PR is currently failing lint on this bug** — none of them introduced it. This unblocks the lint gate repo-wide. ## Proof `mypy headroom --ignore-missing-imports` → `Success: no issues found in 469 source files`. `ruff check .` + `ruff format --check .` clean. ## Scope Type annotation only; no runtime behavior change. The 3-tuple key itself is pre-existing (the Ollama base_url was already part of the key to avoid cross-server embedder cache collisions). |
||
|
Tejas Chopra
|
908a9a1bb1
|
fix(cache): stop DynamicContentDetector false positives corrupting cached prompts (#2110) (#2119)
## Description `DynamicContentDetector` / `RegexDetector` in `headroom/cache/dynamic_detector.py` (used by the `cache_aligner` transform) misclassified ordinary English words and code identifiers (e.g. `in_pr`) as "dynamic content," extracting them from the system prompt and re-appending a `[Dynamic Context]` tail that grows unboundedly and corrupts the cached prompt over a session. Fix tightens detection to require genuinely-dynamic shapes (timestamps, UUIDs, hashes, numbers-with-units, ISO dates) rather than bare tokens — no hardcoded wordlist — and bounds the tail. `cache_aligner` is off by default, so blast radius is limited, but the detector logic is now correct. Closes #2110 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/cache/dynamic_detector.py`: raise the evidence bar so ordinary words/identifiers aren't extracted; bound the dynamic tail. - `tests/test_cache/test_dynamic_detector.py`: assert false positives (ordinary words/identifiers) are NOT extracted while real dynamic values still are. ## Testing - [x] Unit tests pass (`pytest tests/test_cache/test_dynamic_detector.py`) - [x] Linting passes (`ruff check`) ### Test Output ```text 55 passed, 2 skipped ruff: All checks passed! ``` ## Real Behavior Proof - Before: identifiers like `in_pr` extracted into a growing `[Dynamic Context]` tail, corrupting cached prompts. - After: ordinary tokens stay in place; only genuinely-dynamic values are detected. |
||
|
Tejas Chopra
|
c5545d6ac4
|
fix(wrap): use canonical headroom-openclaw npm package for wrap openclaw (#1969) (#2120)
## Description `headroom wrap openclaw` installed a non-existent npm spec — the `--plugin-spec` default was `headroom-ai/openclaw`, which npm reads as a GitHub shorthand and fails; the published package is `headroom-openclaw` (see `plugins/openclaw/package.json`). Fix introduces a single `OPENCLAW_NPM_PACKAGE = "headroom-openclaw"` constant (kept in sync with `package.json` and the release env), uses it as the default, and defers writing the `plugins.entries.headroom` config until after a successful install so a hard failure leaves no stale entry. Closes #1969 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/providers/openclaw/wrap.py` + `__init__.py`: canonical `OPENCLAW_NPM_PACKAGE` constant. - `headroom/cli/wrap.py`: use it as `--plugin-spec` default; write config only after successful install. - `tests/test_cli/test_wrap_openclaw.py`: expect `headroom-openclaw`; install-before-config ordering; failed-install-writes-no-config test. ## Testing - [x] Unit tests pass (`pytest tests/test_cli/test_wrap_openclaw.py`) — 29 passed - [x] Linting passes (`ruff check`) ### Test Output ```text 29 passed ruff: All checks passed! ``` ## Real Behavior Proof - Before: `wrap openclaw` → npm "unsupported spec" error; a failed install left a stale config entry. - After: installs `headroom-openclaw`; no config written on failure. |
||
|
Tejas Chopra
|
dbe2558c18
|
fix(proxy): aggregate tool-output size floor so Codex sessions compress (#2050) (#2116)
## Description Wrapping Codex yielded **0% compression**: the OpenAI Responses path extracts each `function_call_output` as its own `CompressionUnit` with a 512B per-item floor, so a session of many small tool outputs floored every unit (reporter telemetry: 381 units, all `size_floor`/`passthrough`, `tokens_saved=0`). The Anthropic path compresses the whole message list in one batch and isn't subject to a per-item floor. Fix: evaluate the size floor once against the **aggregate** compressible bytes of the extracted group (matching the batch path). Disable the per-unit floor when the group clears the threshold; keep it when the whole group is below it. Generic across any OpenAI-Responses caller — no `codex` special-casing, reuses the existing `OPENAI_RESPONSES_ROUTER_MIN_BYTES`. Closes #2050 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/proxy/handlers/openai.py`: aggregate-then-floor for Responses tool-output units. - `tests/test_openai_responses_compression_units.py`: regression tests (aggregate-clears → compresses all; aggregate-below → skips all). ## Testing - [x] Linting passes (`ruff check .`) - [x] New tests added for the fix ### Test Output ```text $ ruff check headroom/proxy/handlers/openai.py tests/test_openai_responses_compression_units.py All checks passed! ``` ## Real Behavior Proof - Environment: static fix; verified locally via `ruff` + `py_compile` (native `_core` isn't built in the review worktree, so the pytest suite runs in CI). - Before: Codex session floored every tool-output unit → 0 tokens saved. - After: units whose aggregate exceeds the floor reach the ContentRouter and compress; trivially-small groups still skip. - Not tested locally: live Codex end-to-end token-saved delta (needs a real Codex session); CI unit tests cover the floor/routing logic. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review |
||
|
Rod Boev
|
38479fcda1
|
fix(codex): rerun memory lookup on every response.create WS frame (#2113)
## Description Long-lived Codex `/v1/responses` WebSocket sessions only ran memory decision, query construction, and context injection on the first `response.create` frame because `handle_openai_responses_ws` kept that logic outside the relay loop. This change extracts that path into a local helper reused before compression for every eligible `response.create`, while keeping sticky memory tools deduplicated by the existing session helper. Closes #2059 ## 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 - Extracted the first-frame memory preparation path into a local async helper inside `handle_openai_responses_ws`. - Reused that helper for the initial frame and every later eligible `response.create` before compression and shaping. - Added focused two-turn WebSocket regressions for per-frame lookup, bypass and disabled-memory handling, list-shaped later inputs, memory-handler fail-open recovery, and sticky-tool replay. - Raised the locked production floors for `click` and `pillow` to clear the current `pip-audit` findings that now fail external PR merge snapshots. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py -q`) - [x] Linting passes (`uv run ruff check headroom/proxy/handlers/openai.py tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text uv run pytest tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py -q ============================= test session starts ============================= platform win32 -- Python 3.12.13, pytest-9.0.3, pluggy-1.6.0 rootdir: D:\Repos\headroom-pr-2059-codex-ws-memory-lookup configfile: pyproject.toml plugins: anyio-4.12.1, langsmith-0.9.3, asyncio-1.3.0, cov-7.0.0 asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function collected 35 items tests\test_codex_ws_per_frame_memory.py ......... [ 25%] tests\test_openai_codex_ws_lifecycle.py .......................... [100%] ============================= 35 passed in 1.89s ============================== uv run ruff check headroom/proxy/handlers/openai.py tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py All checks passed! uv run ruff format headroom/proxy/handlers/openai.py tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py --check 3 files already formatted ``` ## Real Behavior Proof - Environment: Windows, Python 3.12.13, in-process WebSocket harness with a fake memory handler - Exact command / steps: `uv run pytest tests/test_codex_ws_per_frame_memory.py tests/test_openai_codex_ws_lifecycle.py -q`, which opens one connection, sends distinct `response.create` frames, and records each memory lookup plus the forwarded tool set - Observed result: the handler performs one lookup per eligible frame, later-frame compression sees current-turn memory-prepared input, and both forwarded turns carry the same deduplicated `memory_search` and `memory_save` definitions - Not tested: live Codex subscription WebSocket ## 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 - Security CI currently flags `click 8.3.1` and `pillow 12.2.0`, so this PR carries the narrow floor bump to `click>=8.3.3` and `pillow>=12.3.0` as a supply-chain unblock for the same final merge snapshot. - `CHANGELOG.md` remains untouched because Headroom generates release notes from conventional commits. - This PR is scoped to the OpenAI Responses WebSocket relay lifecycle; it does not change the HTTP `/v1/responses` path or the Anthropic handler. --------- Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
GaryOcean
|
1448718fca
|
fix: strip output-only fallback blocks from request messages (#1870)
Anthropic's server-side refusal-fallback feature
(`server-side-fallback-2026-06-01`) can emit an output-only `{ "type":
"fallback", ... }` block inside an assistant response. That block is
valid on the response path but rejected on the request path, so when a
client replays the assistant turn the next request can 400 with an
invalid `fallback` input tag.
This strips output-only request blocks in the shared request-body
readers before forwarding. `read_request_json_with_bytes` re-encodes raw
bytes only when stripping occurs, so byte-faithful passthrough paths
cannot leak the invalid block while clean requests keep their original
bytes. If stripping empties an assistant content array, the helper
backfills a benign text block so the request remains schema-valid.
## Type of Change
- [x] Bug fix
## Changes Made
- Add `strip_output_only_request_blocks` in `headroom/proxy/helpers.py`.
- Strip fallback blocks in both `_read_request_json` and
`read_request_json_with_bytes`.
- Add regression tests for stripping, empty-turn backfill, raw-byte
re-encoding, and byte-identical clean requests.
- Update `CHANGELOG.md`.
## Testing
Focused verification on the current PR head after merging `main`:
```text
python -m pytest tests/test_output_only_request_blocks.py -q
4 passed
uvx ruff==0.15.17 check headroom/proxy/helpers.py tests/test_output_only_request_blocks.py
All checks passed!
uvx ruff==0.15.17 format --check headroom/proxy/helpers.py tests/test_output_only_request_blocks.py
2 files already formatted
```
## Notes
No live Anthropic request was run; the regression is covered at the
shared request-reader layer used by the proxy forwarding paths.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Ayush Kumar Jha
|
daeff69a75
|
fix(wrap): surface Claude Remote Control base-URL gate accurately (#1… (#1883)
…779) Claude Code 2.1.196 deterministically disables first-party Remote Control (/remote-control, /rc) behind a custom ANTHROPIC_BASE_URL, which Headroom always sets. Make the wrap/doctor warning accurate (state the disable as fact, name the /rc command, detect the installed version), suppress it for auth modes that never had RC (API key, Bedrock/Vertex/Foundry) and for builds older than 2.1.196, co-report the sibling #746/#1158 gates session-accurately, and fix is_custom_anthropic_base_url host handling (scheme-less hosts, malformed URLs). UX/notice-only; no request bytes touched. ## Description <!-- Briefly explain the change and why it is needed. --> Closes # ## Type of Change - [ ] 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 - ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text # Paste relevant command output or artifact links here ``` ## Real Behavior Proof - Environment: - Exact command / steps: - Observed result: - Not tested: ## Review Readiness - [ ] I have performed a self-review - [ ] This PR is ready for human review ## Checklist - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) Add screenshots to help explain your changes. ## Additional Notes <!-- Mention any N/A checklist items, tradeoffs, follow-ups, or maintainer context. --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Shubham Srivastava
|
c3db8e47f8
|
fix(install): default docker image to headroomlabs-ai GHCR registry (#1867) (#2039)
## Description
The GitHub repository was transferred from `chopratejas/headroom` to
`headroomlabs-ai/headroom`. GitHub 301-redirects transferred repos for
web and git operations, but **GitHub Container Registry (GHCR) does
not** — the old package `ghcr.io/chopratejas/headroom` is now orphaned
and frozen (its `latest` tag stopped advancing at `0.27.0`), while CI
publishes new images to `ghcr.io/headroomlabs-ai/headroom` (the workflow
derives the path from `${{ github.repository }}`).
Headroom's install tooling still defaulted to the dead path, so
`headroom install apply --preset persistent-docker`, `headroom init`,
and the standalone install scripts all pulled a stale `0.27.0` image
instead of the current release.
This changes every docker-image **default** to
`ghcr.io/headroomlabs-ai/headroom:latest`.
Closes #1867
## 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
- `headroom/install/models.py` — `InstallManifest.image` default.
- `headroom/cli/install.py` — `--image` Click option default.
- `headroom/cli/init.py` — two `InstallManifest(...)` image args.
- `scripts/install.sh` / `scripts/install.ps1` — `IMAGE_DEFAULT` /
`$ImageDefault` plus the `--image` help-text default.
- `docker/docker-compose.native.yml` — image default in both services.
- `tests/test_install/test_planner.py` (4) and
`tests/test_install/test_runtime.py` (5) — updated the assertions that
pinned the old image (including `assert "<image>" in command`), so they
now verify the corrected registry threads through the planner and docker
runtime command.
Scope note: `github.com/chopratejas/...` links and the plugin
marketplace slug are intentionally **not** changed — GitHub redirects
those, so they still work. Only the genuinely-dead GHCR image references
are touched. No new dependency, no new abstraction.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed
### Test Output
```text
$ uv run pytest tests/test_install/ -q
99 passed, 1 skipped in 48.34s
$ uv run ruff check headroom/install/models.py headroom/cli/install.py headroom/cli/init.py
All checks passed!
$ grep -rn "ghcr.io/chopratejas/headroom" headroom/ scripts/ docker/ tests/
# (no source matches — every default now points at headroomlabs-ai)
```
The install-test assertions pinned the old image, so they fail against
the old defaults and pass after the fix — they are the regression guard.
## Real Behavior Proof
- **Environment:** Windows 11, Python 3.13.5, headroom installed from
this branch (editable, via `uv`).
- **Exact command / steps:** The dead-registry claim is verifiable at
the registry level, independent of a release:
```text
docker pull ghcr.io/chopratejas/headroom:latest # old default → 0.27.0
(frozen / orphaned)
docker pull ghcr.io/headroomlabs-ai/headroom:latest # new default →
current release
```
And the install pipeline now emits the correct image (covered by
`tests/test_install/test_runtime.py`, which asserts the resolved docker
command contains `ghcr.io/headroomlabs-ai/headroom:latest`).
- **Observed result:** `grep` confirms no `ghcr.io/chopratejas/headroom`
default remains in code, scripts, compose, or tests;
`tests/test_install/` is green (99 passed) with the corrected image
asserted end-to-end through planner → runtime command.
- **Not tested:** A live `docker pull` of both tags on this specific
machine (no local Docker daemon guaranteed) — the registry difference is
reproducible by anyone running the two `docker pull` commands above; and
an end-to-end `headroom install apply --preset persistent-docker`
against a real Docker host.
## 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
- [x] I have updated the CHANGELOG.md if applicable
## Screenshots (if applicable)
N/A
## Additional Notes
- Documentation checklist item is N/A — no user-facing docs surface
beyond the CHANGELOG entry.
- `mypy` left unchecked — not run as part of this verification; the
change is a string-default swap with no type-level surface.
- The `--image` help text and `docker-compose.native.yml` were included
so every user-facing default is consistent; only the dead GHCR image
string was changed.
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Ingmar Krusch
|
3d0e59e518
|
fix(content-router): protect_tool_results must not be weakened by profile-derived read_protection_window (#2105)
## Description `ContentRouter.apply()` computes `read_protection_window` from `protect_recent_reads_fraction`, where `0.0` (the sentinel `--protect-tool-results` sets, per #1374's documented contract) means "protect all excluded-tool output regardless of conversation depth." The method then unconditionally overwrote that window with a per-request `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 regardless of `--protect-tool-results` — older excluded-tool output (`Read`, `Glob`, `Grep`, `Write`, `Edit` results) silently fell through to lossy Kompress compression. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/transforms/content_router.py`: the runtime `read_protection_window` kwarg may now only *narrow* the window when `self.config.protect_recent_reads_fraction > 0`. It can no longer override the `0.0` ("protect everything") sentinel that `--protect-tool-results` sets. - `tests/test_content_router_exclude_tools.py`: regression coverage that `--protect-tool-results`-equivalent config (`protect_recent_reads_fraction=0.0`) stays fully protected even when a savings-profile kwarg would otherwise shrink the window. - `tests/test_transforms/test_content_router.py`: unit coverage of the precedence logic itself (kwarg narrows when fraction > 0, kwarg is ignored when fraction == 0.0). - `CHANGELOG.md`: added an `### Bug Fixes` entry under `Unreleased`. ## 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 $ uv run pytest tests/test_content_router_exclude_tools.py tests/test_transforms/test_content_router.py -q ============================= test session starts ============================== platform darwin -- Python 3.13.13, pytest-9.0.3, pluggy-1.6.0 collected 64 items tests/test_content_router_exclude_tools.py ...... [ 9%] tests/test_transforms/test_content_router.py ........................... [ 51%] ............................... [100%] ============================== 64 passed in 2.77s ============================== $ uv run ruff check headroom/transforms/content_router.py tests/test_content_router_exclude_tools.py tests/test_transforms/test_content_router.py All checks passed! $ uv run mypy headroom/transforms/content_router.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - **Environment:** personal fork deployed as a real proxy (macOS launchd service, `headroom install apply`) with `--backend bedrock --mode token --code-aware --protect-tool-results Bash`, `HEADROOM_SAVINGS_PROFILE=coding` (library default `protect_recent=2`), fronting a live Claude Code session. - **Exact command / steps:** in a long-running Claude Code session against this deployment, `Read` a source file, continue the conversation past 2 more assistant turns (so the file's `Read` result ages past the profile's `protect_recent=2` window), then have the agent re-read or reference the same file. - **Observed result:** before the fix, the aged `Read` output for a plain (non-code) file came back as `[N items compressed to M. Retrieve more: hash=...]` despite `--protect-tool-results` being set and `Read` sitting in `DEFAULT_EXCLUDE_TOOLS` — confirmed by direct proxy log inspection (`content_router.py`'s override silently winning over the `0.0` sentinel) and by byte-diffing the installed pipx package against this same fork's git source to rule out a stale build. After applying the fix, the same sequence leaves the aged `Read` output intact (no compression marker) — verified via `pytest` regression tests plus a fresh live-session check post-deploy. - **Not tested:** this deployment has since switched to `--mode cache` (upstream's tested/benchmarked default for the `coding` profile as of `68676daa`), where the whole `read_protection_window` mechanism this bug lives in is structurally unreachable for anything inside the frozen prefix — so the precedence fix in this PR is primarily relevant to `token`-mode deployments (or any deployment where cache mode's frozen-prefix boundary hasn't yet advanced past the affected message). It has not been independently re-verified live under `--mode token` after the most recent rebase onto `main` (only the automated test suite was rerun post-rebase). ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — backend logic change, no UI surface. ## Additional Notes - "I have made corresponding changes to the documentation" is unchecked: no file in `docs/`, `README.md`, or `CONTRIBUTING.md` documents `read_protection_window`, `protect_recent_reads_fraction`, or `--protect-tool-results` precedence at all, so there was no existing section to update, and no new section was added either. This is arguably a pre-existing documentation gap this PR doesn't close. - No linked issue number: this was found via independent investigation of a personal deployment, not filed as a `headroomlabs-ai/headroom` issue first. Co-authored-by: Ingmar Krusch <ingmar.krusch@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Tejas Chopra
|
6efd01f707
|
ci: concurrency-cancel Docker + Merge Conflicts to stop merge-spree pileups (#2138)
## Description
`docker.yml` builds a full multi-arch image on **every push to `main`**
with **no concurrency group**, so a merge spree stacks one build per
commit. Against the Free-plan **20 concurrent-job cap**, those long
builds hog the runner pool and starve the `CI` (`test`/`lint`/`build`)
jobs that PR merges actually depend on. This adds concurrency-cancel so
only the latest build per ref runs — `cancel-in-progress` scoped to
`main` so a release tag's publish (its own ref) is never killed. Same
fix for the per-PR **Merge Conflicts** check.
## Type of Change
- [x] Repo infrastructure / CI (no functional change)
## Changes Made
- `.github/workflows/docker.yml`: `concurrency: docker-${{ github.ref
}}`, cancel-in-progress on `main` only.
- `.github/workflows/merge-conflicts.yml`: per-PR concurrency-cancel.
## Testing
- [x] YAML validated (`yaml.safe_load` on both) — release/tag builds
unaffected (separate ref group).
## Real Behavior Proof
- Before: 24 merges → 24 stacked Docker builds queued against 20 slots.
- After: only the latest `main` Docker build runs; older superseded ones
auto-cancel; release publishes untouched.
|
||
|
Abhay Singh
|
1725cd1f83
|
fix(memory): key the embedder cache on ollama_base_url (#2109)
## Description
The process-wide embedder cache can hand a caller an embedder bound to
the wrong Ollama server.
`_create_embedder` caches by `(backend, model)`:
```python
key = (
config.embedder_backend.value if hasattr(...) else str(...),
config.embedder_model or "",
)
```
But the Ollama branch constructs the embedder with the server URL:
```python
embedder = OllamaEmbedder(base_url=config.ollama_base_url, model_name=config.embedder_model)
```
So two configs in the same process that share a backend and model but
point at different Ollama servers (for example a per-project storage
router, or a fail-over host) collide on the same cache key. The first
call builds and caches an `OllamaEmbedder` bound to server A; the second
call, asking for server B, gets server A's embedder back and silently
embeds against the wrong host.
The code already reasoned about the analogous `openai_api_key` omission
and worked around it with an up-front validation guard (see the comment
above the key), but `ollama_base_url` has no such guard, so it just
resolves to the wrong server.
## Fix
Add `config.ollama_base_url` to the cache key. Same server still hits
the cache (one model load); a different server gets its own embedder.
Non-Ollama backends are unaffected (the URL just becomes an extra,
constant key component).
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/memory/factory.py`: include `config.ollama_base_url` in the
embedder cache key, with a comment explaining why.
- `tests/test_memory/test_factory_embedder_cache.py`: new file with
`test_ollama_embedder_cache_keys_on_base_url` (different servers get
different embedders) and
`test_ollama_embedder_cache_reuses_same_base_url` (same server still
caches). Kept out of `test_factory.py` because that module skips
wholesale without `hnswlib`, which these cases don't need.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/memory/factory.py tests/test_memory/test_factory_embedder_cache.py
All checks passed!
$ python -m py_compile headroom/memory/factory.py tests/test_memory/test_factory_embedder_cache.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the cache-key behavior with a
dependency-free script that models the `(backend, model)` vs `(backend,
model, base_url)` keys against a simulated cache, and left the full
pytest to CI.
- Exact command / steps: created two configs with the same backend and
model but `ollama_base_url` of `http://gpu1:11434` and
`http://gpu2:11434`, and resolved each through the old key and the new
key against a shared cache.
- Observed result: the old key serves the same embedder object for both,
and the config asking for `gpu2` is handed the `gpu1`-bound embedder;
the new key gives each config its own embedder bound to its own server.
The new tests assert distinct embedders with the right `_base_url` for
different servers, and cache reuse for the same server.
- Not tested: a live Ollama round-trip (`OllamaEmbedder` construction is
offline — it stores the URL and lazily creates its client); full local
`pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change adds one component to a cache-key tuple in a
pure function, verified by the standalone proof and the two new tests
for CI. The tests construct only the lightweight (offline) Ollama
embedder, so they don't need a running server or the vector-index deps.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
6979b5245e
|
fix(tokenizers): use o200k_base for gpt-4.1/gpt-4.5/o4 families (#2108)
## Description
`get_encoding_for_model` returns the wrong tiktoken encoding for the
current OpenAI flagship families, so their token counts are computed
with the wrong vocabulary.
The prefix table is ordered most-specific-first, but it has no entry for
the `gpt-4.1` / `gpt-4.5` / `o4` families:
```python
for prefix, encoding in (
("gpt-4o", "o200k_base"),
("gpt-4-turbo", "cl100k_base"),
("gpt-4", "cl100k_base"),
("gpt-3.5", "cl100k_base"),
("o1", "o200k_base"),
("o3", "o200k_base"),
):
if model.startswith(prefix):
return encoding
return DEFAULT_ENCODING # cl100k_base
```
- `gpt-4.1`, `gpt-4.1-mini`, `gpt-4.5-*` all start with `gpt-4`, so they
match the `gpt-4` prefix and get `cl100k_base`.
- `o4-mini` matches no prefix and falls through to the `cl100k_base`
default.
All three families use `o200k_base`. Since `count_text`/`count_messages`
tokenize with the resolved encoding, every token count for those models
is computed against the wrong BPE vocabulary, which skews budget gating
and the compress/skip decision for a large slice of current OpenAI
traffic.
## Fix
Add explicit `gpt-4.1` and `gpt-4.5` prefixes (ordered ahead of `gpt-4`,
which they would otherwise match) and an `o4` prefix, all mapping to
`o200k_base`. Plain `gpt-4` and `gpt-3.5` snapshots still resolve to
`cl100k_base`, and `gpt-4o` still wins for the 4o family.
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/tokenizers/tiktoken_counter.py`: add `gpt-4.1`/`gpt-4.5`
prefixes ahead of `gpt-4`, and an `o4` prefix, all mapping to
`o200k_base`.
- `tests/test_tokenizers.py`: add
`test_gpt41_and_o4_families_use_o200k`.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
All checks passed!
$ python -m py_compile headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the resolution with a
dependency-free script that runs the old and new prefix tables, and left
the full pytest to CI.
- Exact command / steps: resolved `gpt-4.1`, `gpt-4.1-mini`,
`gpt-4.5-preview`, and `o4-mini` under the old table and the new table,
plus `gpt-4o-*`, `gpt-4-2025-*`, `gpt-4-turbo-*`, `gpt-3.5-turbo`, and
`o1-mini` as regression guards.
- Observed result: old table returns `cl100k_base` for all four (wrong);
new table returns `o200k_base`; the guard models are unchanged
(`gpt-4o-*` and `o1-*` stay `o200k_base`,
`gpt-4*`/`gpt-4-turbo*`/`gpt-3.5*` stay `cl100k_base`). The new test
asserts the four families resolve to `o200k_base` and a plain `gpt-4`
snapshot stays `cl100k_base`.
- Not tested: loading the actual tiktoken vocabularies to count tokens
end to end; full local `pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change adds three ordered prefix entries to a pure
function, verified by the standalone proof and the new regression test
for CI. I intentionally left `gpt-5` out since I didn't want to assert
an encoding I couldn't confirm here; happy to add it in a follow-up if
you can confirm the intended mapping.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
eecb81e847
|
fix(cache/ccr): don't count a successful eviction as a retrieval (#2106)
## Description
The compression feedback learner treats a *successful* compression as
evidence that it should compress less, which inverts the learning
signal.
When `CompressionStore` evicts an entry that was never retrieved, it
emits a synthetic event to tell the learner the compression was fine
(the model never needed the original):
```python
success_event = RetrievalEvent(..., retrieval_type="eviction_success")
self._pending_feedback_events.append(success_event)
```
`process_pending_feedback` forwards every pending event to
`CompressionFeedback.record_retrieval` unconditionally. But
`record_retrieval` has no branch for `"eviction_success"` — and since
that string isn't `"full"`, it lands in the `else`:
```python
self._total_retrievals += 1
pattern.total_retrievals += 1
if event.retrieval_type == "full":
pattern.full_retrievals += 1
else:
pattern.search_retrievals += 1 # <-- eviction_success counted here
```
So a compression that worked is booked as a *search retrieval*, which
raises the tool's `retrieval_rate` and `search_rate`.
`get_compression_hints` reads a high retrieval rate as "we're
compressing too aggressively" and recommends larger `max_items` / lower
aggressiveness (or `skip_compression`). Net effect: the more often
compression succeeds, the more the learner backs off from compressing. A
standalone repro books a single successful eviction as a 100% retrieval
rate.
Every sibling consumer of the event distinguishes the type — telemetry
and TOIN both receive `retrieval_type="eviction_success"` and handle it
as its own thing. Only the local feedback counter ignores the
distinction.
## Fix
Recognize `"eviction_success"` in `record_retrieval` and leave it out of
the retrieval counters. The compression itself is already counted by
`record_compression` at store time, so an entry that is compressed and
never retrieved already yields a low retrieval rate — which is the
correct "compression worked" signal. Genuine `full`/`search` retrievals
are unchanged.
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/cache/compression_feedback.py`: early-return in
`record_retrieval` for `retrieval_type == "eviction_success"` so it is
not counted as a retrieval, with a comment explaining the signal.
- `tests/test_ccr_feedback.py`: add
`test_eviction_success_is_not_counted_as_retrieval` (asserts the
counters stay at zero after a successful eviction, and that a genuine
retrieval afterward still counts).
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/cache/compression_feedback.py tests/test_ccr_feedback.py
All checks passed!
$ python -m py_compile headroom/cache/compression_feedback.py tests/test_ccr_feedback.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the counting logic with a
dependency-free script that replicates
`record_compression`/`record_retrieval` and the
`retrieval_rate`/`search_rate` properties, and left the full pytest to
CI.
- Exact command / steps: recorded one compression, then a
`retrieval_type="eviction_success"` event, under the old counting (no
branch) and the new counting (early return), plus a genuine `search`
retrieval as a control.
- Observed result: old counting books the successful eviction as a
retrieval — `retrieval_rate=1.0`, `search_rate=1.0` — so the learner
would back off from compressing; new counting leaves
`retrieval_rate=0.0` and `total_retrievals=0`; a real retrieval
afterward still increments to 1. The new test asserts exactly this.
- Not tested: an end-to-end store-evict-then-hint cycle through
`CompressionStore.process_pending_feedback`; full local `pytest`
deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the fix is a single early-return in a pure counting method,
verified by the standalone proof and the new regression test (which
reuses the existing `test_ccr_feedback.py` pattern) for CI. Scope is
deliberately limited to the local feedback learner — telemetry and TOIN
already receive the `eviction_success` type and handle it separately.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
aa788164fd
|
fix(proxy/anthropic): preserve non-2xx upstream status through security scan (#2100)
## Description
When enterprise security scanning is enabled, the non-streaming
`/v1/messages` handler can turn a failed upstream response into an HTTP
200, so the client never sees the error.
After the upstream call, the handler parses the body unconditionally:
```python
resp_json = None
try:
resp_json = response.json()
except (json.JSONDecodeError, ValueError):
...
```
Every response-mutating block that follows gates on a successful
upstream — CCR handling (`... and response.status_code == 200 ...`), the
response cache (`if self.cache and response.status_code == 200`), and
the buffered-stream CCR block (`if buffered_stream_ccr and
response.status_code == 200 ...`). The enterprise-security block was the
exception:
```python
if self.security and _security_ctx and resp_json:
resp_json = self.security.scan_response(resp_json, _security_ctx)
response = httpx.Response(status_code=200, content=json.dumps(resp_json).encode(), headers=response_headers)
if not buffered_stream_ccr:
return Response(content=response.content, status_code=response.status_code, headers=response_headers)
```
No status check, and the rebuilt response hardcodes `status_code=200`.
`_retry_request` returns 429 (rate limit), 529 (overloaded), and other
4xx responses verbatim to this caller, so when security is configured
any of those — whose JSON error body parses fine — is rebuilt as an HTTP
200 and returned. The client sees success, so its retry/backoff logic
never fires on a rate limit or overload, exactly when it matters most.
## Fix
Gate the security block on a 200 upstream, the same condition the
sibling CCR/cache/buffered-stream blocks already use. A non-2xx response
falls through to the final `return Response(...,
status_code=response.status_code, ...)` and keeps its real status; a 200
is still scanned and returned as before.
```python
if self.security and _security_ctx and resp_json and response.status_code == 200:
...
```
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/handlers/anthropic.py`: gate the enterprise-security
response-scan branch on `response.status_code == 200`.
- `tests/test_anthropic_pre_upstream_backpressure.py`: reuse the
existing `_DummyAnthropicHandler` harness (adds optional `security` and
`upstream_status` params, both defaulting to today's behavior) and add
`test_security_scan_preserves_non_200_upstream_status` (429/529/400)
plus a 200 positive-control test.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/proxy/handlers/anthropic.py tests/test_anthropic_pre_upstream_backpressure.py
All checks passed!
$ python -m py_compile headroom/proxy/handlers/anthropic.py tests/test_anthropic_pre_upstream_backpressure.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and running the handler
test locally OOM-kills this box, so I verified the branch logic with a
dependency-free script that models the security block plus the
fallthrough return, and left the full pytest (including the new handler
test) to CI.
- Exact command / steps: modelled the non-streaming return path with
enterprise security configured, running a `429` upstream through the old
branch (no status gate, rebuilds 200) and the new branch (gated on 200,
falls through), plus a `200` upstream as a control. Also traced
`_retry_request` in `server.py` to confirm it returns 429/529/4xx
verbatim to this caller (only 5xx raises), so the branch is reachable
for those statuses.
- Observed result: old branch returns HTTP 200 for a 429 upstream
(laundered); new branch returns 429; a 200 upstream returns 200 under
both. The new parametrized handler test asserts the returned status
equals the upstream status for 429/529/400, and the control test asserts
a 200 upstream still returns 200.
- Not tested: a live enterprise-security plugin (`scan_response` is an
out-of-repo component; the test uses a passthrough stub); full local
`pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the fix is a one-condition gate mirroring three sibling
blocks in the same method, and the regression test reuses the file's
existing, proven `handle_anthropic_messages` harness (the added handler
params default to current behavior, so existing tests are unaffected).
The branch-logic proof and the new tests cover the fix for CI. The
security block only runs when an enterprise-security component is
configured; without it, this path is inert.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
c7b5a24b4f
|
fix(learn): don't shadow TIMEOUT/CONNECTION with the generic RUNTIME_ERROR (#2099)
## Description `classify_error` (used by `headroom learn` to categorize failed tool calls) miscategorizes timeouts and connection failures as generic runtime errors. The pattern list is checked in order, first match wins, and it puts the generic catch-all *before* the specific categories: ```python (re.compile(r"Traceback \(most recent|Exception:|Error:", re.I), ErrorCategory.RUNTIME_ERROR), (re.compile(r"timed? ?out|TimeoutError|deadline exceeded", re.I), ErrorCategory.TIMEOUT), ... (re.compile(r"ConnectionError|ConnectionRefused|ECONNREFUSED|network", re.I), ErrorCategory.CONNECTION_ERROR), ``` Every Python exception repr is `XxxError: ...` (or `Exception: ...`), so the generic `Error:`/`Exception:` pattern matches first. A tool result of `"TimeoutError: timed out after 30s"` is classified `RUNTIME_ERROR` instead of `TIMEOUT`; `"ConnectionError: [Errno 111] Connection refused"` is classified `RUNTIME_ERROR` instead of `CONNECTION_ERROR`. The dedicated `TIMEOUT` and `CONNECTION_ERROR` categories — which explicitly list `TimeoutError` and `ConnectionError` — are therefore unreachable for the most common (colon-repr) message shape; they only fire for tokenless phrasings like `deadline exceeded`. That mislabels the learn digest's per-category error stats. ## Fix Check the two specific categories (`TIMEOUT`, `CONNECTION_ERROR`) before the generic `RUNTIME_ERROR` catch-all. A generic exception repr with no timeout/connection token still classifies as `RUNTIME_ERROR`, so existing behavior for those is unchanged (including the opencode scanner's `"Error: command failed with exit code 1"` → `RUNTIME_ERROR`). 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/learn/_shared.py`: move the `TIMEOUT` and `CONNECTION_ERROR` patterns above the generic `RUNTIME_ERROR` pattern, with a comment explaining the ordering. - `tests/test_learn/test_error_classification.py`: new tests asserting `TimeoutError:`/`ConnectionError:` reprs classify specifically, a generic `Error:` stays `RUNTIME_ERROR`, and non-error text is `UNKNOWN`. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/learn/_shared.py tests/test_learn/test_error_classification.py All checks passed! $ python -m py_compile headroom/learn/_shared.py tests/test_learn/test_error_classification.py OK ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the ordering with a dependency-free script that replicates the pattern list under both the old and new orderings, and left the full pytest to CI. - Exact command / steps: classified `"ConnectionError: [Errno 111] Connection refused"` and `"TimeoutError: timed out after 30s"` under the old order (RUNTIME before TIMEOUT/CONNECTION) and the new order (TIMEOUT/CONNECTION before RUNTIME), plus the opencode scanner's `"Error: command failed with exit code 1"` as a regression guard. - Observed result: old order classifies both as `RUNTIME_ERROR`; new order classifies them as `CONNECTION_ERROR` and `TIMEOUT` respectively; the guard string stays `RUNTIME_ERROR` under both orderings, so the existing opencode scanner test is unaffected. - Not tested: a full `headroom learn` digest run; full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The "unit tests pass locally" and "type checking" boxes are unchecked because the full suite imports the ML stack, which I can't run in this environment; the change is a reordering of two entries in a pure pattern list, verified by the standalone proof (which also confirms the one existing test that touches this path stays green) and the new regression tests for CI. Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
e0232df9b4
|
fix(tokenizers): resolve HF tokenizer names by most-specific prefix (#2096)
## Description
`get_tokenizer_name` can pick the wrong tokenizer for a versioned model,
which silently produces wrong token counts.
For a model that isn't a literal key in `MODEL_TO_TOKENIZER`, it falls
back to prefix matching:
```python
for key, value in MODEL_TO_TOKENIZER.items():
if model_lower.startswith(key):
return value
```
That returns the first key the model merely *starts with*, in
dict-insertion order. The table lists short family keys before their
more-specific siblings — `"qwen"` (→ `Qwen/Qwen-7B`) appears before
`"qwen2"`/`"qwen2-7b"`/`"qwen2.5"`. So
`get_tokenizer_name("qwen2-7b-instruct")` matches `"qwen"` first and
returns the **Qwen1** tokenizer, not Qwen2. Qwen1 and Qwen2 have
different vocabularies, so every `count_text`/`count_messages` for that
model is off. `qwen2.5-*` and `deepseek-v2.x` are mis-resolved the same
way.
The sibling tiktoken resolver already documents and guards this exact
pitfall — `get_encoding_for_model` uses an explicit most-specific-first
prefix list with a comment that scanning "for the first key that merely
starts with the prefix is order-dependent and wrong." The HuggingFace
resolver is the one that still scans insertion order.
## Fix
Match the **longest** (most-specific) prefix instead of the first in
insertion order:
```python
for key in sorted(MODEL_TO_TOKENIZER, key=len, reverse=True):
if model_lower.startswith(key):
return MODEL_TO_TOKENIZER[key]
```
Direct-key lookups and the shorter-family fallback (e.g. `deepseek-chat`
→ `deepseek-ai/deepseek-llm-7b-base`) are unchanged.
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/tokenizers/huggingface.py`: `get_tokenizer_name` prefix
matching now iterates keys longest-first and returns the most-specific
match.
- `tests/test_huggingface_tokenizer_timeout.py`: add
`test_get_tokenizer_name_prefers_most_specific_prefix`.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/tokenizers/huggingface.py tests/test_huggingface_tokenizer_timeout.py
All checks passed!
$ python -m py_compile headroom/tokenizers/huggingface.py tests/test_huggingface_tokenizer_timeout.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified against the real key table
with a dependency-free script that parses `MODEL_TO_TOKENIZER` out of
the source and runs both the old (insertion-order) and new
(longest-first) scans, then left the full pytest to CI.
- Exact command / steps: resolved `qwen2-7b-instruct`, `qwen2.5-turbo`,
and `deepseek-v2.5` under both strategies, plus `deepseek-chat` as a
regression guard.
- Observed result: old scan returns `Qwen/Qwen-7B` (Qwen1) for both
qwen2 models and `deepseek-ai/deepseek-llm-7b-base` (v1) for
`deepseek-v2.5`; new scan returns `Qwen/Qwen2-7B`, `Qwen/Qwen2.5-7B`,
and `deepseek-ai/DeepSeek-V2` respectively. `deepseek-chat` resolves
identically under both (`deepseek-ai/deepseek-llm-7b-base`), so the
existing timeout test's model is unaffected.
- Not tested: loading the actual HuggingFace tokenizers
(network/`transformers`); full local `pytest` deferred to CI (OOM, per
above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change is a localized swap of the prefix-scan order in
a pure function, verified by the standalone proof (run against the real
key table) and the new regression test for CI. This mirrors the
same-class fix already present in the sibling tiktoken resolver.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
6137967083
|
fix(pricing): alias retired claude-3-sonnet to Sonnet-tier price, not Haiku (#2095)
## Description The `MODEL_ALIASES` fallback prices the retired Claude 3 Sonnet as Claude 3 Haiku — a different, ~12x cheaper tier. `MODEL_ALIASES` maps models that LiteLLM's cost DB no longer knows about to a current key "that has equivalent pricing" (per the module comment). The two Claude 3.5 Sonnet entries follow that rule — both map to `claude-sonnet-4-20250514`, which is the same `$3 / $15` per-1M tier. But the Claude 3 Sonnet entry was: ```python "claude-3-sonnet-20240229": "claude-3-haiku-20240307", ``` `claude-3-sonnet-20240229` was a Sonnet-tier model at `$3.00 / $15.00` per 1M (input/output). `claude-3-haiku-20240307` is `$0.25 / $1.25`. So whenever LiteLLM lacks the retired Sonnet key and resolution falls through to this alias (via `resolution_candidates` / `pricing_lookup_candidates`), every cost and savings figure for that model is understated **~12x on both input and output**. That's the opposite of the "equivalent pricing" the alias table promises, and it silently biases dashboards/ledger numbers for anyone still routing that model. ## Fix Alias the retired Claude 3 Sonnet to `claude-sonnet-4-20250514` — the same-price ($3/$15) target the sibling retired-Sonnet aliases already use — so the fallback preserves the tier instead of downgrading it. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/pricing/litellm_model_resolution.py`: change the `claude-3-sonnet-20240229` alias target from `claude-3-haiku-20240307` to `claude-sonnet-4-20250514`, with a comment explaining the tier. - `tests/test_pricing_litellm_model_resolution.py`: add `test_retired_claude_3_sonnet_aliases_to_sonnet_tier_not_haiku`. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 check headroom/pricing/litellm_model_resolution.py tests/test_pricing_litellm_model_resolution.py All checks passed! $ python -m py_compile headroom/pricing/litellm_model_resolution.py tests/test_pricing_litellm_model_resolution.py OK ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I checked the tier delta with a dependency-free script against the public list prices and left the full pytest to CI. - Exact command / steps: compared the old alias target (`claude-3-haiku-20240307`, $0.25/$1.25) against the new one (`claude-sonnet-4-20250514`, $3.00/$15.00), which matches the retired Claude 3 Sonnet's own $3/$15 tier. - Observed result: the Haiku target underpriced input 12x ($3.00 / $0.25) and output 12x ($15.00 / $1.25). The new regression test asserts the alias contains no `haiku` and equals the same-tier target used by the other retired-Sonnet aliases. - Not tested: an end-to-end resolution through a live LiteLLM cost DB (the alias only fires when LiteLLM lacks the retired key); full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The "unit tests pass locally" and "type checking" boxes are unchecked because the full suite imports the ML stack, which I can't run in this environment; this is a one-line data fix in a pure module, verified by the tier-delta proof and the new regression test for CI. Reachability is bounded — the alias only matters when LiteLLM's cost DB doesn't already know the retired model — but when it does fire the price is off by a full tier. Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
cf6367add4
|
fix(cache/semantic): don't evict an unrelated entry on an update at capacity (#2094)
## Description
`SemanticCache.put` can evict a perfectly good, unrelated entry when it
merely updates a key that is already cached.
The method runs its at-capacity eviction loop *before* it computes the
entry's key:
```python
self._cleanup_expired()
# Evict if at capacity
while len(self._cache) >= self.config.max_entries:
self._evict_oldest()
...
key = messages_hash or self._generate_key(query)
...
self._cache[key] = entry
```
So when the same key is stored again while the cache is full (a
duplicate store, or a retried request that produces the same
`messages_hash`), the loop fires because `len == max_entries`, evicts
the LRU-oldest *distinct* entry, and only then overwrites the existing
key in place. Writing to an already-present key does not grow the map,
so nothing needed to be evicted — but an unrelated live entry is now
gone, and the next `get` for it is a false miss.
Concretely, with `max_entries=2` and keys `[h1, h2]`, re-storing `h2`
evicts `h1`, leaving `[h2]` even though only two distinct keys were ever
stored.
The sibling `CompressionCache.store_compressed` gets this right: it
deletes the existing key first, inserts, and only then trims — so
re-storing a present key never drops an unrelated entry.
## Fix
Compute the key first, then run the eviction loop only while the key is
genuinely new:
```python
key = messages_hash or self._generate_key(query)
while key not in self._cache and len(self._cache) >= self.config.max_entries:
self._evict_oldest()
```
An in-place update of an existing key no longer evicts anything; adding
a new key still trims to make room exactly as before.
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/cache/semantic.py`: move the cache-key computation above the
eviction loop and gate the loop on `key not in self._cache` so an
in-place update never evicts.
- `tests/test_cache/test_semantic.py`: add
`test_update_at_capacity_does_not_evict_unrelated_entry`.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/cache/semantic.py tests/test_cache/test_semantic.py
All checks passed!
$ python -m py_compile headroom/cache/semantic.py tests/test_cache/test_semantic.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the eviction logic with a
dependency-free script that replicates the `OrderedDict` +
`_evict_oldest` (popitem last=False) behavior for the old vs new loop,
and left the full pytest to CI.
- Exact command / steps: with `max_entries=2`, store `h1` then `h2`,
then re-store the already-present `h2`, under both the old loop (evict
before key dedup) and the new loop (evict only when key is new).
- Observed result: old loop leaves `['h2']` and `get(h1)` returns `None`
(h1 wrongly evicted); new loop leaves `['h1', 'h2']` with `get(h1)`
intact and `h2` updated. The regression test asserts h1 survives and h2
reflects the update.
- Not tested: a live embedding-backed cache round-trip; full local
`pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change is a localized reordering of two existing
statements plus a loop guard, verified by the standalone proof and the
new regression test for CI. This is a different defect from the earlier
messages-hash keying fix — that one was about which slot a request maps
to; this one is about eviction dropping a live entry on an in-place
update.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
ae10d6c99d
|
fix(tokenizers): don't tokenize image blocks as text in TiktokenCounter (#2093)
## Description
`TiktokenCounter.count_messages` explodes the token count for any
content block that isn't plain text or an OpenAI `image_url`.
The multi-part content loop handles exactly two shapes:
```python
if part.get("type") == "text":
total += self.count_text(part.get("text", ""))
elif part.get("type") == "image_url":
... # 85 / 170 tokens by detail
else:
total += self.count_text(str(part)) # <-- everything else
```
Every other block shape reaching the `else` gets `str(part)`-ified and
tokenized as text. That includes Anthropic's `{"type": "image",
"source": {"type": "base64", "data": "<...>"}}`, `tool_result`,
`tool_use`, and the Strands SDK blocks. Over the wire the image `data`
is a base64 string, so a 1MB image turns into ~1.4M characters of "text"
and is counted as **~330K tokens** for a single image (a ~218x overcount
in a standalone repro). Anything that relies on the count — budget
gating, the compress/skip decision, savings math — is thrown off for
multimodal requests that route through the tiktoken counter.
The base class already solved this: `BaseTokenizer._count_content_parts`
prices `image`/`image_url`/`input_image` at a flat bounded estimate and
has a comment stating it exists specifically to stop "a 1MB image =
~330K fake tokens". The tiktoken override just never delegated to it for
the non-text shapes.
## Fix
Delegate unknown block shapes in the `else` branch to
`self._count_content_parts([part])` instead of stringifying them. `text`
and `image_url` keep the existing tiktoken-specific handling (including
the 85/170 detail split); everything else now gets the base handler's
bounded pricing.
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
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/tokenizers/tiktoken_counter.py`: the `count_messages`
multi-part `else` branch delegates to the base
`_count_content_parts([part])` rather than `count_text(str(part))`.
- `tests/test_tokenizers.py`: add
`test_count_messages_image_block_is_not_stringified` — a base64 image
block inside list content must stay bounded (well under the tens of
thousands of tokens the blob would produce as text).
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
All checks passed!
$ python -m py_compile headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the magnitude with a
dependency-free script that models the old `count_text(str(part))` path
against the base handler's bounded image estimate, and left the full
pytest to CI.
- Exact command / steps: built a ~1MB PNG as an Anthropic `image` block
with the payload base64-encoded (as it arrives over the wire), computed
the old path (`len(str(part)) / ~4` chars-per-token) versus the new path
(base handler prices an image block at a flat 1600).
- Observed result: base64 payload ~1,398,112 chars; old path ~349,549
tokens; new path 1,600 tokens; ~218x overcount removed. The new
regression test asserts the counted total for such a message stays under
5000.
- Not tested: a live tiktoken end-to-end count through the proxy; full
local `pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change is a localized delegation to an existing base
method, verified by the standalone magnitude proof and the new
regression test for CI. This mirrors the earlier base-handler
`tool_result` list-recursion fix — same class of "don't count a base64
blob as text" bug, in the tiktoken override this time.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
b097ef3e25
|
fix(install): don't let host env override the manifest in persistent-docker (#2090)
## Description In persistent-docker deployments a stale host env var can silently override the value the deployment manifest pinned for the container. `build_runtime_command` builds the `docker run` argv in two passes: 1. It emits the manifest's pinned env as `--env NAME=VALUE` (from `base_env` plus the deployment env). 2. It then walks `os.environ` and, for every name matching a `PASSTHROUGH_ENV_PREFIXES` prefix, appends a bare `--env NAME` so the host value is forwarded into the container. A manifest-pinned name and a host-exported name can collide when they share a passthrough prefix. `HEADROOM_BACKEND` is the clearest case: the manifest pins `--env HEADROOM_BACKEND=anthropic` in pass 1, and pass 2 also matches the `HEADROOM_` prefix and appends a bare `--env HEADROOM_BACKEND`. Docker resolves duplicate `--env` flags last-wins, and the bare passthrough comes last, so a stale host export `HEADROOM_BACKEND=anyllm` wins and the container runs a different backend than its deployment config says. `start_persistent_docker` runs the resulting command through `subprocess.run` with the parent process environment, so whatever the operator happened to have exported leaks in and overrides the manifest. The fix skips the bare passthrough for any name the manifest already pins, so the pinned value stands while unrelated host secrets (API keys and so on) are still passed through as before. 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 - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/install/runtime.py`: skip the bare `--env NAME` passthrough when `NAME` is already pinned by the manifest (`and name not in runtime_env`). - `tests/test_install/test_runtime.py`: add `test_build_runtime_command_docker_manifest_env_beats_host_passthrough`, which exports a conflicting `HEADROOM_BACKEND` and asserts the command keeps the manifest value and emits no bare passthrough for it. - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uvx ruff@0.15.17 format headroom/install/runtime.py tests/test_install/test_runtime.py 2 files left unchanged $ uvx ruff@0.15.17 check headroom/install/runtime.py tests/test_install/test_runtime.py All checks passed! $ python -m py_compile headroom/install/runtime.py tests/test_install/test_runtime.py OK ``` ## Real Behavior Proof - Environment: local checkout, Python 3.11, `uvx ruff@0.15.17`. - Exact command / steps: ran a standalone script that reproduces the two-pass argv build and models Docker's duplicate `--env` last-wins resolution, with the manifest pinning `HEADROOM_BACKEND=anthropic` and the host exporting `HEADROOM_BACKEND=anyllm`. - Observed result: the old build resolves the effective `HEADROOM_BACKEND` to the host value `anyllm` (bare passthrough wins); the new build keeps the manifest value `anthropic` and emits no bare `HEADROOM_BACKEND` token, while a non-pinned passthrough (`ANTHROPIC_API_KEY`) is still forwarded. - Not tested: I did not run the full `pytest` suite locally because it pulls in the ML stack; the new regression test is left for CI. ## 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 - [ ] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The "unit tests pass locally" and "type checking" boxes are unchecked because the full suite imports the ML dependencies, which I can't run in this environment; the change is a pure function over `build_runtime_command`, verified by the standalone proof above and covered by the new regression test for CI. Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Tejas Chopra
|
508530a6fe
|
chore: union-merge CHANGELOG.md to stop cross-PR merge conflicts (#2118)
## Description Nearly every open PR appends an entry to the same `## [Unreleased]` section of `CHANGELOG.md`. With dozens of concurrent PRs, merging **any** one flips the others to `CONFLICTING`, so bulk-merging the backlog becomes quadratic (each merge re-conflicts the rest on the changelog alone — code is fine). This adds git's built-in **`union`** merge driver for `CHANGELOG.md` via `.gitattributes`, so changelog conflicts auto-resolve by keeping every entry — removing the single largest merge-conflict blocker across the PR backlog. `union` is a built-in driver (no `.git/config` needed) and is honored by GitHub's server-side merge. ## Type of Change - [x] Code refactoring / repo infrastructure (no functional change) ## Changes Made - `.gitattributes`: `CHANGELOG.md merge=union` ## Testing - [x] Linting passes (no code changed) ### Test Output ```text N/A — .gitattributes-only change; verified the union driver resolves overlapping [Unreleased] appends locally. ``` ## Real Behavior Proof - Before: merging one PR that touches `CHANGELOG.md` conflicts every other PR that touches it. - After: overlapping `CHANGELOG.md` appends merge automatically (both entries kept), no manual resolution. |
||
|
Vinay Gupta
|
bd8de9f382
|
fix(proxy): keep recent stats request rows (#1922)
## Description `/stats.recent_requests` was built from the request-log tail, but then filtered out any row whose compact token fields were incomplete. That made the dashboard/API summary diverge from the raw request counters: `requests.by_model` could show many recent requests for a model while `recent_requests` only showed the few rows with complete compression-token accounting. This fixes the stats payload so the compact `recent_requests` table mirrors the latest request-log rows without masking unknown token accounting as measured zero. Missing/non-finite compact numeric fields now remain `null`, and each row exposes `token_accounting_status` plus `has_exact_tokens` so API clients and the dashboard can distinguish complete, partial, and missing token accounting. Closes #1914 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Changes Made - Removed the token-completeness filter from `_build_recent_request_payload()`. - Preserved unknown compact recent-request numeric fields as `null`. - Added `token_accounting_status` and `has_exact_tokens` to compact recent request rows. - Updated the dashboard recent-requests table to render unknown token/latency fields as `unknown`. - Hardened `build_session_summary()` against token-incomplete request-log rows and surfaced an `unknown_token_accounting` bucket. - Added TypeScript SDK fields for the compact recent-request stats contract. - Added a regression test covering missing, partial, and complete token-accounting rows. ## Testing - [x] Focused stats regression passes - [x] Adjacent summary/MCP tests pass - [x] TypeScript SDK typecheck/tests pass - [x] Ruff check passes - [x] Ruff format check passes - [x] Diff whitespace check passes - [ ] Full suite not run ### Test Output ```text $ uv run --no-project ... pytest tests/test_proxy_stats_recent_requests.py -q 4 passed, 1 warning $ uv run --no-project ... pytest tests/test_proxy_dashboard_stats_cache.py::test_session_summary_uses_generic_cli_filtering_keys tests/test_proxy_dashboard_stats_cache.py::test_session_summary_surfaces_codex_ws_counters tests/test_ccr_mcp_server.py -q 19 passed, 1 skipped $ npm run typecheck tsc --noEmit $ npm test 296 passed, 33 skipped $ uv run --no-project --with ruff ruff check headroom/proxy/server.py headroom/proxy/cost.py headroom/ccr/mcp_server.py tests/test_proxy_stats_recent_requests.py All checks passed! $ uv run --no-project --with ruff ruff format --check headroom/proxy/server.py headroom/proxy/cost.py headroom/ccr/mcp_server.py tests/test_proxy_stats_recent_requests.py 4 files already formatted $ git diff --check clean ``` ## Verification - Principal engineer review: no blockers after the final finite-number/accounting-status alignment. - Senior developer review: no blockers; implementation and focused coverage look solid for #1914. - Architect/design review: original API/UI contract blocker resolved; unknowns remain visible as `null`/`unknown` instead of measured zero. ## Notes The normal editable `uv run pytest ...` path is blocked locally by the known native build issue in `esaxx-rs` (`fatal error: 'cstdint' file not found`) while building the Rust extension on this machine. I validated the Python-only stats path with a temporary `headroom._core` import stub and `HEADROOM_REQUIRE_RUST_CORE=false`; no repository files were changed for that stub. ## Review Readiness - [x] I have performed a self-review - [ ] This PR is ready for human review |
||
|
Rod Boev
|
10ed14e7f6
|
fix(proxy): keep Kompress warmup off the startup path (#2001)
## Description Proxy startup can enter cached Kompress native model initialization before binding its port. On the RHEL/CentOS 7-family environment reported in #1908, that path terminates in a deterministic `libarrow.so.2400` jemalloc-thread segfault with no Python traceback. Cache-only preload still initializes native libraries when the model is already cached. Defer Kompress model and tokenizer loading out of startup while preserving the existing lazy request path and the eager warmups for non-Kompress components. Closes #1908. ## 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 - Stop cached and uncached Kompress models from entering native preload during proxy startup. - Record enabled Kompress as deferred until its existing lazy request path needs it. - Preserve disabled-Kompress routing and non-Kompress eager warmups. - Add lifecycle, cache-state, disabled-mode, and warmup-preservation regression coverage. - Document the startup behavior change in the changelog. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_kompress_preload_deferral.py tests/test_kompress_request_nonblocking.py -q`) - [x] Linting passes (`uv run ruff check headroom/transforms/content_router.py tests/test_kompress_preload_deferral.py tests/test_kompress_request_nonblocking.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_kompress_preload_deferral.py tests/test_kompress_request_nonblocking.py -q 17 passed in 1.28s uv run ruff check headroom/transforms/content_router.py tests/test_kompress_preload_deferral.py tests/test_kompress_request_nonblocking.py All checks passed ``` ## Real Behavior Proof - Environment: Red OS 7.3 or equivalent RHEL/CentOS 7-family system, glibc 2.17, Python 3.11, pyarrow 24.0.0, onnxruntime 1.27.0, cached Kompress model - Exact command / steps: start `headroom proxy` with Kompress enabled, wait 30 seconds, query the loopback health endpoint, verify deferred Kompress warmup in logs, then inspect `journalctl -k` for new `libarrow.so.2400` or `jemalloc_bg_thd` faults - Observed result: automated coverage now proves startup avoids the cached Kompress preload boundary, keeps non-Kompress warmups live, and preserves `unavailable` status when dependencies are absent - Not tested: the native reporter-host run ## 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 - [ ] 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 ## Screenshots Not applicable. ## Additional Notes Focused local validation included `tests/test_kompress_request_nonblocking.py` alongside the startup regression suite. The change is scoped to startup warmup; it does not claim to repair the external `libarrow.so` or jemalloc incompatibility when Kompress later executes. `HEADROOM_DISABLE_KOMPRESS=1` remains the supported narrow workaround for hosts that cannot run the native path. Co-authored-by: JD Davis <mxjerrett@gmail.com> |
||
|
GUOHAO LIU
|
8322677ddd
|
docs(proxy): document --protect-tool-results side effect on protect_recent_reads_fraction (#2054)
## Description
Documents the side effect of `--protect-tool-results`: in token mode,
naming
any tool also resets `protect_recent_reads_fraction` from 0.3 to 0.0
(Closes
#1921). Previously this interaction was only visible in the source code
ordering in server.py and led to user confusion.
## Type of Change
- [ ] 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)
- [x] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/cli/proxy.py`: extend `--protect-tool-results` CLI help text
to
explain that in token mode it also resets
`protect_recent_reads_fraction`
from 0.3 → 0.0, restoring full indefinite protection for all
excluded-tool
results (Read/Glob/Grep/Write/Edit)
- `headroom/proxy/server.py`: add inline comment documenting the
ordering
dependency (token mode runs first, protect_tool_results overrides after)
## Testing
- [x] Linting passes (`ruff check`)
- [x] No functional changes (docs only)
- [ ] N/A — no code logic changes
### Test Output
```text
$ uv run ruff check headroom/cli/proxy.py headroom/proxy/server.py
All checks passed!
```
## Real Behavior Proof
- Environment: Linux, headroom main @
|
||
|
Rudimar Ronsoni
|
c4ddcb93a7
|
fix(codex): skip sockets in session home overlay (#2104)
## Description Prevent `headroom wrap codex` from failing when the active `CODEX_HOME` contains a Unix socket. The session overlay copied every entry with `shutil.copytree()`, which raises `shutil.Error` when it reaches Git's `fsmonitor--daemon.ipc` socket. The overlay now skips socket entries while continuing to copy regular Codex state and surface unrelated copy errors. Closes #2103 ## 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 - Ignore filesystem sockets while seeding the temporary Codex session home. - Add a regression test with a real nested `fsmonitor--daemon.ipc` socket and a regular sibling file. ## Testing - [x] Focused unit tests pass (`pytest tests/test_cli/test_wrap_codex.py -q`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New regression test added - [ ] Manual interactive testing performed ### Test Output ```text Docker, Linux arm64, Python 3.12.12 pytest tests/test_cli/test_wrap_codex.py -q 88 passed in 5.92s ruff check . All checks passed! ruff format --check . 1191 files already formatted mypy headroom --ignore-missing-imports Success: no issues found in 469 source files ``` ## Real Behavior Proof - Environment: isolated Docker container on Linux arm64 with Python 3.12.12 and Rust 1.95.0 - Exact command / steps: bind a real Unix socket at `vendor_imports/skills/.git/fsmonitor--daemon.ipc`, then enter `_codex_session_home_overlay()` through the focused pytest regression - Observed result: the regular sibling file is copied, the socket is omitted, the source socket remains active, and the overlay exits cleanly - Not tested: an interactive Codex launch against the live host `~/.codex` ## 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] My changes generate no new warnings - [x] I have added tests that prove the fix is effective - [x] New and existing focused Codex wrapper tests pass with my changes ## Additional Notes The filter is intentionally limited to socket entries. Permission errors and failures involving regular files still propagate from `shutil.copytree()`. |
||
|
Abhay Singh
|
20968a4fa4
|
fix(wrap/opencode): unwrap removes the rtk block from AGENTS.md (#2025)
## Description `headroom wrap opencode` injects the marker-fenced rtk guidance block — "prefix shell commands with `rtk`" — into **both** instruction files (`headroom/cli/wrap.py`): ```python # wrap opencode project_agents = Path.cwd() / "AGENTS.md" _inject_rtk_instructions(project_agents, verbose=verbose) global_agents = _opencode_home_dir() / "AGENTS.md" _inject_rtk_instructions(global_agents, verbose=verbose) ``` But `unwrap_opencode` only restores the OpenCode config and cleans up MCP servers — it never removes that rtk block. So after `unwrap opencode`, both `AGENTS.md` files still contain the marker-fenced instruction, and a plain `opencode` launch keeps following "prefix shell commands with `rtk`" and fails once the managed rtk binary is off PATH. `unwrap_codex` (#1421) and `unwrap_copilot` both already do this cleanup via `_remove_rtk_instructions`; opencode was simply never given the equivalent — a wrap/unwrap asymmetry. Closes: no issue filed — found while auditing wrap/unwrap symmetry across agents. ## Fix In `unwrap_opencode`, after the MCP cleanup, strip the rtk block from both files it was injected into, mirroring `unwrap_codex`: ```python for _agents_md in (Path.cwd() / "AGENTS.md", _opencode_home_dir() / "AGENTS.md"): if _remove_rtk_instructions(_agents_md): click.echo(f" Removed Headroom rtk instructions from {_agents_md}.") ``` Best-effort and unconditional, matching the existing MCP cleanup and the codex/copilot unwrap paths. `_remove_rtk_instructions` already no-ops when the file or marker is absent. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/cli/wrap.py`: `unwrap_opencode` removes the rtk block from the project and global `AGENTS.md`. - `tests/test_cli/test_wrap_opencode.py`: add `test_unwrap_opencode_removes_rtk_from_agents_md` (wrap injects into both, unwrap removes from both). ## Testing - [x] New regression test added (`tests/test_cli/test_wrap_opencode.py`) - [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17` - [ ] Full `pytest` deferred to CI (local-OOM reason below). ```text $ uvx ruff@0.15.17 check headroom/cli/wrap.py tests/test_cli/test_wrap_opencode.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.10, headroom from this branch. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the wrap→unwrap round-trip through the new Click-runner test (which drives the real command) and reasoned through the marker logic; the full pytest runs on CI. - Exact command / steps: the added test runs `wrap opencode --no-mcp` (asserts `_RTK_MARKER` present in both the project and global `AGENTS.md`), then `unwrap opencode`, and asserts the marker is gone from both. - Observed result (the assertions the test enforces): before the fix, `unwrap opencode` left `_RTK_MARKER` in both files; after the fix both are clean: ```text after wrap: _RTK_MARKER in project AGENTS.md ✓ _RTK_MARKER in global AGENTS.md ✓ after unwrap: _RTK_MARKER absent (project) ✓ _RTK_MARKER absent (global) ✓ ``` - Not tested: launching a real `opencode` binary (mocked in the test, as the existing wrap tests do). Full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes — ran lint + the new Click-runner test path; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Directly parallels the merged `unwrap codex` rtk cleanup (#1421); no new dependencies. - @JerrettDavis tagging you — same class as the codex rtk fix, just the opencode side that was missed. Thanks! --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
6ecbdd6b52
|
fix(proxy/savings): don't bill fallback rate for free (0-priced) models (#2024)
## Description
Two savings/cost estimators in `headroom/proxy/savings_tracker.py` read
`input_cost_per_token`
from litellm and use a falsy check to decide whether the price is known:
```python
# _estimate_compression_savings_usd
input_cost_per_token = info.get("input_cost_per_token")
if not input_cost_per_token:
raise RuntimeError("input cost unavailable")
return float(tokens_saved) * float(input_cost_per_token)
except Exception:
return float(tokens_saved) * float(DEFAULT_FALLBACK_INPUT_COST_PER_TOKEN) # $3/M
```
`if not input_cost_per_token` is true for **both** a missing key
(`None`) *and* a legitimate
`0.0`. So a genuinely **free** model — free-tier / local / vendored-at-0
entries, which litellm
does carry — is treated as "price unavailable" and billed the
`DEFAULT_FALLBACK_INPUT_COST_PER_TOKEN`
($3/M) fallback. The result is fabricated dollar savings (and, in
`_estimate_input_cost_usd`,
fabricated cost) for a model that costs nothing. The same pattern is at
`_estimate_input_cost_usd`.
(`_estimate_cache_savings_usd` also uses `if not ...`, but there both
branches correctly resolve
to `$0` for a free model, so it is left unchanged.)
Closes: no issue filed — found while auditing the cost/savings
estimators.
## Fix
Distinguish "missing" from "legitimately zero" with an explicit `is
None` check, so a present
`0.0` flows through as `$0` while an absent key still falls back:
```python
input_cost_per_token = info.get("input_cost_per_token")
if input_cost_per_token is None:
raise RuntimeError("input cost unavailable")
```
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/proxy/savings_tracker.py`: `is None` check (instead of `if
not ...`) in `_estimate_compression_savings_usd` and
`_estimate_input_cost_usd`.
- `tests/test_savings_tracker_zero_price.py`: free model → `$0`, unknown
model → fallback, paid model → real price.
## Testing
- [x] New regression tests added
(`tests/test_savings_tracker_zero_price.py`)
- [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17`
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uvx ruff@0.15.17 check headroom/proxy/savings_tracker.py tests/test_savings_tracker_zero_price.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I verified the estimator logic
with a dependency-free script and left the full pytest to CI.
- Exact command / steps: ran a free model (`input_cost_per_token: 0.0`),
an unknown model (key absent), and a paid model through both the old `if
not ...` and new `is None` logic.
- Observed result: the old logic bills $3/M for the free model; the new
logic charges $0 while still falling back for the unknown model and
leaving the paid model unchanged:
```text
FREE old=3.0000 new=0.0000
UNKNOWN old=3.0000 new=3.0000
PAID old=3.0000 new=3.0000
PHANTOM-COST FIX VERIFIED (free model: $3.00 phantom -> $0.00; unknown still falls back)
```
- Not tested: a full `record_request` round-trip persisted to the
savings file (needs the heavy stack). The fix is confined to the two
estimators and the new tests drive them directly with a stubbed litellm.
Full local `pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes — ran
lint + a standalone logic check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- Two one-line `is None` fixes plus tests; no new dependencies. Same
falsy-zero class as the `HEADROOM_MIN_TOKENS=0` (#1886) and Copilot
`remaining: 0` (#1997) fixes.
- @JerrettDavis tagging you — small one, surfaces phantom savings for $0
models. Thanks!
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Rod Boev
|
31abb696dd
|
fix(memory): honor explicit store=false on Responses requests (#2017)
## Description This PR addresses the source-backed `store=false` mutation documented inside #1944. Headroom currently injects Responses memory tools by silently flipping explicit `store=false` to `true`, which Codex-backed Responses requests reject. The fix respects explicit `store=false` by skipping only the tool-continuation memory path for those requests. Memory context injection stays unchanged. Refs #1944 ## 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 - Honor explicit `store=false` on `/v1/responses`. - Skip only Responses memory tools that depend on stored-response continuation. - Preserve current behavior when the client does not opt out of storage. - Add focused regression coverage and a changelog note. ## Testing - [x] Unit tests pass - [x] Linting passes - [ ] Type checking passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_openai_responses_context_compaction.py -q 11 passed uv run ruff check headroom/proxy/handlers/openai.py tests/test_openai_responses_context_compaction.py All checks passed uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_openai_responses_context_compaction.py 2 files already formatted ``` ## Real Behavior Proof - Environment: Responses client with explicit `store=false` - Exact command / steps: send a memory-enabled `/v1/responses` request with `store=false` - Observed result: the handler now preserves explicit `store=false` and skips only the Responses memory-tool injection path that depends on stored-response continuation; focused regression coverage proves stored/default requests still allow the path - Not tested: the Desktop disconnect tracked separately in #1944 ## 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 - [ ] 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 The large Codex Desktop mid-stream disconnect remains a separate external-proof problem. This PR is intentionally limited to the explicit `store=false` mutation proven in the same issue thread. Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
f542b70413
|
fix(proxy/memory): capture user text blocks for the retrieval query (#2064)
## Description
`extract_memory_query_sources` (`headroom/proxy/memory_query_policy.py`)
builds the text used to
retrieve relevant memories. It captures `latest_user` **only** when a
user message's `content` is
a plain `str`:
```python
if role == "user":
if isinstance(content, list):
_append_anthropic_tool_results(content, tool_outputs=..., lookback_tools=...)
elif isinstance(content, str) and not latest_user:
latest_user = content
```
But the standard Anthropic `/v1/messages` shape (used by Claude Code)
sends the user turn as a
**list of content blocks** — `content=[{"type":"text","text":"help me
refactor auth"}]`. That
routes into `_append_anthropic_tool_results`, which extracts only
`type=="tool_result"` blocks
and **never reads the `type=="text"` blocks** — so the actual user
prompt is discarded.
Downstream (`handlers/anthropic.py` → `MemoryQuery.from_messages` →
`to_embedding_input`):
- On a **first turn** (no prior assistant/tool context) the embedding
input is `""`, and the
memory handler then returns `None` — **memory injection is silently
skipped entirely**.
- With history present, the query is assembled from stale assistant/tool
context **minus the
current question**, so retrieval targets the wrong text.
Closes: no issue filed — found while auditing the memory retrieval query
policy.
## Fix
In the list-content user branch, also collect the `text` blocks into
`latest_user` (guarded by
`if not latest_user` so the latest turn wins), alongside the existing
tool-result extraction.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/proxy/memory_query_policy.py`: capture Anthropic user `text`
blocks into `latest_user`.
- `tests/test_memory_query_policy.py`: add
`test_extract_sources_captures_anthropic_user_text_blocks` and
`test_extract_sources_captures_user_text_alongside_tool_result`.
## Testing
- [x] New regression tests added (`tests/test_memory_query_policy.py`)
- [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17`
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uvx ruff@0.15.17 check headroom/proxy/memory_query_policy.py tests/test_memory_query_policy.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I verified the extraction logic
with a dependency-free script and left the full pytest to CI.
- Exact command / steps: ran a text-block user turn (and a mixed
text+tool_result turn, a plain-string turn, and multiple user turns)
through the old and new logic.
- Observed result: the old logic drops the user text (empty query →
injection skipped); the new logic captures it, still gathers tool
output, and keeps the plain-string / latest-turn behavior:
```text
text-block user: OLD user_text='' NEW user_text='help me refactor auth'
MEMORY QUERY TEXT-BLOCK FIX VERIFIED (old drops user text; new captures it)
```
- Not tested: a full memory retrieval round-trip through the embedder
(needs the heavy stack). The fix is confined to
`extract_memory_query_sources` and the new tests drive it directly. Full
local `pytest` deferred to CI (OOM, per above).
## 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
- [ ] New and existing unit tests pass locally with my changes — ran
lint + a standalone logic check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- Small, contained fix in the query-source extractor; no new
dependencies. The existing
`test_extract_sources_handles_anthropic_tool_result_without_user_text`
still passes (its list turn has no text block).
- @JerrettDavis tagging you — this silently disables memory injection
for the standard Claude Code request shape on a first turn, so it seemed
worth surfacing. Thanks!
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
a5bdc5491f
|
fix(memory/sqlite): don't emit OFFSET without LIMIT in query (#2063)
## Description
`SQLiteMemoryStore.query` (`headroom/memory/adapters/sqlite.py`) builds
pagination like this:
```python
if filter.limit is not None:
query += " LIMIT ?"
params.append(filter.limit)
if filter.offset > 0:
query += " OFFSET ?"
params.append(filter.offset)
```
SQLite's grammar allows `OFFSET` **only** as part of a `LIMIT` clause.
So a `MemoryFilter` with
an offset but no limit produces `... ORDER BY created_at DESC OFFSET ?`,
which SQLite rejects:
```
sqlite3.OperationalError: near "OFFSET": syntax error
```
Both `offset` and `limit` are public `MemoryFilter` fields (`ports.py`:
`limit` defaults to
`None`, `offset` to `0`), so any caller paginating with an offset but no
explicit limit crashes.
Closes: no issue filed — found while auditing the memory store query
builder.
## Fix
When an offset is present without a limit, emit SQLite's unbounded
`LIMIT -1` so `OFFSET` is
grammatically valid:
```python
if filter.offset > 0:
if filter.limit is None:
query += " LIMIT -1"
query += " OFFSET ?"
params.append(filter.offset)
```
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/memory/adapters/sqlite.py`: emit `LIMIT -1` when paginating
with an offset but no limit.
- `tests/test_memory/test_hierarchical.py`: add
`test_query_offset_without_limit` (offset skips rows; offset past the
end returns `[]`; no crash).
## Testing
- [x] New regression test added
(`tests/test_memory/test_hierarchical.py`)
- [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17`
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uvx ruff@0.15.17 check headroom/memory/adapters/sqlite.py tests/test_memory/test_hierarchical.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I reproduced the exact SQL
against a real stdlib `sqlite3` in-memory DB (the store's query is pure
SQL) and left the full pytest to CI.
- Exact command / steps: built the same `ORDER BY ... [LIMIT] [OFFSET]`
query for `offset=2, limit=None` with the old and new logic and ran it
against a 5-row table.
- Observed result: the old builder raises the exact `OperationalError`;
the new builder skips `offset` rows and returns the rest, and
`LIMIT`-only / `LIMIT`+`OFFSET` still work:
```text
OLD offset-no-limit: OperationalError -> near "OFFSET": syntax error
NEW offset-no-limit: rows=[2, 1, 0]
SQLITE OFFSET-WITHOUT-LIMIT FIX VERIFIED (old crashes; new paginates)
```
- Not tested: the full `HierarchicalMemory` stack (needs the heavy
embedder). The new test drives `SQLiteMemoryStore.query` directly with
`save_batch` + `MemoryFilter`. Full local `pytest` deferred to CI (OOM,
per above).
## 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
- [ ] New and existing unit tests pass locally with my changes — ran
lint + a standalone SQLite check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- One-line grammar fix plus a test; no new dependencies.
- @JerrettDavis tagging you — a paginating caller (offset, no limit)
currently crashes the memory store query; quick one. Thanks!
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
4056117d90
|
fix(proxy/gemini): preserve non-text content across the compression round-trip (#2079)
## Description Two related content-loss bugs in the Gemini `contents[]` <-> `messages[]` compression round-trip. Both drop or misplace real user content that entries with **non-text** parts should carry through untouched. They share the same theme (non-text preservation), so they're bundled here as two commits. ### 1. Google batch handler restores preserved entries by the wrong index (`handlers/batch.py`) The `batchGenerateContent` handler restored preserved (non-text) entries with the raw-index loop that commit #836 (`_rebuild_gemini_contents`) replaced in the three non-batch Gemini handlers: ```python for orig_idx, original_content in preserved_contents.items(): if orig_idx < len(optimized_contents): optimized_contents[orig_idx] = original_content ``` `preserved_indices` are indices into the **original** `contents[]`, but `optimized_contents` is a **shorter** list (text-less entries produce no message). Indexing `optimized_contents` by `orig_idx` overwrites the wrong entry and drops any preserved entry whose original index is past the optimized length. For: ```python [user text, model functionCall, user functionResponse, model text] ``` the batch was forwarded to Google as **two** entries: the model's answer overwritten by the functionCall, and the functionResponse dropped. Unlike `gemini.py` there is no `if optimized_messages != messages` gate, so it runs on every mixed batch item. **Fix:** use the shared `_rebuild_gemini_contents` interleaving helper. ### 2. Code-execution parts not detected as non-text (`handlers/gemini.py`) `_has_non_text_parts` only recognized `inlineData`/`fileData`/`functionCall`/`functionResponse`. Gemini's code-execution feature emits `executableCode` and `codeExecutionResult` parts, echoed back in `contents[]` on later turns. Because they weren't detected: - a mixed `text`+`executableCode` entry lost its code payload (only the text survived the round-trip); - a text-less `executableCode`+`codeExecutionResult` entry was treated as a phantom in `_rebuild_gemini_contents` — it consumed the next optimized message, dropping the whole code turn and shifting a following user turn into the model's role slot (corrupting role alternation). **Fix:** add both keys to the non-text detection so those entries are preserved verbatim. Closes: no issue filed — both found while auditing the Gemini contents<->messages round-trip. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/proxy/handlers/batch.py`: use `_rebuild_gemini_contents` instead of the raw-index restore loop. - `headroom/proxy/handlers/gemini.py`: recognize `executableCode` / `codeExecutionResult` in `_has_non_text_parts`. - `tests/test_proxy_handlers_batch.py`: add `test_handle_google_batch_create_preserves_functioncall_response_order`, driving the handler with the **real** Gemini converters (the existing batch tests stub them, which hid the bug); mix `GeminiHandlerMixin` into the shared `DummyBatchHandler` so `_rebuild_gemini_contents` is available. - `tests/test_google_multimodal.py`: extend the parametrized `test_each_non_text_key_detected` to the two new keys, and add `test_code_execution_entry_survives`. ## Testing - [x] New regression tests added (`tests/test_proxy_handlers_batch.py`, `tests/test_google_multimodal.py`) - [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17` - [ ] Full `pytest` deferred to CI (local-OOM reason below). ```text $ uvx ruff@0.15.17 check headroom/proxy/handlers/batch.py headroom/proxy/handlers/gemini.py \ tests/test_proxy_handlers_batch.py tests/test_google_multimodal.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.10, headroom from this branch. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the interleaving/detection with dependency-free scripts (replicating the Gemini converters, the old loop, and `_rebuild_gemini_contents`) and left the full pytest to CI. - Exact command / steps: ran two standalone scripts. Script 1 rebuilds a Gemini batch request with `preserved_indices` holding a `functionCall`/`functionResponse` pair and compares the old raw-index loop against `_rebuild_gemini_contents`. Script 2 feeds a `codeExecutionResult` entry through `_has_non_text_parts` and the preserve path with and without the two new allowlist keys. Also ran `uvx ruff@0.15.17 check` on the changed files and tests. - Observed result: the old batch loop drops the `functionResponse` and overwrites the answer (4 parts collapse to 2); `_rebuild_gemini_contents` keeps all 4. Without the new keys the code-execution entry is dropped/shifted (2 parts, code absent); with them it survives intact (3 parts, code present). Lint clean. See the two blocks below. Batch fix (bug #1): ```text preserved_indices: [1, 2] OLD result parts: ['text', 'functionCall'] len 2 NEW result parts: ['text', 'functionCall', 'functionResponse', 'text'] len 4 GEMINI BATCH REBUILD FIX VERIFIED (old drops response + overwrites answer; new keeps all 4) ``` Code-execution fix (bug #2): ```text (b) OLD len=2 NEW len=3 (a) OLD has code=False NEW has code=True GEMINI CODE-EXECUTION PRESERVE FIX VERIFIED (old drops/shifts; new keeps intact) ``` - Not tested: a live Google/Gemini round-trip (handlers stubbed, as the existing tests do). Full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes — ran lint + standalone logic checks; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Two small behavioral changes (one loop -> shared helper, two keys added to an allowlist) plus regression tests; no new dependencies. Both complete/extend the non-text preservation the non-batch handlers already do (the #836 line). - @JerrettDavis tagging you since you reviewed the recent Gemini fixes. Both of these drop content (functionResponse/images on batch; code-execution on the normal round-trip), so they seemed worth surfacing together. Thanks. --------- Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |