mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
6cdfd3f64d
## Description Addresses the chat/completions portion of #2060. The live traffic learner is wired into the Anthropic `/v1/messages` handler and, since then, the OpenAI Responses HTTP handler (`_observe_openai_responses_traffic`, called from `handle_openai_responses`). But `handle_openai_chat` has **no** ingestion call site: ```text headroom/proxy/handlers/openai.py handle_openai_responses -> _observe_openai_responses_traffic (wired) handle_openai_chat -> (no traffic_learner call) (gap) ``` So OpenAI-compatible clients that route through `/v1/chat/completions` — GitHub Copilot CLI, opencode, OpenAI SDKs — run through an apparently healthy proxy with Learn enabled while producing no learned patterns: the learner starts, but it never receives their tool results or user messages. ## Fix Observe the original client payload (before memory/compression mutates it) at the top of `handle_openai_chat`, mirroring the Responses and Anthropic ingestion paths: ```python await self._observe_openai_chat_traffic(original_client_messages, request_id=request_id) ``` `_observe_openai_chat_traffic` is the chat counterpart of `_observe_openai_responses_traffic`: same lazy backend wiring, same `on_tool_result` / `on_messages` lifecycle, same fail-soft `try/except`. The one format-specific piece is tool-result extraction. chat/completions encodes tool calls differently from Anthropic — the call is on an assistant message's `tool_calls` array (`id` -> function `name` + `arguments`) and each result is a separate `role: "tool"` message keyed by `tool_call_id`, so the existing `extract_tool_results_from_messages` (which scans for Anthropic `tool_use`/`tool_result` blocks) finds nothing. A new `TrafficLearner.extract_tool_results_from_openai_messages`: - builds the `tool_call_id -> function` map from assistant `tool_calls`; - for each `role: "tool"` message, resolves the tool name and joins string-or-list content; - parses the OpenAI `arguments` JSON string into a dict, so the downstream environment/recovery extractors (which call `input.get("command")`, `input.get("file_path")`, ...) see the same shape as an Anthropic `tool_use.input` instead of a raw string; - sniffs `is_error` from the output (chat tool messages carry no error flag). It returns the same `{tool_name, input, output, is_error}` shape as the Anthropic extractor, so `on_tool_result` stays format-agnostic. User-message preference extraction (`on_messages`) already reads plain `role`/`content`, so it consumes chat messages unchanged. Scope: this wires the **chat/completions** path. Codex WebSocket ingestion (`handle_openai_responses_ws`) additionally needs per-`response.create` evaluation plus transcript-replay baselining on reconnect, so it is intentionally left as a follow-up rather than half-implemented here. ## 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`: add `extract_tool_results_from_openai_messages` (OpenAI chat tool-result extraction with `arguments` JSON parsed to a dict). - `headroom/proxy/handlers/openai.py`: add `_observe_openai_chat_traffic` and call it from `handle_openai_chat` on the original client payload. - `tests/test_memory/test_traffic_learner.py`: cover the OpenAI extractor (name resolution, arguments parsing, list content, error sniff, malformed/orphan handling, empty case). - `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_memory/test_traffic_learner.py All checks passed! $ uvx ruff@0.15.17 format --check <same files> 3 files already formatted $ uvx mypy@1.20.2 --ignore-missing-imports headroom/memory/traffic_learner.py # clean for this file (the one reported error is a pre-existing # headroom/_subprocess.py:18 no-any-return, unrelated to this change and # 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 extractor with a dependency-free script and left the full pytest to CI. - Exact command / steps: replicated `extract_tool_results_from_openai_messages` and ran it over a typical chat round-trip (assistant `tool_calls` for `bash` + `read_file`, then two `role: "tool"` results, one erroring and one with list content), plus malformed-`arguments`, orphan-`tool_call_id`, and no-tool cases. - Observed result: tool names resolved from the call-id map; `arguments` parsed to a dict so `input.get("command")` works; list content joined; `is_error` sniffed from output; malformed arguments degrade to `{}` and an orphan id yields `unknown` without raising. The added unit tests assert the same through a real `TrafficLearner`. - Not tested: a live Copilot CLI session end to end; the added tests drive `TrafficLearner.extract_tool_results_from_openai_messages` directly, matching the existing `test_extract_tool_results_from_messages` pattern. ## 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 `TrafficLearner(backend=None, ...)` harness in `test_traffic_learner.py` (no real backend) and run under the normal CI pytest job, and the extractor behavior is corroborated by the standalone proof above. This PR is deliberately scoped to `/v1/chat/completions`; I'm happy to follow up with the Codex WebSocket ingestion path (which needs the transcript-replay baselining discussed in the issue) as a separate change if useful. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|---|---|---|
| .. | ||
| __init__.py | ||
| conftest.py | ||
| test_budget.py | ||
| test_core_operations.py | ||
| test_easy.py | ||
| test_embedder_mps_serialization.py | ||
| test_embedder_thread_cap.py | ||
| test_extraction.py | ||
| test_factory.py | ||
| test_factory_embedder_cache.py | ||
| test_factory_external.py | ||
| test_hierarchical.py | ||
| test_hnsw_batch_capacity.py | ||
| test_learn_flag.py | ||
| test_local_backend_search.py | ||
| test_mcp_server.py | ||
| test_qdrant_env.py | ||
| test_query_conditions.py | ||
| test_skip_helpers.py | ||
| test_supersession_repair.py | ||
| test_traffic_learner.py | ||
| test_writers.py | ||