mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
2 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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> |
||
|
Rod Boev
|
51d4bcfc11
|
fix(proxy): add --protect-tool-results to prevent lossy compression of exact-output Bash results (#1374)
## Description Bash tool outputs that contain exact reference data (grep results, cat output, ls listings) are lossy-compressed by SmartCrusher because Bash is intentionally absent from `DEFAULT_EXCLUDE_TOOLS`. The agent re-reads these compressed results and acts on fabricated content, producing corrupt edits and wrong reasoning with no visible error. This PR adds `--protect-tool-results` / `HEADROOM_PROTECT_TOOL_RESULTS` as a comma-separated list of tool names whose results must never be lossy-compressed. Named tools are merged into the exclude set before ContentRouter processes the conversation. The default is empty; existing behavior is unchanged unless the user opts in. Closes #1307 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) ## Changes Made - `headroom/proxy/models.py`: add `protect_tool_results: frozenset[str]` field to `ProxyConfig` - `headroom/proxy/server.py`: merge `protect_tool_results` into `router_config.exclude_tools` in the router config block; add `--protect-tool-results` argparse argument; wire to `ProxyConfig` - `headroom/cli/proxy.py`: add `--protect-tool-results` Click option with `envvar="HEADROOM_PROTECT_TOOL_RESULTS"`; wire to `ProxyConfig` - `headroom/config.py`: extend comment block to document the escape hatch - `CHANGELOG.md`: bug fix entry - `tests/test_content_router_exclude_tools.py`: focused tests for merge behavior, env var parsing, and lossless passthrough of a protected Bash tool_result ## Testing - [x] Unit tests pass (`uv run pytest tests/test_content_router_exclude_tools.py -v`) - [x] Linting passes (`uv run ruff check .`) - [ ] Type checking passes (`uv run mypy headroom`) — N/A: repo does not enforce mypy in CI - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text # paste actual pytest -v output here after running ``` ## Real Behavior Proof - Environment: headroom proxy with `HEADROOM_PROTECT_TOOL_RESULTS=Bash` - Exact command / steps: Agent issues `Bash(command="grep -n 'class Foo' src/main.py")`, proxy proxies the response; inspect ContentRouter routing decision in debug logs - Observed result: Bash tool_result block is present verbatim in the compressed output; SmartCrusher skips it; agent reads the correct line numbers - Not tested: multi-worker scenarios; per-tool age-decay granularity (when `protect_tool_results` is set in token mode, age-decay is disabled for all excluded tools, not just the protected ones, because ContentRouter lacks per-tool windowing) ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes When `protect_tool_results` is set, `protect_recent_reads_fraction` is forced to `0.0` so that token-mode age-decay never compresses protected tool results regardless of conversation depth. A dedicated `_parse_csv_tools` helper parses the CSV without merging `HEADROOM_EXCLUDE_TOOLS`, preventing cross-contamination between the two config surfaces. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com> |