Commit graph

3 commits

Author SHA1 Message Date
Abhay Singh
f669149769
fix(proxy/openai): feed Codex WS traffic into the traffic learner (#2334)
## Description

Follow-up to the chat/completions ingestion work — this wires the Codex
`/v1/responses` **WebSocket** path into the traffic learner, the
remaining gap in #2060.

`handle_openai_responses_ws` (the transport newer Codex versions default
to) had no traffic-learner ingestion, so Codex subscription traffic
produced no learned patterns even with Learn enabled. Unlike the
one-shot HTTP path, a long-lived Codex WebSocket:

- resends the **full transcript** on every `response.create` frame, and
- replays it **wholesale on reconnect/resume**.

So naive per-turn ingestion would count the same tool result as evidence
over and over, and every reconnect would re-ingest the whole history.

## Fix

Add `_observe_openai_ws_response_create`, which dedups per connection by
tool-call id:

- A per-connection `ws_learner_seen_call_ids: set[str]` tracks which
tool-call ids have been observed on this WebSocket.
- The **first** `response.create` frame is a **baseline**: its
already-present transcript is recorded as seen but **not learned**, and
preference extraction is skipped. This is the replayed/initial history,
which may already have been learned on a prior connection.
- **Later** frames learn only the tool results whose call id first
appears after the baseline, then mark them seen. Preference extraction
(`on_messages`) runs on these frames (it already looks only at the most
recent messages).

On reconnect the client opens a fresh WebSocket and replays the
transcript in its first frame, which is baselined again, so it adds no
spurious evidence. It hooks both frame paths: the first-frame handler
seeds the baseline from the original client frame (parsed before memory
injection / compression), and `_maybe_compress_response_create_frame`
observes each subsequent frame.

To dedup by identity,
`TrafficLearner.extract_tool_results_from_messages` now also returns the
`call_id` (the `tool_use`/`tool_result` id, which
`_responses_input_to_learner_messages` already sets from the Responses
`call_id`). This is additive — existing callers that don't read it are
unaffected.

Relationship to the chat path: the `/v1/chat/completions` ingestion is a
separate change; together they cover HTTP chat, HTTP Responses (already
wired), and Codex WS. This PR is independent and branches off `main`.

## 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/memory/traffic_learner.py`:
`extract_tool_results_from_messages` now returns `call_id` for per-turn
dedup (additive).
- `headroom/proxy/handlers/openai.py`: add
`_observe_openai_ws_response_create` (per-connection dedup + baseline);
initialise `ws_learner_seen_call_ids`; observe the first frame as a
baseline and each subsequent `response.create` frame.
- `tests/test_openai_responses_traffic_learner.py`: add WS
dedup/baseline coverage (baseline records-not-learns, later frames learn
only new results, reconnect replay adds no evidence); update the
existing extractor-equality assertion to include `call_id`.
- `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/memory/traffic_learner.py headroom/proxy/handlers/openai.py tests/test_openai_responses_traffic_learner.py
All checks passed!
$ uvx ruff@0.15.17 format --check <same files + test_memory/test_traffic_learner.py>
all files already formatted
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/proxy/handlers/openai.py
# no errors in the changed files (the one reported error is a pre-existing
# headroom/_subprocess.py:18 no-any-return, present on main with these edits stashed)
```

## 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 dedup/baseline loop with a dependency-free asyncio script
and left the full pytest to CI.
- Exact command / steps: simulated a connection where the baseline frame
carries tool-call ids A,B; later frames replay A,B and append C, then D;
plus a reconnect whose first frame replays A,B,C,D.
- Observed result: baseline recorded A,B without learning; frame 2
learned only C; frame 3 learned only D (A/B/C never re-counted); the
reconnect's replayed transcript was baselined and learned nothing. The
added unit tests assert the same through the real handler method with a
recording learner.
- Not tested: a live Codex WebSocket session end to end; the added tests
drive `_observe_openai_ws_response_create` directly with a recording
learner and the real `_responses_input_to_learner_messages` + extractor.

## 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 reuse the
existing `_RecordingLearner` harness (no real backend) and run under the
normal CI pytest job, and the dedup/baseline behavior is corroborated by
the standalone proof above. Design note: baselining the first frame
means a brand-new conversation's first-turn tool results are not learned
on that connection (subsequent turns are); this is the deliberate
trade-off the issue calls for to keep reconnect/resume from inflating
evidence.

---------

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-08-11 23:54:41 -05:00
Chester
3eb0122068
fix(learn): filter ambient user-role scaffolding (#2275)
## Description

Fixes #2274.

Headroom Learn currently trusts `role=user` as sufficient preference
provenance. Agent harnesses can transport ambient UI and orchestration
context in user-role messages, and OpenAI Responses normalization also
promotes missing roles to `user`. Correction-like text in those inputs
can therefore become durable user preferences.

This change keeps preference learning fail-closed for known non-user
sources while preserving genuine user corrections.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Refactoring only

## Changes Made

- Preserve missing OpenAI Responses roles as `unknown` instead of
promoting them to `user`.
- Canonicalize user-role text before preference extraction.
- Remove proxy-appended `## Relevant Memories` suffixes from preference
evidence.
- Reject strict ambient-only harness prefixes such as heartbeat,
environment, workspace-instruction, delegation, and app-context
envelopes.
- Apply the same guard in `on_messages` and `_extract_preferences` for
defense in depth.
- Add regression coverage for system/developer/unknown roles,
ambient-only user messages, memory-only messages, and mixed
genuine-user-plus-memory input.

## 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
149 passed, 1 warning
ruff check: passed
ruff format --check: passed
git diff --check: passed
```

Focused test files:

```text
tests/test_memory/test_traffic_learner.py
tests/test_openai_responses_traffic_learner.py
```

## Real Behavior Proof

- Environment: macOS; Python 3.13; current Headroom main; direct
invocation of the real `TrafficLearner` class, with no proxy or database
mocks
- Exact command / steps: create `TrafficLearner(backend=None,
min_evidence=1)`; feed system, developer, heartbeat user-role, and
memory-only user-role messages; read `patterns_extracted`; feed a
genuine user correction followed by a `## Relevant Memories` suffix;
read `patterns_extracted` again
- Observed result: `ambient_patterns=0`, `after_user_patterns=1` — the
ambient batch produced no preference evidence; the genuine correction
produced one pattern, while the appended memory content did not become
evidence
- Not tested: live provider traffic against a remote OpenAI endpoint;
every possible third-party harness envelope; migration or cleanup of
already-persisted noisy memories

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] No new dependency
- [x] Fail-open proxy behavior is unchanged
- [x] Regression tests added
- [x] Public examples contain no real user data
- [x] CHANGELOG update, if requested (not requested — N/A)

## Additional Notes

This extends the source filtering introduced by #466 rather than
replacing it. The prefix checks are deliberately strict and anchored at
the start of a canonicalized message. The intended failure mode is a
missed preference, not durable storage of non-user instructions.

Note: the strict prefix set was discussed and confirmed in
JerrettDavis's review approvals.
2026-08-02 19:40:14 -07:00
Chester
ce141301f1
fix(learn): ingest OpenAI Responses HTTP traffic (#2167)
## Description

OpenAI Responses HTTP requests currently bypass `TrafficLearner`, so
Learn can be enabled and healthy while receiving no preference or
tool-result evidence from this transport.

This draft adds the first, intentionally narrow part of #2060: HTTP
ingestion only. Codex WebSocket per-turn ingestion and transcript
baselining remain separate follow-ups.

Part of #2060.

## 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

- Normalize Responses `message`, `function_call`, and tool-output items
into the message and tool-result shape already understood by
`TrafficLearner`.
- Observe the original client payload before memory injection or
compression mutates it.
- Reuse the existing lazy memory-backend wiring and recent-tool-result
limit from the Anthropic path.
- Keep ingestion fail-open so learner failures never block proxy
traffic.
- Add focused normalization and real HTTP-handler regression tests.

## Testing

- [x] Focused unit tests pass
- [x] Linting passes (`ruff check` on changed files)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual focused test execution performed

### Test Output

```text
uvx ruff check headroom/proxy/handlers/openai.py tests/test_openai_responses_traffic_learner.py
All checks passed!

uvx ruff format --check headroom/proxy/handlers/openai.py tests/test_openai_responses_traffic_learner.py
2 files already formatted

Focused source-checkout execution:
2 focused tests passed

GitHub CI:
All test shards, lint, builds, security checks, and E2E jobs passed
```

## Real Behavior Proof

- Environment: macOS, Python 3.13, in-process FastAPI test client with a
fake OpenAI Responses upstream and a recording learner.
- Exact command / steps: execute both focused test functions in
`tests/test_openai_responses_traffic_learner.py`; the handler test posts
a payload containing one user message, one `function_call`, and its
failed `function_call_output` to `/v1/responses`.
- Observed result: the HTTP response remained successful, the learner
received exactly one normalized message batch, and it received the
matched failed tool result with parsed arguments.
- Not tested: live OpenAI traffic, Codex WebSocket ingestion, or
replay/transcript baselining. The complete GitHub CI test matrix passes.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented the provider-boundary normalization and ingestion
behavior
- [ ] Documentation changes are not included because this is an internal
transport wiring fix
- [x] I have added tests that prove the new ingestion path
- [x] Focused tests pass locally and the complete GitHub CI test matrix
passes
- [ ] CHANGELOG update is not included because release notes are
generated from conventional commits

## Screenshots (if applicable)

Not applicable.

## Additional Notes

This PR intentionally excludes Codex WebSocket ingestion,
replay/transcript baselining, and scaffolding/noise filters. Keeping
those separate avoids coupling transport lifecycle semantics to the
basic HTTP parity fix. Maintainer feedback on whether provider-boundary
normalization is the preferred ownership layer is welcome.
2026-07-14 11:52:54 -04:00