From ecdcf13f3fe28a77cdf03fb3739e20d47d1f383b Mon Sep 17 00:00:00 2001 From: thejesh23 <35212698+thejesh23@users.noreply.github.com> Date: Mon, 13 Jul 2026 16:57:04 -0700 Subject: [PATCH] fix(compress): don't mutate the caller's CompressConfig via kwargs (#2134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 - [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) Co-authored-by: Tejas Chopra --- CHANGELOG.md | 1 + headroom/compress.py | 9 ++++++--- tests/test_compress_api.py | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd342a574..395b3d338 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed +- **compress:** stop mutating the caller's `CompressConfig`. `compress(config=my_cfg, protect_recent=0, target_ratio=0.2)` used to write those kwargs onto `my_cfg`, so a shared per-agent config was silently rewritten by every request that overrode a single option. - **paths:** reject `.`, `..`, and NUL as plugin names so `plugin_config_dir` / `plugin_workspace_dir` cannot resolve outside the `plugins/` sandbox. Previously `plugin_config_dir("..")` returned the entire config root and `plugin_workspace_dir("..")` returned the workspace root (savings ledger, memory DB, license cache, logs). - **backends/litellm:** drop tool names over 64 chars before calling Bedrock Converse (`send_message` and `stream_message`), instead of letting the whole request 401. The Bedrock Converse API hard-rejects any tool name past that length, and Claude Code includes every globally-added claude.ai MCP connector tool in every request, even ones the user hasn't enabled locally, so a single oversized connector name broke every call through this backend. Only the `bedrock` provider filters; other providers forward tool names unfiltered. - **memory:** annotate `_EMBEDDER_CACHE` as `dict[tuple[str, str, str], Embedder]` to match the 3-element key (backend, model, ollama_base_url). The stale 2-tuple annotation made `mypy headroom` fail on `main`, which broke the `lint` CI job on every open PR. diff --git a/headroom/compress.py b/headroom/compress.py index 0b967026b..ca9f0dc73 100644 --- a/headroom/compress.py +++ b/headroom/compress.py @@ -202,14 +202,17 @@ def compress( if not messages or not optimize: return CompressResult(messages=messages) - # Build config from explicit config + kwargs - cfg = config or CompressConfig() + # Build config from explicit config + kwargs. ``replace(config)`` up front + # so kwargs overrides and any savings-profile pass never mutate the + # caller's long-lived ``CompressConfig`` — a shared per-agent config being + # silently rewritten by every request that overrode a single option is the + # scenario this guards against. + cfg = replace(config) if config is not None else CompressConfig() config_fields = {f.name for f in cfg.__dataclass_fields__.values()} for key, value in kwargs.items(): if key in config_fields: setattr(cfg, key, value) if cfg.savings_profile: - cfg = replace(cfg) apply_agent_savings_profile(cfg, cfg.savings_profile) pipeline = _get_pipeline() diff --git a/tests/test_compress_api.py b/tests/test_compress_api.py index 7f9956b53..1e7f6f423 100644 --- a/tests/test_compress_api.py +++ b/tests/test_compress_api.py @@ -1,10 +1,11 @@ """Tests for the one-function compress() API and integrations.""" import json +from dataclasses import replace as _dc_replace import pytest -from headroom.compress import CompressResult, compress +from headroom.compress import CompressConfig, CompressResult, compress from headroom.hooks import CompressionHooks try: @@ -97,6 +98,39 @@ class TestCompressFunction: assert result.messages is messages assert result.tokens_saved == 0 + def test_kwargs_do_not_mutate_caller_config(self): + """kwargs must not smuggle their values onto the caller's CompressConfig. + + Regression: ``compress`` did ``cfg = config or CompressConfig()`` and + then ``setattr(cfg, key, value)`` for every matching kwarg — so a caller + who passed ``config=my_cfg, protect_recent=0`` came back to find their + long-lived ``my_cfg`` silently rewritten. A shared, per-agent config + was corrupted by every request that overrode a single option. + """ + + big_data = json.dumps([{"id": i, "status": "active"} for i in range(200)]) + messages = [ + {"role": "user", "content": "analyze"}, + {"role": "tool", "content": big_data, "tool_call_id": "c1"}, + ] + cfg = CompressConfig(protect_recent=4, target_ratio=0.8) + snapshot = _dc_replace(cfg) + + compress( + messages, + model="claude-sonnet-4-5-20250929", + config=cfg, + protect_recent=0, + target_ratio=0.2, + ) + + assert cfg.protect_recent == snapshot.protect_recent, ( + "compress() mutated caller's config.protect_recent via kwargs" + ) + assert cfg.target_ratio == snapshot.target_ratio, ( + "compress() mutated caller's config.target_ratio via kwargs" + ) + def test_with_custom_hooks(self): """Hooks are called when provided.""" calls = []