Commit graph

3 commits

Author SHA1 Message Date
Abhay Singh
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
2026-07-22 06:16:20 -07:00
Abhay Singh
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>
2026-07-17 16:16:29 -07:00
JD Davis
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`.
2026-07-10 17:39:39 -05:00