mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3e976712e7
|
fix(proxy/output-shaping): tolerate a non-string system block text in steering (#2435)
## Description
`apply_verbosity_steering` (the Anthropic output-shaping path) scans the
`system` block list to find and update an existing steering block:
```python
if isinstance(system, list):
for block in system:
if isinstance(block, dict) and block.get("text", "").startswith(_STEERING_SENTINEL):
```
`.get("text", "")` only substitutes the default when the key is
**absent**. A malformed client block with a null text (`{"type": "text",
"text": null}`) returns `None`, so `None.startswith(...)` raises
`AttributeError`. In the output-shaping treatment arm that call runs
inside `shape_request`, which is not individually guarded, so the
exception propagates and 502s the request.
The OpenAI chat sibling in the same module already defends against this
exact case (`isinstance(part.get("text"), str)`), so the Anthropic path
is the inconsistent one.
## Fix
Guard that the block text is a string before `startswith`, mirroring the
OpenAI sibling. Well-formed bodies are unchanged: the steering block is
still replaced idempotently when a level changes, or appended when
absent. The malformed block is left untouched.
## 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/output_steering.py`: string-guard the system block
text before `startswith` in `apply_verbosity_steering`.
- `tests/test_output_steering.py`: regression asserting a `system` list
containing a `{"text": null}` block does not crash and still appends the
steering block.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ python -m pytest tests/test_output_steering.py -q
9 passed
# with the fix reverted, the new test fails (AttributeError on None.startswith):
$ git stash push -- headroom/proxy/output_steering.py
$ python -m pytest "tests/test_output_steering.py::test_anthropic_steering_tolerates_non_string_system_block_text" -q
1 failed
$ uvx ruff@0.15.17 check headroom/proxy/output_steering.py tests/test_output_steering.py
All checks passed!
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/proxy/output_steering.py
Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, project venv (`uv sync --extra
proxy`), `uvx ruff@0.15.17` / `uvx mypy@1.20.2`, pytest in the venv.
- Exact command / steps: called the real `apply_verbosity_steering` with
`system=[{"type":"text","text":None},{"type":"text","text":"Real system
prompt."}]`; also confirmed the OpenAI sibling
`apply_openai_chat_verbosity_steering` handles the same shape.
- Observed result: pre-fix the Anthropic call raised `AttributeError:
'NoneType' object has no attribute 'startswith'` while the OpenAI
sibling returned True; post-fix the Anthropic call returns True, leaves
the malformed block as-is, appends the steering block, and stays
idempotent on a repeat. Ran against the actual module.
- Not tested: a live client that sends a null system block text end to
end.
## 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
|
||
|
|
1b8c11ebfb
|
fix(proxy/openai): apply output shaping on /v1/chat/completions (#2328)
## Description Fixes #2302. Output shaping (`HEADROOM_OUTPUT_SHAPER=1`) verbosity steering is wired into the Anthropic `/v1/messages` handler and the OpenAI `/v1/responses` handler, but never into `handle_openai_chat`. OpenAI-compatible clients that route through `/v1/chat/completions` — GitHub Copilot CLI, opencode, older SDKs — therefore got zero output savings, and `headroom output-savings` reported: ``` No shaped requests recorded yet. ``` `handle_openai_chat` referenced verbosity only for cache-key construction, never for actual shaping. The shared helpers (`OutputShaperSettings`, `resolve_verbosity_level`, `assign_arm`, `classify_turn`) existed but were not called from the chat path. ## Fix Run the same shaping block the Anthropic handler already uses, at the end of `handle_openai_chat` (after every other body mutation, before the upstream forward, skipped under `x-headroom-bypass`): - conversation-stable holdout via `assign_arm(conversation_key_from_body(body), holdout)` — `conversation_key_from_body` already reads `messages`, so it works unchanged for a chat body; - stratum labelling on the transforms channel so the outcome funnel feeds the output-savings ledger from the chat path; - for the treatment arm, verbosity steering via a new `shape_openai_chat_request`. The one genuinely new piece is a chat-specific steering injector. Anthropic carries the system prompt in a top-level `system` field and Responses in `instructions`; **chat/completions carries it as a `role: "system"` message inside `messages`**, which neither existing injector touches. `apply_openai_chat_verbosity_steering`: - appends the byte-stable steering block to the tail of the last `system`/`developer` message (idempotent via the `<headroom_output_shaping>` sentinel, and it swaps cleanly when the level changes); - handles both string content and the content-part list form (`[{"type": "text", ...}]`); - inserts a `role: "system"` message at the front only when the request has no system message. Because a whole conversation is stably treatment or control and the block text is fixed per level, a treatment conversation's steering is byte-stable across turns, so the provider prefix cache is not thrashed. Effort routing is intentionally not applied on this path — `route_effort` writes Anthropic-shaped `output_config`/thinking config with no portable chat/completions equivalent — so only the token-reducing verbosity lever runs. Mutating `body` in place is enough on this path; the outbound request serializes `body` fresh, so no body-mutation tracker is needed. ## 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/output_steering.py`: add `apply_openai_chat_verbosity_steering` (inject the steering block into the chat `messages` system prompt). - `headroom/proxy/output_shaper.py`: add `shape_openai_chat_request` (verbosity-only chat shaper) and export both new names. - `headroom/proxy/handlers/openai.py`: run the holdout/stratum + shaping block at the end of `handle_openai_chat`, mirroring the Anthropic handler and respecting bypass. - `tests/test_output_steering.py`: cover the injector (append, idempotency, level swap, insert-when-absent, list content, level-0 no-op). - `tests/test_output_shaper.py`: cover `shape_openai_chat_request` (disabled no-op, applies steering, level override, stable second pass). - `CHANGELOG.md`: Bug Fixes entry. ## Testing - [ ] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] 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/output_steering.py headroom/proxy/output_shaper.py headroom/proxy/handlers/openai.py tests/test_output_steering.py tests/test_output_shaper.py All checks passed! $ uvx ruff@0.15.17 format --check <same files> 5 files already formatted $ uvx mypy@1.20.2 --ignore-missing-imports headroom/proxy/output_steering.py headroom/proxy/output_shaper.py Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx mypy@1.20.2`. A full `pytest` OOMs this box (ML-stack import), so I reproduced the injector with a dependency-free script and left the full pytest to CI. - Exact command / steps: replicated `apply_openai_chat_verbosity_steering` (and the `steering_text`/`replace_or_append_steering_block` primitives it uses) and exercised: an existing string system message, an existing content-part list, no system message, re-apply at the same level, and a level swap. - Observed result: the steering block is appended to the system message while user turns and message order are untouched; re-applying at the same level is a no-op; a level change replaces the block (exactly one remains); a request with no system message gets one inserted at the front; level 0 is a no-op. The added unit tests assert the same through `shape_openai_chat_request`. - Not tested: a live Copilot CLI `/v1/chat/completions` round trip; the added tests drive the pure shaper/injector directly, matching the existing `test_output_shaper.py` / `test_output_steering.py` patterns. ## 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" box is unchecked because a local pytest run imports the ML stack and OOMs this box; the added tests are pure (no ML imports) and run under the normal CI pytest job, and the injector behavior is corroborated by the standalone proof above. Effort routing on chat/completions is deliberately out of scope here (no portable equivalent to the Anthropic effort levers); this PR restores the verbosity-steering savings the issue reports as missing, and effort routing for chat can follow separately if wanted. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
|
0ce09fb63f
|
refactor(output): isolate verbosity steering (#1940)
## Description Extract byte-stable output verbosity steering into `headroom.proxy.output_steering` so `output_shaper` can focus on turn classification and effort routing while preserving the existing public import surface. 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 - [x] Code refactoring (no functional changes) ## Changes Made - Added `headroom.proxy.output_steering` for Anthropic system steering and OpenAI Responses instruction steering. - Kept existing `headroom.proxy.output_shaper` imports compatible by re-exporting the moved helpers. - Added direct tests for replacement, cache-prefix preservation, and idempotent OpenAI Responses steering. - Included the LiteLLM callback hook compatibility shim needed for repo-wide mypy on branches based on `main`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text python -m pytest tests/test_output_steering.py tests/test_output_shaper.py tests/test_litellm_callback.py tests/test_compress_api.py::TestLiteLLMCallback -q 57 passed in 6.17s python -m ruff check . All checks passed! python -m ruff format --check . 1095 files already formatted python -m mypy headroom --ignore-missing-imports Success: no issues found in 409 source files ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13, local worktree `C:\git\headroom-pr-slice7`. - Exact command / steps: Ran the focused pytest suite plus repo-wide Ruff, format check, and mypy commands listed above. - Observed result: Steering behavior remains covered through the existing `output_shaper` tests and the new direct `output_steering` tests. - Not tested: Full test suite locally; CI will run the full matrix. ## 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 and changelog updates are not applicable for this internal refactor. The LiteLLM shim is repeated here because this branch is intentionally independent from the other open architecture slices and must stay green against current `main`. |