Commit graph

3 commits

Author SHA1 Message Date
Abhay Singh
def2f9a728
fix(learn): don't desync verbosity pairing on empty assistant turns (#2123)
## Description

`verbosity._ordered_events` and `_parse_session` disagree about empty
assistant turns, which desyncs the response list and produces spurious
fast-skips.

`_parse_session` only creates a `_Response` when an assistant message
actually said something:

```python
if words > 0 or out_tok > 0:
    responses.append(_Response(...))
```

But `_ordered_events` consumes one `responses[ri]` for **every**
assistant line, with no matching filter:

```python
if ltype == "assistant" and ri < len(responses):
    out.append((responses[ri].ts, "assistant", responses[ri]))
    ri += 1
```

So an assistant turn with no text and no output tokens — for example a
pure `tool_use` turn where `usage` is absent — creates no `_Response` at
parse time, yet still consumes a slot in `_ordered_events`. That slot
actually belongs to a *later* real response, so the two lists drift by
one. A human reply that follows the real answer is then paired with the
next answer's (future) timestamp, `ts - last_resp.ts` goes negative, and
since a negative gap is always below the read-fraction threshold, a
spurious `fast_skip` is recorded. That inflates `fast_skip_rate`, which
feeds `pressure`, which lowers the recommended verbosity level.

The user side of `_ordered_events` already replicates its parse-site
filter (`_human_text(...) is None -> continue`); only the assistant side
was missing the equivalent guard. That asymmetry is the bug.

## Fix

In `_ordered_events`, compute `words`/`out_tok` for the assistant line
the same way `_parse_session` does and only consume a response when
`words > 0 or out_tok > 0`, keeping the two functions in lockstep.

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/verbosity.py`: `_ordered_events` applies the `words >
0 or out_tok > 0` guard on the assistant branch before consuming a
response, with a comment explaining the desync.
- `tests/test_verbosity_learn.py`: add `_empty_assistant` helper and
`test_empty_assistant_message_does_not_desync_fast_skip` (an empty
assistant turn before a real answer + a slow reply must not record a
fast skip).
- `CHANGELOG.md`: Bug Fixes entry.

## Testing

- [x] Unit tests pass (`uv run --extra dev pytest
tests/test_verbosity_learn.py::TestSignalExtraction::test_empty_assistant_message_does_not_desync_fast_skip
-q`)
- [x] Linting passes (`uvx ruff@0.15.17 check
headroom/learn/verbosity.py tests/test_verbosity_learn.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/learn/verbosity.py tests/test_verbosity_learn.py
All checks passed!
$ python -m py_compile headroom/learn/verbosity.py tests/test_verbosity_learn.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 alignment with a
dependency-free script that models the parse-site filter, the old vs new
`_ordered_events` consume, and the resulting human-to-response pairing,
and left the full pytest to CI.
- Exact command / steps: built an event stream `[empty assistant, real
answer #1, fast human reply, real answer #2, reply]`, computed the
response list from the parse filter, then walked the old (unfiltered)
and new (filtered) consume to find the gap between the first human and
the response paired before it.
- Observed result: old consume pairs the reply with answer #2 (a future
timestamp) -> gap `-8` (spurious fast_skip); new consume keeps alignment
and pairs it with answer #1 -> gap `+1`. The new test builds a session
with an empty assistant turn and a genuinely slow reply and asserts
`fast_skips == 0`.
- Not tested: a real Claude Code transcript 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

Merged current `main` to pick up the repository-wide mypy cache-key
annotation fix, then verified the focused regression locally. the change
adds the existing parse-site filter to one branch of a pure file-parsing
function, verified by the standalone alignment proof and the new
regression test for CI.

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-07-14 12:04:32 -04:00
Rod Boev
e3b45e402b
fix(learn): handle Windows UTF-8, drive-letter paths, and CLI shim fallback (#1895)
## Description

`headroom learn --verbosity` is broken on Windows in three related ways:

- Transcript/profile reads can use the platform default codec, so
non-ASCII content can raise `UnicodeDecodeError` and collapse learning
signals to empty output.
- `--project <path>` can miss real Claude project directories because
Windows profile junctions can raise `PermissionError` during directory
walks, and escaped Claude project folder names cannot always distinguish
`vibe-remote` from `vibe\remote`.
- `headroom learn --agent codex` can fail with `` `claude` not found in
PATH `` even when the npm-installed CLI exists, because Windows `.cmd`
shims require `PATHEXT` resolution.

Refs https://github.com/headroomlabs-ai/headroom/issues/1624 for the
Windows learn failures. The dashboard-hint UX and
third-party-provider-auth items in that issue are unrelated and out of
scope for this PR.

## 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/verbosity.py`: read and write verbosity
transcripts/profiles with `encoding="utf-8"` so non-ASCII content works
regardless of the Windows locale codec.
- `headroom/learn/plugins/claude.py`: skip inaccessible siblings one
entry at a time during greedy project path decoding, so one Windows
junction no longer hides valid project directories.
- `headroom/learn/plugins/claude.py`: prefer a valid `cwd` found in
Claude session JSONL when discovering project paths, which resolves
ambiguous escaped folder names such as `vibe-remote` versus
`vibe\remote`.
- `headroom/learn/analyzer.py`: resolve Windows CLI shim paths through
`shutil.which()` after `FileNotFoundError`, then retry once for
streaming and non-streaming CLI calls.
- `CHANGELOG.md`: document the Windows learn fixes under `Unreleased`.

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_verbosity_learn.py::TestWindowsEncoding
tests/test_learn/test_scanner.py::TestGreedyPathDecode::test_permission_denied_sibling_does_not_abort_the_walk
tests/test_learn/test_analyzer.py::TestWindowsCliShimFallback
tests/test_learn/test_scanner.py::TestDecodeProjectPath::test_discover_project_prefers_session_cwd_over_ambiguous_folder_name
-q`)
- [x] Linting passes (`uv run ruff check
headroom/learn/plugins/claude.py tests/test_learn/test_scanner.py`)
- [x] Formatting passes (`uv run ruff format --check
headroom/learn/plugins/claude.py tests/test_learn/test_scanner.py`)
- [ ] Type checking passes (`uv run mypy headroom`) not run; no new
public type surface
- [x] New tests added for the Windows `cwd` disambiguation regression
- [x] Manual testing performed

### Test Output

```text
uv run ruff format headroom/learn/plugins/claude.py
1 file reformatted

uv run ruff check headroom/learn/plugins/claude.py tests/test_learn/test_scanner.py
All checks passed!

uv run ruff format --check headroom/learn/plugins/claude.py tests/test_learn/test_scanner.py
2 files already formatted

uv run pytest tests/test_verbosity_learn.py::TestWindowsEncoding tests/test_learn/test_scanner.py::TestGreedyPathDecode::test_permission_denied_sibling_does_not_abort_the_walk tests/test_learn/test_analyzer.py::TestWindowsCliShimFallback tests/test_learn/test_scanner.py::TestDecodeProjectPath::test_discover_project_prefers_session_cwd_over_ambiguous_folder_name -q
9 passed in 0.25s
```

CI on current head `c6dbac40` is green. A prior `test (1)` run hit an
unrelated timing-sensitive scheduler assertion; GitHub did not permit
direct rerun without admin rights, so the empty commit `c6dbac40`
retriggered CI and the shard passed.

## Real Behavior Proof

- Environment: Windows 11, Python 3.12, real filesystem for the
path-decoding reproduction.
- Exact command / steps: Ran `uv run pytest
tests/test_verbosity_learn.py::TestWindowsEncoding
tests/test_learn/test_scanner.py::TestGreedyPathDecode::test_permission_denied_sibling_does_not_abort_the_walk
tests/test_learn/test_analyzer.py::TestWindowsCliShimFallback
tests/test_learn/test_scanner.py::TestDecodeProjectPath::test_discover_project_prefers_session_cwd_over_ambiguous_folder_name
-q`, `uv run ruff check headroom/learn/plugins/claude.py
tests/test_learn/test_scanner.py`, and `uv run ruff format --check
headroom/learn/plugins/claude.py tests/test_learn/test_scanner.py`; the
path tests create real Windows-style project directories, inaccessible
siblings, ambiguous `vibe\remote` versus `vibe-remote` folders, and
Claude session JSONL with `cwd` pointing at the intended project.
- Observed result: The focused test command returned `9 passed in
0.25s`, Ruff check passed, and Ruff format check passed. The decoder
skips the inaccessible sibling and reaches `vibe-remote`; the
session-`cwd` test returns `vibe-remote` instead of trusting the
ambiguous escaped folder name; the UTF-8 tests round-trip non-ASCII
transcript/profile content under a non-UTF-8 Windows-style codec; the
CLI shim tests retry once through `shutil.which()` after
`FileNotFoundError`.
- Not tested: real npm-installed `claude`/`codex` CLI shims,
dashboard-hint UX, and third-party-provider auth.

## Review Readiness

- [x] I have performed a self-review
- [x] Retrospective review completed after opening; it found one missing
`cwd` disambiguation case, now fixed in this PR
- [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

- The post-open retrospective review concluded this needed targeted
rework rather than only a retrospective sign-off.
- The `cwd` recovery commit is `ee720bb1`; `0905be7b` contains the
required formatter cleanup; current head `c6dbac40` is an empty CI-rerun
commit after GitHub denied direct rerun without admin rights.

---------

Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-07-09 12:49:38 -05:00
Tejas Chopra
a99dc61424
feat: output-token reduction — verbosity shaper, per-user learning, counterfactual savings (#965)
## Description

Adds the first levers that reduce the tokens the model **writes back**
(output), complementing Headroom's existing input compression. Output
costs 5× input on Opus-class models and is full of waste (ceremony,
restated code, deep "thinking" on routine steps). Two phases in one
self-contained PR off `main`: the request-side output shaper, then
per-user verbosity learning plus an honest counterfactual savings
estimator and dashboard surfacing.

## 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)
- [x] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- **Output shaper** (`output_shaper.py`, opt-in
`HEADROOM_OUTPUT_SHAPER=1`): cache-safe verbosity steering appended to
the system-prompt tail (5 levels); effort routing that lowers
`output_config.effort` on mechanical tool-result continuations; legacy
`thinking.budget_tokens` clamp. Never injects effort where absent, never
toggles `thinking.type`.
- **`headroom learn --verbosity`**: mines Claude Code transcripts for
behavioral signals (interrupts, length-adaptive fast-skips, echo ratio),
recommends a verbosity level (heuristic + optional `--llm-judge`), and
seeds the savings baseline.
- **Counterfactual estimator** (`output_savings.py`): per-stratum
synthetic-control (estimated) + A/B holdout (measured) with a propagated
95% CI; conversation-stable arm assignment for A/B validity and
prefix-cache safety.
- **AIMD verbosity controller** (`verbosity_controller.py`):
additive-increase / fast-back-off state machine; live signal emission
gated off by default.
- **Wiring + surfaces**: shaper resolves the learned level; recording
rides the existing `transforms_applied` channel through the outcome
funnel (no `RequestOutcome` changes); `headroom output-savings` CLI;
dashboard "Output Tokens Saved" card.
- **Docs**: simple-words user guide + design doc with the counterfactual
methodology.

## 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
$ pytest tests/test_output_savings.py tests/test_output_savings_cli.py \
        tests/test_verbosity_learn.py tests/test_verbosity_controller.py \
        tests/test_output_shaper.py -q
94 passed in 0.54s

$ pytest tests/test_request_outcome.py tests/test_handler_outcome_tag_invariant.py \
        tests/test_proxy_dashboard_stats_cache.py -q
44 passed

$ ruff format --check .
831 files already formatted

$ mypy headroom --ignore-missing-imports
Success: no issues found in 361 source files
```

## Real Behavior Proof

- Environment: macOS, Python 3.12 (`.venv`), `anthropic` 0.76, live API
model `claude-opus-4-8`.
- Exact command / steps: `HEADROOM_OUTPUT_SHAPER=1`; `headroom learn
--verbosity --apply` (seeds level + baseline); `python
scripts/eval_output_shaper.py A` (live before/after); simulate holdout
traffic then `headroom output-savings`.
- Observed result: code-review ask — baseline 1,750 output tokens → L2
1,354 (−22.7%) → L3 599 (−65.8%), same bugs found. `learn --verbosity`
on 24 real sessions → 11% interrupt / 26% fast-skip → L3 (high
confidence). Measured A/B path → 31.7% reduction (95% CI 27.7%–35.7%).
94 new tests + 44 existing outcome/dashboard tests green; ruff + mypy
clean.
- Not tested: live streaming-path recording exercised only via unit
tests (the `transforms_applied` funnel is shared across paths); runtime
AIMD signal emission is gated off by default and not exercised live.

## 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
- [ ] I have updated the CHANGELOG.md if applicable

## Additional Notes

Output savings are counterfactual (we never observe what the model
*would* have written), so the estimator separates **estimated** (vs a
learned baseline) from **measured** (A/B holdout via
`HEADROOM_OUTPUT_HOLDOUT`) and always reports a confidence band — never
a single made-up number. CHANGELOG left unchecked (release-please
manages it). Runtime AIMD self-tuning is intentionally a TODO
(controller built/tested; live signal emission gated behind
`HEADROOM_VERBOSITY_AUTOTUNE`).
2026-06-16 21:06:43 -07:00