mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
56 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Parideboy
|
2d1e96b85c
|
fix(memory): sanitize entity_refs to prevent dict-shaped entries crashing search (#2951)
## Description Closes #2947 `entity_refs` is annotated `list[str]` everywhere, but nothing enforced that at runtime. `LocalBackend.save_memory`'s `entities` argument is filled straight from LLM-supplied `memory_save` tool input (`headroom/memory/system.py:575` into `memory_handler.py:1242`), so a caller can pass the typed `{"entity": ..., "entity_type": ...}` shape, which is the format `extracted_entities` expects, into it by mistake. Those dicts were then persisted verbatim into `entity_refs`, both in the `memories` table and in the duplicated copy the vector index keeps for post-filtering. Every later `search_memories` call does `set().update(memory.entity_refs)` while collecting entities for graph expansion. Hashing a dict raises `TypeError: unhashable type: 'dict'`, and because that happens inside the vector-result loop rather than per-item, **one** poisoned row aborted the **entire** search. The proxy's memory handler catches the exception and returns no memories, so recall went quietly dark rather than failing loudly, and the bad row kept re-appearing in top-k for related queries, so it stayed dark. The issue reporter hit this in production: 4 bad rows disabled memory search for a whole project for a day, with nothing visible to the end user beyond a swallowed warning in `proxy.log`. The same root cause has two more crash modes, both confirmed below: `AttributeError: 'dict' object has no attribute 'lower'` during graph linking on the save path, and the same error in the `entities` search filter (`ref.lower()`). The fix adds one helper and applies it at both ends of the data flow. Dicts are **unwrapped to their `entity` name** rather than dropped, so rows that are already corrupted keep contributing to graph expansion instead of silently losing their entities. ## 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 - **New helper `normalize_entity_refs()` in `headroom/memory/models.py`.** Coerces a raw entity-reference list into the `list[str]` it claims to be: strings pass through, dicts are unwrapped via their `entity` (or `name`) key, and anything with no recoverable name is dropped rather than stringified, since a ref like `"{'entity_type': 'project'}"` would only pollute the graph. Order is preserved and duplicate names are collapsed. - **Write path, to stop new corruption at the door.** `LocalBackend.save_memory` normalizes `entities` before it reaches `entity_refs` and graph linking. `LocalBackend.search_memories` normalizes the `entities` *filter* argument too, since it arrives from the same untrusted tool input (`memory_handler.py:1320`). - **Read path, to heal rows that were written before this fix.** Applied at the three deserialization boundaries, so no data migration is needed and corrupted rows normalize themselves the next time they are loaded: `Memory.from_dict` (`headroom/memory/models.py`), `SQLiteMemoryStore._row_to_memory` (`headroom/memory/adapters/sqlite.py`), and the vector indexes' own `entity_refs` copies used for post-filtering, `VectorMetadata.from_json` (`headroom/memory/adapters/sqlite_vector.py`) and `IndexedMemoryMetadata.from_dict` (`headroom/memory/adapters/hnsw.py`). - **Defensive normalization on emitted results.** `search_memories` and `text_search` normalize the refs they return as `related_entities`, so a backend that produces `Memory` objects by some path not covered above still cannot take a whole query down, and callers never receive a dict where they expect an entity name. **Note on scope versus the patch proposed in the issue.** The issue proposed normalizing in two places (`save_memory` plus the `set().update()` line). I widened it slightly because that pair leaves three related failures live: the `entities` filter still crashes on `ref.lower()`, `related_entities` still hands dicts back to the caller, and, most importantly, already-poisoned rows stay poisoned in storage. Normalizing at the deserialization boundaries fixes all three at once and is what makes existing corrupted databases recover on their own. **Behavior change worth flagging.** `entity_refs` is now de-duplicated (case-sensitively) on both save and load. Refs were already treated as a set for graph expansion, so this is semantically a no-op, but it is a visible difference if anything asserts on exact list contents. ## 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 New file `tests/test_memory/test_entity_ref_sanitization.py` adds 10 tests covering the helper, both write paths, all three deserialization boundaries, and the three crash modes. ### Test Output ```text $ python -m pytest tests/test_memory/test_entity_ref_sanitization.py -q .......... [100%] 10 passed, 17 warnings in 0.34s ``` Full memory suite, plus a before/after comparison of the failure set to prove no regressions: ```text $ python -m pytest tests/test_memory/ -q 13 failed, 576 passed, 3 skipped, 1072 warnings, 25 errors in 44.74s # the same run with the source changes stashed (baseline on upstream/main @ |
||
|
Chester
|
c471800e8e
|
fix(memory): keep vector metadata in sync (#2295)
## Description
Fixes #2296.
Metadata-only memory updates can leave the primary store, vector-index
metadata, and cache inconsistent. TrafficLearner also performs an atomic
SQLite evidence increment that bypasses normal secondary-index refresh.
## 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
- Refresh vector metadata for HierarchicalMemory metadata-only,
importance, and entity-reference updates.
- Add a LocalBackend path that reloads a memory from the primary store
and refreshes vector metadata plus cache state.
- Preserve the atomic TrafficLearner SQL evidence increment, then
refresh secondary state only when a row was updated.
- Keep refresh failures fail-open and distinguish them from
primary-store increment failures in logs.
- Add backend-neutral contract tests instead of inspecting a specific
vector adapter private field.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`) — verified locally: mypy
1.20.2, no issues in 504 source files
- [x] New tests added for new functionality
- [x] Manual testing performed
### Test Output
```text
141 passed, 1 skipped
ruff check: passed
ruff format --check: passed
```
The first CI run exposed one backend-specific test assertion against
HNSW private state while CI used SQLiteVectorIndex. Commit
|
||
|
Abhinav Kumar Singh
|
65961827cf
|
fix(memory): close DirectMem0 resources
## Description `DirectMem0Adapter.close()` now deterministically drains or cancels background writes and releases every initialized client/driver. Fixes #2897 ## 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 - Initialize the OpenAI client field to `None` so cleanup is safe before or after initialization. - Drain background tasks within a configurable 60-second default, cancel tasks that exceed the timeout, await cancellation, and retain completed/cancelled task status. - Close Mem0, OpenAI, Qdrant, Neo4j, embedder, and graph resources independently, including async close methods, while continuing cleanup if one resource fails. - Clear task and client references and keep `close()` idempotent. - Add regression tests for task draining, timeout cancellation, all resource cleanup, and repeated close calls. ## 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 python -m pytest -q tests/test_memory/test_direct_mem0.py tests/test_memory/test_qdrant_env.py 52 passed ruff check . All checks passed! ruff format --check . 1383 files already formatted python -m mypy headroom Success: no issues found in 515 source files python -m pytest -q Collection blocked in this Windows environment by 174 errors, primarily missing compiled headroom._core; 18 tests skipped. ``` ## Real Behavior Proof - Environment: Windows, Python 3.12, local DirectMem0Adapter instance using real `httpx.Client` resources. - Exact command / steps: Assigned real `httpx.Client()` instances to the adapter's OpenAI and Qdrant resource slots, registered an asynchronous background task, awaited `adapter.close(timeout=1.0)`, then checked both clients' `is_closed` state and the task status. - Observed result: `real httpx clients closed and background task drained`; both clients reported closed, no pending task IDs remained, and the task status was `completed`. - Who maintains it: Headroom Labs maintains this active upstream repository and memory backend. - Install surface: No dependencies or install behavior changed. The fix uses the standard-library asyncio/inspect modules and existing resource close methods; no native code or runtime network access is introduced. - Not tested: The complete test suite could not run past collection because this Windows environment lacks the compiled `headroom._core` extension. ## 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 - [ ] New and existing unit tests pass locally with my changes (full suite blocked by missing native extension; targeted tests pass) - [x] I did not edit `CHANGELOG.md` - it is generated by release-please from my Conventional Commit PR title. ## Screenshots (if applicable) Not applicable. ## Additional Notes The default close timeout is 60 seconds and can be overridden by callers that need a shorter shutdown budget. --------- Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> |
||
|
Abhinav Kumar Singh
|
4bd8ecd1e3
|
fix(memory): close MCP backend on shutdown
## Description Closes the initialized LocalBackend and cancels in-flight initialization whenever the memory MCP stdio transport exits. Fixes #2898 ## 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 - Added an explicit server cleanup callback that cancels and awaits pending backend initialization. - Closes an initialized backend exactly once and clears the backend/task references. - Runs cleanup in `_run()` through a `finally` block after the stdio transport exits, including transport errors. - Added regression coverage for initialized cleanup, pending initialization cancellation, idempotence, and `_run()` shutdown behavior. ## 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 python -m pytest -q tests/test_memory/test_mcp_server.py 15 passed, 20 warnings ruff check . All checks passed! ruff format --check . 1382 files already formatted python -m mypy headroom Success: no issues found in 515 source files python -m pytest -q Collected 8881 items / 174 errors / 18 skipped. Interrupted during collection because this Windows environment lacks the compiled headroom._core extension. ``` ## Real Behavior Proof - Environment: Windows, Python 3.12, async MCP server lifecycle test with the real `create_memory_server()` closure and an embedded server transport stub. - Exact command / steps: Ran `python -m pytest -q tests/test_memory/test_mcp_server.py`; the regression tests initialized a backend through the server's registered tool lifecycle, returned the stdio transport, and invoked the cleanup callback from `_run()`'s `finally` path. - Observed result: 15 tests passed. Initialized backends were closed once, pending initialization was cancelled and awaited, and transport exit invoked cleanup even when the server run returned. - Who maintains it: Headroom Labs maintains this active upstream repository and memory MCP server. - Install surface: No dependencies or install behavior changed. The fix uses existing asyncio lifecycle handling and `LocalBackend.close()`; no native code or runtime network access is introduced. - Not tested: The complete repository suite could not run past collection because this Windows environment lacks the compiled `headroom._core` extension. ## 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 - [ ] New and existing unit tests pass locally with my changes (full suite blocked by missing native extension; targeted tests pass) - [x] I did not edit `CHANGELOG.md` - it is generated by release-please from my Conventional Commit PR title. ## Screenshots (if applicable) Not applicable. ## Additional Notes Cleanup is attached to each created memory MCP server and is idempotent, so embedded callers can invoke the same lifecycle callback safely if needed. |
||
|
Abhay Singh
|
1f5fefffd3
|
fix(memory): bound the TrafficLearner pending-pattern accumulator (memory leak) (#2579)
## Description
`TrafficLearner` (the memory/learning subsystem that accumulates
patterns from proxy traffic) has an unbounded in-memory accumulator.
`_pattern_counts` maps `content_hash -> (pattern, count)`. A pattern is
added on first sighting, its count is bumped on each re-sighting, and it
is **removed only when it reaches `min_evidence`** (default 5), at which
point it is promoted and its hash moves to `_saved_hashes`:
```python
if h in self._pattern_counts:
existing, count = self._pattern_counts[h]
count += 1
self._pattern_counts[h] = (existing, count)
else:
self._pattern_counts[h] = (pattern, 1)
return # first sighting — wait for more evidence
...
if count >= self._min_evidence:
del self._pattern_counts[h] # only removal path
self._saved_hashes.add(h)
if len(self._saved_hashes) > self._dedup_window: # sibling IS trimmed
self._saved_hashes.pop()
```
A pattern seen **once but never corroborated** — the common case for
one-off traffic (a unique error string, an ad-hoc shell command, a
distinct file path) — never reaches `min_evidence`, so it is **never
removed**. Over a long-lived proxy processing varied traffic,
`_pattern_counts` grows without bound and RSS climbs. The sibling
`_saved_hashes` is explicitly trimmed to `dedup_window` ("prevent
unbounded growth"); `_pattern_counts` was missed.
Reproduced directly: feeding 500 distinct one-off patterns leaves 500
entries in `_pattern_counts` (one per pattern, forever).
## Fix
Make `_pattern_counts` an LRU-ordered `OrderedDict` capped at a new
`max_pending_patterns` (default 2048):
- On each corroboration, `move_to_end(h)` so an actively-accumulating
pattern stays "fresh" and is never evicted before it can be promoted.
- On a first sighting when the accumulator is full, evict the
least-recently-corroborated pending entry (`popitem(last=False)`).
Evicting a stale one-off is safe: if it recurs it simply restarts
accumulation (delayed promotion at worst) — the same tradeoff
`_saved_hashes` already makes. Promotion at `min_evidence` is unchanged,
and the cap (2048) is generous enough that any pattern receiving repeat
sightings within a normal window reaches `min_evidence=5` long before
eviction.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring
## Changes Made
- `headroom/memory/traffic_learner.py`: `_pattern_counts` becomes a
capped LRU `OrderedDict`; add `max_pending_patterns` (default 2048);
`move_to_end` on corroboration and evict-oldest on overflow.
- `tests/test_memory/test_traffic_learner.py`: a regression that 500
one-off patterns keep the accumulator at its cap, and one that a
corroborated pattern still promotes into `_saved_hashes` (both sync via
`asyncio.run` so they run without the pytest-asyncio plugin).
## 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_memory/test_traffic_learner.py -q
35 failed, 109 passed
# the 35 failures are pre-existing @pytest.mark.asyncio tests that need
# pytest-asyncio (not configured in this environment); they fail identically
# on clean main (35 failed, 107 passed) and pass in CI. My two new tests are
# synchronous and pass; they add +2 passing with no new failures.
# with the fix reverted, test_pending_accumulator_is_bounded fails
# (the accumulator holds all 500 one-off patterns)
$ uvx ruff@0.15.17 check headroom/memory/traffic_learner.py tests/test_memory/test_traffic_learner.py
All checks passed!
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/memory/traffic_learner.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: built a `TrafficLearner(backend=None,
min_evidence=5, max_pending_patterns=8)` and drove `_accumulate` with
500 distinct one-off `ExtractedPattern`s; separately corroborated one
pattern to `min_evidence`; then reverted the source and re-ran.
- Observed result: with the fix `len(_pattern_counts)` stays at the cap
(8) after 500 one-offs, the corroborated pattern is removed from pending
and present in `_saved_hashes`, and an actively-bumped pattern survives
LRU eviction; with the fix reverted the accumulator holds all 500
one-off entries (the unbounded leak). Ran against the actual module.
- Not tested: a live multi-day proxy run measuring RSS (the leak is
inferred from the removed unbounded-growth path; the accumulator bound
is verified directly).
## 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
|
||
|
gglucass
|
a70e5ff78d
|
fix(learn): run project discovery off the event loop (#2731)
## Description
`TrafficLearner.flush_to_file` is a coroutine, but it called
`plugin.discover_projects()` inline. That function walks the filesystem
to decode escaped project directory names — in
`learn/plugins/claude.py`, `_greedy_path_decode` recurses through
`iterdir()` at every level and tries each tokenization of each child,
backtracking on a miss — so on a large home tree it runs for minutes.
Doing that on the event loop freezes uvicorn for the whole window. The
port keeps accepting TCP, but `/readyz` never answers, so a supervisor
health-checking the proxy kills a process that is merely busy.
Field thread dumps show exactly that:
```
Current thread (most recent call first):
File "python3.12/pathlib.py", line 1056 in iterdir
File "headroom/learn/plugins/claude.py", line 454 in _greedy_path_decode
File "headroom/learn/plugins/claude.py", line 478 in _greedy_path_decode
File "headroom/learn/plugins/claude.py", line 478 in _greedy_path_decode
File "headroom/learn/plugins/claude.py", line 426 in _decode_project_path
File "headroom/learn/plugins/claude.py", line 71 in discover_projects
File "headroom/memory/traffic_learner.py", line 591 in flush_to_file
File "headroom/memory/traffic_learner.py", line 535 in _flush_worker
File "python3.12/asyncio/events.py", line 88 in _run
File "python3.12/asyncio/base_events.py", line 1999 in _run_once
File "python3.12/asyncio/base_events.py", line 645 in run_forever
File "uvicorn/server.py", line 75 in run
File "headroom/proxy/server.py", line 4992 in run_server
```
Accompanying signals from the same incidents: port accepts TCP,
`/readyz` times out, process CPU 2-13s across the window (I/O bound, not
spinning), proxy log silent 66-336s.
## Type of Change
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which 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`: `flush_to_file` now awaits
`asyncio.to_thread(plugin.discover_projects)` instead of calling it
inline. `asyncio` was already imported. The result is cached per learner
(`_project_roots_cache`), so the steady-state flush path pays nothing
for the thread hop.
- `tests/test_memory/test_traffic_learner.py`: added
`test_discover_projects_does_not_block_the_event_loop`.
## 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
$ uv run --frozen --extra dev pytest tests/test_memory/test_traffic_learner.py -q
.................................................. [100%]
============================= 152 passed in 2.93s ==============================
$ uvx ruff check headroom/memory/traffic_learner.py tests/test_memory/test_traffic_learner.py
All checks passed!
$ uvx ruff format --check headroom/memory/traffic_learner.py tests/test_memory/test_traffic_learner.py
2 files already formatted
$ uv run --frozen --extra dev mypy headroom/memory/traffic_learner.py
Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: macOS 15.6 arm64, Python 3.10.18, pytest 9.0.3, branch
off `main` @ `
|
||
|
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. |
||
|
Abhay Singh
|
6cdfd3f64d
|
fix(proxy/openai): feed chat/completions traffic into the traffic learner (#2333)
## 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> |
||
|
Gautam Sharma
|
0924755591
|
fix(memory): serialize MCP backend initialization (#2309)
## Description
The Memory MCP server previously assigned its backend before
asynchronous embedder and vector-index warm-up completed. A tool call
arriving
during the handshake could therefore receive a partially initialized
backend.
Backend initialization is now atomic and shared between concurrent
callers. The backend is published only after warm-up succeeds. Failed
candidates are closed and discarded so later calls can retry with a
fresh backend.
## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation-only change
- [ ] Refactoring
## Changes Made
- Keep the initializing backend local until warm-up completes
successfully.
- Share one initialization task between handshake and concurrent tool
calls.
- Await the shared task before exposing the backend to tool handlers.
- Shield shared initialization from cancellation by an individual tool
caller.
- Close failed or cancelled backend candidates.
- Clear failed initialization state so subsequent calls can retry.
- Retrieve and log background initialization failures.
- Add regression tests for handshake races, failure recovery, and
concurrent initialization.
- Add an Unreleased changelog entry.
## Testing
- [x] Added regression tests
- [x] Focused test suite passes
- [x] Ruff checks pass
- [x] Mypy passes
- [x] Changed files pass formatting checks
- [ ] Entire repository test suite passes without baseline failures
Commands and results:
- `uv run --extra dev --frozen pytest
tests/test_memory/test_mcp_server.py -q`
- `12 passed`
- `uv run --extra dev --frozen ruff check .`
- Passed
- `uv run --extra dev --frozen ruff format --check
headroom/memory/mcp_server.py tests/test_memory/test_mcp_server.py`
- Passed
- `uv run --extra dev --frozen mypy headroom --ignore-missing-imports`
- Success across 504 source files
- `uv run --extra dev --frozen pytest -q`
- `9363 passed, 565 skipped, 4 failed`
- The four failures are existing, unrelated failures outside the changed
code:
- `test_l2_appends_transform_label`
- `test_recovery_records_sockets_and_secures_both_backups`
- `test_dashboard_uses_cached_stats_and_lazy_history_feed_polling`
- `test_smart_crusher_log_fallback_runs_for_valid_json`
Repository-wide `ruff format --check .` also identifies pre-existing
formatting drift in the untouched
`headroom/proxy/handlers/anthropic.py`.
## Real Behavior Proof
The regression tests exercise the affected lifecycle directly:
1. Start backend initialization through the MCP handshake.
2. Suspend warm-up before it completes.
3. Issue a memory tool call and verify its handler is not invoked.
4. Release warm-up and verify the tool receives the initialized backend.
5. Force background initialization to fail and verify the candidate is
closed.
6. Issue another tool call and verify initialization retries with a
fresh backend.
7. Start two tool calls concurrently and verify only one backend is
constructed.
Observed behavior:
- Tool calls remain pending while handshake warm-up is incomplete.
- A partially initialized backend never reaches a tool handler.
- Failed candidates are closed and discarded.
- A later tool call successfully retries initialization.
- Concurrent calls share one initialization task and backend.
Environment: macOS arm64, CPython 3.12.13.
Not tested: a live stdio MCP client using the real ONNX model and
database. The affected initialization lifecycle is covered with
deterministic asynchronous regression tests.
## Review Readiness
- [x] I have performed a self-review before requesting human review.
- [x] This PR is ready for human review.
## Checklist
- [x] The implementation follows the repository’s existing style and
error-handling conventions.
- [x] Tests cover the reported race, concurrent initialization, and
failure recovery.
- [x] Failed initialization does not leave a partially published
backend.
- [x] Failed backend candidates are closed before retry.
- [x] No unrelated files or formatting changes are included.
- [x] No temporary logging, debug code, or commented-out code remains.
- [x] Public behavior changes are documented in the changelog.
- [x] The branch has been rebased from the intended base and is ready
for review.
## Additional Notes
The four full-suite failures listed above occur outside the changed
Memory MCP code and are unrelated to this PR. All tests covering the
modified initialization lifecycle pass.
---------
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Chester
|
aa4515cf7a
|
fix(memory): filter inactive graph-expanded results (#2210)
## DescriptionKeep graph-expanded local-memory results consistent with the current-only contract already applied by vector search.Closes #2209## 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- Reject graph-expanded memories whose `valid_until` is set.- Reject graph-expanded memories whose `superseded_by` is set.- Add focused coverage for active, expired, and superseded related memories.## Testing- [x] Unit tests pass (`pytest`)- [x] Linting passes (`ruff check .`)- [ ] Type checking passes (`mypy headroom`)- [x] New tests added for new functionality- [ ] Manual testing performed### Test Output```text$ uv run --with pytest --with pytest-asyncio --with numpy pytest tests/test_memory/test_local_backend_search.py -q3 passed$ uv run --with ruff ruff check headroom/memory/backends/local.py tests/test_memory/test_local_backend_search.pyAll checks passed!$ uv run --with ruff ruff format --check headroom/memory/backends/local.py tests/test_memory/test_local_backend_search.py2 files already formatted```## Real Behavior Proof- Environment: Python 3.13, synthetic in-memory test doubles- Exact command / steps: run the focused test file above- Observed result: active graph-linked memory is returned; records with `valid_until` or `superseded_by` are excluded- Not tested: full repository suite, external vector/graph implementations## 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- [ ] I have commented my code, particularly in hard-to-understand areas- [ ] I have made corresponding changes to the documentation- [ ] 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 NotesDocumentation and changelog changes are not needed for this narrow internal behavior fix. The existing temporal-history APIs remain unchanged. Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Chester
|
6d897e8eaa
|
fix(memory): require explicit updates for supersession (#2188)
## Description The standalone Memory MCP `memory_save` handler currently treats vector similarity as update identity. A score of `0.70` can therefore supersede a valid but distinct memory that merely shares domain vocabulary. This change makes `memory_save` append-only. Supersession remains available through explicit update paths that receive an existing memory ID. Closes #2187. ## 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 causes existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Remove vector-similarity-based auto-supersession from the standalone MCP `memory_save` handler. - Clarify in the tool description that corrections require an explicit update path with the existing memory ID. - Add a regression test proving that a high-scoring but distinct memory is neither searched for replacement nor updated. - Preserve the existing save result summary shape for compatibility. ## 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 uv run --with pytest --with numpy pytest tests/test_memory/test_mcp_server.py -q 9 passed, 21 warnings in 0.70s uvx ruff check headroom/memory/mcp_server.py tests/test_memory/test_mcp_server.py All checks passed! uvx ruff format --check headroom/memory/mcp_server.py tests/test_memory/test_mcp_server.py 2 files already formatted ``` The warnings are pre-existing pytest configuration and `datetime.utcnow()` deprecation warnings in the test environment. ## Real Behavior Proof - Environment: Python 3.13 with the MCP module stub and an async recording backend. - Exact command / steps: run `tests/test_memory/test_mcp_server.py`; the new regression supplies a search result with similarity `0.91`, then saves a distinct fact. - Observed result: `search_memories` and `update_memory` are not called; `save_memory` is called once with the new fact and requested importance. - Not tested: live embedding backends or migration of supersession chains created by earlier versions. ## 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 non-obvious identity boundary - [ ] Documentation changes are limited to the MCP tool description - [x] My changes generate no new warnings - [x] I have added tests that prove the fix is effective - [ ] New and existing unit tests pass locally; the focused MCP suite passes and full CI is pending - [ ] CHANGELOG update is not included because release notes are generated from conventional commits ## Screenshots (if applicable) Not applicable. ## Additional Notes This patch intentionally does not infer replacement identity from category, entity references, or a higher vector threshold: none of those alone proves that two statements are versions of the same fact. Exposing an explicit update tool from the standalone MCP server can be considered separately without retaining the unsafe automatic behavior. Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Chester
|
ce52b30c8f
|
feat(memory): add explicit supersession repair (#2217)
## Description Add an explicit, reviewable way to detach one incorrect supersession edge while preserving both memories and all neighboring version history. Closes #2216 ## 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) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Add an atomic SQLite `detach_supersession(old_id, new_id)` primitive that requires reciprocal direct lineage. - Restore only the old memory's validity and clear only the selected edge. - Re-index both affected memories and refresh cache state through `HierarchicalMemory`. - Expose the operation through `LocalBackend`. - Add `headroom memory repair-supersession OLD_ID NEW_ID`, dry-run by default with explicit `--apply`. - Resolve unambiguous partial IDs for preview but pass full IDs to the mutation. - Add chain-locality, rejection, index/cache, dry-run, and apply-path tests. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ uv run --with pytest --with pytest-asyncio --with numpy --with fastapi --with httpx pytest \ tests/test_memory/test_supersession_repair.py \ tests/test_memory/test_hierarchical.py::TestSQLiteMemoryStore \ tests/test_cli/test_main_help_version.py -q 22 passed $ uv run --with ruff ruff check <touched Python files> All checks passed! $ uv run --with ruff ruff format --check <touched Python files> 6 files already formatted ``` ## Real Behavior Proof - Environment: Python 3.13, temporary SQLite databases, synthetic two- and three-version chains - Exact command / steps: run the focused test set above - Observed result: detaching `v1 -> v2` restores `v1` as current, leaves `v2 -> v3` intact, re-indexes both records, refreshes cache, and keeps CLI preview read-only until `--apply` - Not tested: live proxy process, external MemoryStore/VectorIndex plugins, full repository suite ## Review Readiness - [x] I have performed a self-review - [ ] 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 - [ ] 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 This is intentionally separate from #2188: that PR prevents new false edges, while this PR repairs historical data. The CLI help requires stopping any proxy that is actively using the same database before `--apply`, because another process can retain an old in-memory index snapshot. External backend semantics and stronger cross-store rollback behavior are left visible for maintainer review before this Draft is marked ready. Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Abhay Singh
|
fa330f3e2b
|
fix(memory): remove a superseded memory from the search indexes (#2143)
## Description
Superseding a memory leaves the old version live in the search indexes,
so outdated content can still come back from search after supersession.
`HierarchicalMemory.supersede` updates the store and indexes the new
memory, but previously never removed the old entry from the vector/text
indexes. The store sets the old row's `valid_until` and `superseded_by`,
but the indexes keep their own cached metadata copy from first indexing.
Default search filters superseded rows from that cached metadata, so the
old entry could still look live and be returned with stale content.
Concretely: `add("User prefers Python")`, then `supersede(id, "User now
prefers JavaScript frameworks")`, then `search("Python")` could return
the superseded "prefers Python" entry alongside the new one. That
defeats supersession and can recall contradictory facts.
## Fix
After `store.supersede`, remove the old id from the vector and text
indexes, mirroring `delete`. The store keeps the old row for
`get_history`; only the search indexes are corrected. The new memory is
indexed as before.
## 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/core.py`: `supersede` removes the old id from the
vector and text indexes after the store supersession, before indexing
the new memory.
- `tests/test_memory/test_core_operations.py`: adds
`test_superseded_memory_does_not_resurface_in_search`.
- `CHANGELOG.md`: adds a bug-fix entry.
- Merged current `main` to pick up the repository-wide memory factory
type annotation fix that was breaking the PR lint job.
## Testing
- [x] Unit tests pass (`pytest` in CI on the pre-merge head; fresh CI is
running on the main-merged head)
- [x] Linting passes (`ruff check` focused locally; previous CI `ruff
check` and `ruff format` passed before hitting unrelated mypy)
- [x] Type checking passes for the prior mypy blocker after merging
current `main`
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
uvx ruff@0.15.17 check headroom/memory/core.py tests/test_memory/test_core_operations.py headroom/memory/factory.py
All checks passed!
uvx ruff@0.15.17 format --check headroom/memory/core.py tests/test_memory/test_core_operations.py headroom/memory/factory.py
3 files already formatted
git diff --check headroomlabs/main...HEAD
# no output
uv run --extra dev python -m pytest tests/test_memory/test_core_operations.py::TestSupersede::test_superseded_memory_does_not_resurface_in_search -q
# assertion passed; local Windows teardown hit a locked temp SQLite file during fixture cleanup
```
## Real Behavior Proof
- Environment: Windows 11 review worktree, plus GitHub Actions on the
pre-merge head.
- Exact command / steps: ran focused ruff/format/diff checks; ran the
new supersede regression test directly.
- Observed result: focused checks passed; the new test body passed and
confirmed the old memory id does not resurface when searching for old
content while the new memory remains searchable. The local run then
errored during Windows temp SQLite cleanup after the assertion
completed.
- Not tested: full memory suite, because it pulls the ML embedding
stack. CI already passed the broader test matrix on the pre-merge head;
fresh checks are queued after the main merge.
## 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
Removing the old entry from indexes is intentionally aligned with
`delete`; the store row remains available for `get_history`.
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
b0afee85b3
|
fix(memory): size HNSW index_batch resize off the id high-water mark (#2139)
## Description `HNSWVectorIndex.index_batch()` used the live memory map size to decide whether to resize hnswlib before adding new labels. hnswlib does not reclaim capacity slots when labels are removed with `mark_deleted`, so after delete/evict churn the live count can be much lower than the assigned-id high-water mark. That lets a batch add skip resizing and then fail in `add_items` with `number of elements exceeds the specified limit`. ## Fix - Size the batch resize check from `self._next_hnsw_id`, which has already been incremented for the new batch labels. - Match the single-item `index()` path's high-water-mark capacity behavior. - Add a regression test that deletes most entries from a small index and then batch-adds enough new memories to require a resize. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/memory/adapters/hnsw.py tests/test_memory/test_hnsw_batch_capacity.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/memory/adapters/hnsw.py tests/test_memory/test_hnsw_batch_capacity.py headroom/memory/factory.py 3 files already formatted git diff --check headroomlabs/main...HEAD # no output uv run --extra dev python -m pytest tests/test_memory/test_hnsw_batch_capacity.py -q 1 passed, 18 warnings ``` ## Review Readiness - [x] Ready for review - [x] Regression test added - [x] CHANGELOG updated Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
9e38905a7d
|
fix(memory): apply turn_id scope filter even without agent_id (#2130)
## Description `SQLiteMemoryStore._build_query_conditions()` dropped `turn_id` when a query specified `session_id` and `turn_id` without also specifying `agent_id`. That made a single-turn scope return every memory in the session, and `count()` uses the same helper. ## Fix - Apply `agent_id` and `turn_id` as independent narrowing predicates inside the `session_id` branch. - Preserve the existing `agent_id`-only behavior. - Add direct query-condition regression tests for turn-only, agent+turn, and agent-only scopes. - Merge current `main` to refresh mergeability and stale lint results. ## Testing ```text uvx ruff@0.15.17 check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.py headroom/memory/factory.py All checks passed! uvx ruff@0.15.17 format --check headroom/memory/adapters/sqlite.py tests/test_memory/test_query_conditions.py headroom/memory/factory.py 3 files already formatted git diff --check headroomlabs/main...HEAD # no output uv run --extra dev python -m pytest tests/test_memory/test_query_conditions.py -q 3 passed ``` ## Review Readiness - [x] Ready for review - [x] Regression tests added - [x] CHANGELOG updated Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Abhay Singh
|
1725cd1f83
|
fix(memory): key the embedder cache on ollama_base_url (#2109)
## Description
The process-wide embedder cache can hand a caller an embedder bound to
the wrong Ollama server.
`_create_embedder` caches by `(backend, model)`:
```python
key = (
config.embedder_backend.value if hasattr(...) else str(...),
config.embedder_model or "",
)
```
But the Ollama branch constructs the embedder with the server URL:
```python
embedder = OllamaEmbedder(base_url=config.ollama_base_url, model_name=config.embedder_model)
```
So two configs in the same process that share a backend and model but
point at different Ollama servers (for example a per-project storage
router, or a fail-over host) collide on the same cache key. The first
call builds and caches an `OllamaEmbedder` bound to server A; the second
call, asking for server B, gets server A's embedder back and silently
embeds against the wrong host.
The code already reasoned about the analogous `openai_api_key` omission
and worked around it with an up-front validation guard (see the comment
above the key), but `ollama_base_url` has no such guard, so it just
resolves to the wrong server.
## Fix
Add `config.ollama_base_url` to the cache key. Same server still hits
the cache (one model load); a different server gets its own embedder.
Non-Ollama backends are unaffected (the URL just becomes an extra,
constant key component).
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/memory/factory.py`: include `config.ollama_base_url` in the
embedder cache key, with a comment explaining why.
- `tests/test_memory/test_factory_embedder_cache.py`: new file with
`test_ollama_embedder_cache_keys_on_base_url` (different servers get
different embedders) and
`test_ollama_embedder_cache_reuses_same_base_url` (same server still
caches). Kept out of `test_factory.py` because that module skips
wholesale without `hnswlib`, which these cases don't need.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] 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/factory.py tests/test_memory/test_factory_embedder_cache.py
All checks passed!
$ python -m py_compile headroom/memory/factory.py tests/test_memory/test_factory_embedder_cache.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 cache-key behavior with a
dependency-free script that models the `(backend, model)` vs `(backend,
model, base_url)` keys against a simulated cache, and left the full
pytest to CI.
- Exact command / steps: created two configs with the same backend and
model but `ollama_base_url` of `http://gpu1:11434` and
`http://gpu2:11434`, and resolved each through the old key and the new
key against a shared cache.
- Observed result: the old key serves the same embedder object for both,
and the config asking for `gpu2` is handed the `gpu1`-bound embedder;
the new key gives each config its own embedder bound to its own server.
The new tests assert distinct embedders with the right `_base_url` for
different servers, and cache reuse for the same server.
- Not tested: a live Ollama round-trip (`OllamaEmbedder` construction is
offline — it stores the URL and lazily creates its client); 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
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change adds one component to a cache-key tuple in a
pure function, verified by the standalone proof and the two new tests
for CI. The tests construct only the lightweight (offline) Ollama
embedder, so they don't need a running server or the vector-index deps.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Abhay Singh
|
a5bdc5491f
|
fix(memory/sqlite): don't emit OFFSET without LIMIT in query (#2063)
## Description
`SQLiteMemoryStore.query` (`headroom/memory/adapters/sqlite.py`) builds
pagination like this:
```python
if filter.limit is not None:
query += " LIMIT ?"
params.append(filter.limit)
if filter.offset > 0:
query += " OFFSET ?"
params.append(filter.offset)
```
SQLite's grammar allows `OFFSET` **only** as part of a `LIMIT` clause.
So a `MemoryFilter` with
an offset but no limit produces `... ORDER BY created_at DESC OFFSET ?`,
which SQLite rejects:
```
sqlite3.OperationalError: near "OFFSET": syntax error
```
Both `offset` and `limit` are public `MemoryFilter` fields (`ports.py`:
`limit` defaults to
`None`, `offset` to `0`), so any caller paginating with an offset but no
explicit limit crashes.
Closes: no issue filed — found while auditing the memory store query
builder.
## Fix
When an offset is present without a limit, emit SQLite's unbounded
`LIMIT -1` so `OFFSET` is
grammatically valid:
```python
if filter.offset > 0:
if filter.limit is None:
query += " LIMIT -1"
query += " OFFSET ?"
params.append(filter.offset)
```
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/memory/adapters/sqlite.py`: emit `LIMIT -1` when paginating
with an offset but no limit.
- `tests/test_memory/test_hierarchical.py`: add
`test_query_offset_without_limit` (offset skips rows; offset past the
end returns `[]`; no crash).
## Testing
- [x] New regression test added
(`tests/test_memory/test_hierarchical.py`)
- [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17`
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uvx ruff@0.15.17 check headroom/memory/adapters/sqlite.py tests/test_memory/test_hierarchical.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I reproduced the exact SQL
against a real stdlib `sqlite3` in-memory DB (the store's query is pure
SQL) and left the full pytest to CI.
- Exact command / steps: built the same `ORDER BY ... [LIMIT] [OFFSET]`
query for `offset=2, limit=None` with the old and new logic and ran it
against a 5-row table.
- Observed result: the old builder raises the exact `OperationalError`;
the new builder skips `offset` rows and returns the rest, and
`LIMIT`-only / `LIMIT`+`OFFSET` still work:
```text
OLD offset-no-limit: OperationalError -> near "OFFSET": syntax error
NEW offset-no-limit: rows=[2, 1, 0]
SQLITE OFFSET-WITHOUT-LIMIT FIX VERIFIED (old crashes; new paginates)
```
- Not tested: the full `HierarchicalMemory` stack (needs the heavy
embedder). The new test drives `SQLiteMemoryStore.query` directly with
`save_batch` + `MemoryFilter`. 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 — ran
lint + a standalone SQLite check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- One-line grammar fix plus a test; no new dependencies.
- @JerrettDavis tagging you — a paginating caller (offset, no limit)
currently crashes the memory store query; quick one. Thanks!
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
|
||
|
Chester
|
d0ecc9a556
|
fix(memory): track MCP retrieval access (#2065)
## Description Track successful native MCP `memory_search` retrievals in persistent memory metadata. Returned memories now increment `access_count` and update `last_accessed`, so MCP usage contributes to memory budget and retention signals. Closes #2061 ## 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 - Add an atomic, deduplicated `MemoryStore.record_access` operation. - Expose access recording through `HierarchicalMemory` and `LocalBackend`, invalidating stale cache entries. - Record only the final active memories actually returned by MCP search. - Fail open if usage metadata cannot be written. - Add SQLite and MCP regression coverage. ## Testing - [x] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text pytest tests/test_memory --ignore=tests/test_memory/test_learn_flag.py -q 368 passed, 142 skipped, 158 warnings in 3.28s pytest tests/test_memory/test_hierarchical.py tests/test_memory/test_mcp_server.py tests/test_memory/test_factory.py -q 40 passed, 53 skipped, 158 warnings in 0.75s ``` ## Real Behavior Proof - Environment: macOS, Python 3.13, SQLite memory store. - Exact command / steps: save two memories; call `record_access` with duplicate IDs plus a missing ID; read both rows; call it again for one row. - Observed result: each existing memory increments once per call, duplicates do not double-count, missing IDs are ignored, and `last_accessed` advances to the supplied timestamp. - Not tested: the full repository suite and `tests/test_memory/test_learn_flag.py`; the source checkout does not include the compiled `headroom._core` Rust extension. Ruff and mypy were not available in the local development environment; CI remains authoritative for those checks. ## 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 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 ## Additional Notes Documentation and changelog changes are not included because this is an internal retrieval-metadata correction with no user-facing configuration change. Access tracking is intentionally fail-open so a metadata write failure cannot suppress a valid memory search result. --------- Co-authored-by: xuyidiao <xuyidiao@bytedance.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
JD Davis
|
4e19bcf6ce
|
test(memory): skip decorators on offline model misses (#2020)
## Description Current `main` already has the shared `external_model_skip_reason` helper and pytest hooks for transient/offline model dependency failures. This follow-up applies the same classifier to the async memory integration test decorators in `test_core_operations.py` and `test_easy.py`, so decorated tests also skip offline Hugging Face cache-miss errors instead of only `httpx.ReadTimeout`. Supersedes #1017 with a clean branch based on current `main`. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Updated `network_timeout_handler` in `tests/test_memory/test_core_operations.py` to call `external_model_skip_reason` and re-raise unrelated exceptions. - Updated `network_timeout_handler` in `tests/test_memory/test_easy.py` the same way. - Removed now-unnecessary direct `httpx` imports from those files. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy` via commit hook) - [x] New tests added for new functionality ### Test Output ```text $ python -m pytest tests/test_memory/test_skip_helpers.py -q 4 passed in 0.12s $ python -m ruff check tests/test_memory/test_core_operations.py tests/test_memory/test_easy.py tests/test_memory/test_skip_helpers.py All checks passed! $ python -m ruff format --check tests/test_memory/test_core_operations.py tests/test_memory/test_easy.py tests/test_memory/test_skip_helpers.py 3 files already formatted $ git commit -m "test(memory): skip decorators on offline model misses" Sync plugin versions.....................................................Passed check for merge conflicts................................................Passed ruff.....................................................................Passed ruff-format..............................................................Passed mypy.....................................................................Passed ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13, local `C:\git\headroom` checkout. - Exact command / steps: ran `python -m pytest tests/test_memory/test_skip_helpers.py -q` against the skip classifier used by these decorators. - Observed result: `4 passed`, covering `httpx.ReadTimeout`, `LocalEntryNotFoundError`, offline Hugging Face `OSError`, and unrelated errors. - Not tested: live memory integration against an intentionally missing Hugging Face cache. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review |
||
|
Krishna Chaitanya
|
b84afbfb83
|
fix(memory): cap local embedder CPU thread oversubscription (#198) (#1559)
## Description The torch/sentence-transformers `LocalEmbedder` ran encodes on the shared default executor with **no BLAS/OpenMP thread cap**. Under concurrent load each `encode()` fans out to ~`os.cpu_count()` BLAS/OpenMP threads, so N in-flight encodes spawn ~`N × cpu_count` OS threads — oversubscribing the CPU, slowing the `memory_context` stage and (on smaller boxes) starving the asyncio event loop. The ONNX embedder already bounds its threads (`create_cpu_session_options(intra_op_num_threads=1, inter_op_num_threads=1)`); this brings the torch path to parity. Supersedes #691 by @oxura — closed only for the open-PR cap, with an explicit invitation to resubmit; no technical objection was raised, and its CI was fully green. Credit to @oxura for the original diagnosis and fix. That PR capped threads by setting BLAS/OpenMP env vars at import time plus `torch.set_num_threads`; this PR instead runs CPU encodes on a dedicated, size-limited executor whose workers each pin their thread pool — which additionally bounds in-flight encode concurrency (the issue's Fix B/C) and keeps the cap contained to the embedder rather than mutating process-global env at import. Closes #198 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] Performance improvement ## Changes Made - CPU encodes now run on a **dedicated, size-limited executor** whose worker `initializer` pins each worker's torch intra-op pool (and sets BLAS/OpenMP env defaults). torch's OpenMP thread count is per-thread, so a one-shot cap misses pooled executor workers — the per-worker initializer caps every worker deterministically. - Total embedding threads are bounded by `HEADROOM_EMBED_CONCURRENCY` (default `min(4, os.cpu_count())`) × `HEADROOM_EMBED_NUM_THREADS` (default `1`); invalid/non-positive values fall back safely (≥1). - Mirrors the existing MPS dedicated-single-worker-executor pattern; CUDA keeps the shared default executor (GPU compute is off-CPU). `setdefault` never overrides an operator's explicit `OMP_NUM_THREADS`. ## 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 $ uv run pytest tests/test_memory/test_embedder_thread_cap.py tests/test_memory/test_embedder_mps_serialization.py -q 13 passed $ uv run pytest tests/test_memory/ tests/test_cli_proxy_embedding_server.py -q 533 passed # no regressions from the executor change $ uv run ruff check . && uv run ruff format --check . All checks passed! / 1016 files already formatted $ uv run mypy headroom --ignore-missing-imports Success: no issues found in 404 source files ``` New `tests/test_memory/test_embedder_thread_cap.py`: env resolution for both knobs (default / positive / invalid / clamped), worker-init env application + operator-override safety, and a behavioral test that loads the real CPU embedder and asserts every executor worker is pinned to the configured intra-op thread count. Updated `test_embedder_mps_serialization.py` to the new CPU contract. ## Real Behavior Proof - Environment: built this branch into a CPU-only Linux container, removed `onnxruntime` so the proxy falls back to the torch `LocalEmbedder`; a container has no MPS/CUDA, so it resolves to `device=cpu` — the deployment where #198 occurs. Python 3.12, torch 2.12.1, `all-MiniLM-L6-v2`, container capped to 4 CPUs, 32 concurrent clients. - Exact command / steps: `headroom proxy --host 0.0.0.0 --memory` in-container; a concurrent `/v1/messages` driver from the host (invalid key — `memory_context` runs before the upstream call); measured the `memory_context` stage from `/metrics` before vs after the cap. - Observed result: the embedder stage this PR targets improved — `memory_context` avg 73.5 ms → 58.7 ms and max 279 ms → 242 ms (uncapped 12×8 = 96 threads vs fix 4×1): ~20% faster and steadier inside the real proxy. Isolated component benchmarks (heavy concurrent `embed_batch`; `LocalBackend.search_memories`) show a larger effect — tail event-loop stall ~16–24 ms → ~3 ms, and search throughput +57%. Unit/regression: 13 new tests + 533 memory-suite tests pass; `ruff` + `mypy` clean. - Not tested: the issue's absolute multi-second `/livez` spike. On my hardware/synthetic load, `/livez` stalls were dominated by the upstream-connection path (invalid-key DNS/TLS), not the ~250 ms `memory_context` stage, so I can't attribute the multi-second figure to the embedder here — the original report was on an 8-core box with real Claude Code transcripts that drove `memory_context` itself to several seconds. Linux/CUDA hardware not exercised; no live LLM provider used; ONNX path unchanged. This PR removes the documented thread oversubscription and brings the torch path to ONNX parity; it does not claim to single-handedly resolve the 4 s figure. Measured `memory_context` stage timing (real containerized proxy, torch CPU embedder, 4 CPUs, 32 concurrent clients): | `memory_context` | avg | max | |---|---|---| | Before (uncapped, 12×8 = 96 threads) | 73.5 ms | 279 ms | | After (fix, 4×1) | 58.7 ms | 242 ms | ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes Default-behavior change: CPU encodes use a dedicated bounded pool instead of the shared default executor (`close()` tears it down). Both knobs are opt-in overrides with safe defaults. No new dependencies. Signed-off-by: Krishnachaitanyakc <krishnabkc15@gmail.com> |
||
|
Rod Boev
|
2cae13dd79
|
fix(ccr): skip Anthropic marker emission when tool injection is deferred (#1273)
## Description Anthropic request-side CCR can still compress a turn into retrieval markers after the frozen-prefix cache guard suppresses `headroom_retrieve` registration. That leaves the model with marker-only context it cannot redeem, so the proxy silently drops recoverable data on exactly the turns where cache preservation deferred tool injection. This change couples the Anthropic request-side CCR path to tool availability so a turn never emits retrieve-only markers without the retrieval tool, even when token mode or cache-mode prefix replay could otherwise reuse already-compressed marker text. Closes #1006 After a collaborator merged current `main` into this branch, CI also picked up unrelated offline-memory failures from the merged base. Those follow-up changes are test-only: they keep the offline Hugging Face cache lanes skipping cleanly instead of failing in memory tests that are outside the CCR runtime path. ## 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 - Couple Anthropic request-side CCR compression to the same frozen-prefix guard that already defers `headroom_retrieve` registration. - Keep the existing cache-preservation behavior: frozen-prefix turns stop emitting CCR retrieval markers instead of forcing tool injection into the cached prefix. - Make the skip decision use the effective frozen prefix after token-mode reclamping, so turns that genuinely reclamp to zero still keep normal reversible CCR behavior. - Bypass cached marker reuse in both token mode and cache-mode prefix replay when tool injection is deferred. - Add focused regressions for the Anthropic request-path seams under this bug: - frozen-prefix turns do not emit marker-only payloads - unfrozen turns still keep normal reversible CCR behavior - token-mode reclamp back to zero still compresses normally - existing `headroom_retrieve` tools keep reversible CCR on frozen turns - cache-mode delta reuse and exact-prefix replay both forward original content when retrieval is unavailable - Add a `CHANGELOG.md` entry because the proxy's user-visible Anthropic CCR behavior changes. - Add a shared test skip helper for offline Hugging Face cache misses and apply it to the merged `main` memory tests that were failing only in the offline CI shards after the branch picked up current `main`. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_proxy/test_anthropic_ccr_deferred_injection.py`) - [x] Linting passes (`uv run ruff check . && uv run ruff format . --check`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_proxy/test_anthropic_ccr_deferred_injection.py 14 passed, 1 warning in 34.19s uv run pytest tests/test_memory/test_skip_helpers.py tests/test_memory/test_embedder_mps_serialization.py::test_cpu_uses_shared_executor tests/test_memory/test_hierarchical.py::TestLocalEmbedder::test_embed_single tests/test_memory_bridge.py::TestMemoryBridgeImport::test_import_claude_code_memory tests/test_memory_handler_concurrent_init.py::test_real_localbackend_initializes_via_public_entrypoint tests/test_memory_system.py::TestLocalBackend::test_save_memory_basic -q 4 passed, 5 skipped, 11 warnings in 7.65s uv run ruff check . All checks passed! uv run ruff format . --check 968 files already formatted ``` ## Real Behavior Proof - Environment: local FastAPI `TestClient` for the Anthropic request path, plus Windows Python 3.12 offline-memory repros with `TRANSFORMERS_OFFLINE=1` - Exact command / steps: Run the focused CCR regression command and the offline-memory repro subset below on the merged branch state. - `uv run pytest tests/test_proxy/test_anthropic_ccr_deferred_injection.py` - `uv run pytest tests/test_memory/test_skip_helpers.py tests/test_memory/test_embedder_mps_serialization.py::test_cpu_uses_shared_executor tests/test_memory/test_hierarchical.py::TestLocalEmbedder::test_embed_single tests/test_memory_bridge.py::TestMemoryBridgeImport::test_import_claude_code_memory tests/test_memory_handler_concurrent_init.py::test_real_localbackend_initializes_via_public_entrypoint tests/test_memory_system.py::TestLocalBackend::test_save_memory_basic -q` - Scenario coverage from the CCR pytest command: - frozen prefix with deferred tool injection and cached marker text available in token mode - unfrozen turn with normal CCR marker emission - token mode where the tracked frozen prefix reclamps back to zero - frozen prefix where the client already supplied `headroom_retrieve` - cache-mode append-only delta reuse with a previously forwarded compressed prefix - cache-mode exact-prefix replay where the previous forwarded prefix already contained a marker - Scenario coverage from the offline-memory repro command: - direct local embedder startup on CPU with no cached HF model - hierarchical memory embedder startup with offline model cache missing - bridge import through `LocalBackend` - `MemoryHandler` public init warmup path - `LocalBackend` save path under the offline lane - Observed result: The CCR regression keeps marker-free forwarding on frozen turns without tool availability, and the merged-`main` offline-memory lanes now skip cleanly instead of failing unrelated CI shards. - frozen-prefix Anthropic turns without tool availability forward the original long transcript across both token-mode and cache-mode reuse paths, while unfrozen turns, reclamped token-mode turns, and frozen turns that already advertise `headroom_retrieve` keep the reversible CCR marker path - the merged-`main` offline-memory regressions now skip cleanly when the Hugging Face cache is unavailable instead of failing unrelated CI shards - Not tested: live Zed session cache-hit behavior, provider latency under real Anthropic upstreams, and online Hugging Face download lanes ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Runtime scope is still intentionally narrow to Anthropic request-side CCR. OpenAI, Gemini, streaming, and response-side CCR behavior are unchanged. - The only non-CCR diff is the test-only offline-memory follow-up required after current `main` was merged into the branch. - The focused proxy pytest run still emits the Windows-local `StarletteDeprecationWarning` from `fastapi.testclient`'s `httpx` bridge. The offline-memory repro command also emits existing datetime deprecation warnings and a pytest teardown warning around skipped offline lanes; none of those warnings were introduced by the CCR runtime change. |
||
|
Rod Boev
|
ad7993bf15
|
fix(codex): stop pinning Codex memory MCP to one project db (#1269)
## Description Stop `headroom wrap codex --memory` from pinning the global `headroom_memory` MCP server to one absolute SQLite path. Today the wrapper writes `--db <wrap-cwd>/.headroom/memory.db` into `~/.codex/config.toml`, which makes later Codex sessions either reopen a stale project-local DB or fail with `unable to open database file` when that original path disappears. This change lets the MCP server use its existing per-cwd default again, so each Codex session resolves `.headroom/memory.db` from the active project instead of a serialized past cwd. Closes #1147 The current Codex-memory config surface was shaped by https://github.com/chopratejas/headroom/issues/462 and https://github.com/chopratejas/headroom/issues/730; this PR keeps that surface project-scoped again instead of globally pinning one DB. ## 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 - remove the injected `--db` argument from the global `headroom_memory` Codex MCP block while keeping `--user` intact - preserve the wrap-time local `.headroom/memory.db` setup and Claude-memory import path for the current project - treat only wrap-owned Codex markers as snapshot-suppression and unwrap-cleanup signals, so pre-existing named MCP blocks still back up and restore - log a startup diagnostic from `headroom.memory.mcp_server` that records the configured DB path, config source, cwd/project root, resolved storage scope, path existence/readability, and whether the path was static or cwd-derived - add a shared MCP SDK test stub so both the memory MCP and CCR MCP test surfaces still run in CI when `mcp` is absent - make the shared MCP stub re-import target modules under the stubbed dependency set and restore any pre-existing target module object plus dotted parent-package attribute state after cleanup - add focused regressions and guard coverage for the persisted Codex config shape, named-MCP marker backup and restore, the no-backup memory-only unwrap path, the wrap-memory-then-unwrap cleanup path, the failed-wrap memory-only cleanup path, the startup-diagnostic path classification, the shared-store CCR retrieval path, and the shared MCP stub import lifecycle - add a `CHANGELOG.md` entry for the user-visible Codex memory scoping fix ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_ccr_mcp_server.py tests/test_memory/test_mcp_server.py tests/test_cli/test_wrap_codex.py tests/test_mcp_stub.py ======================== 78 passed, 1 warning in 5.96s ======================== Pytest warning: PytestConfigWarning: Unknown config option: asyncio_mode Pytest post-success atexit noise: PermissionError: [WinError 5] Access is denied: 'C:\Users\Rod\AppData\Local\Temp\pytest-of-Rod\pytest-current' uv run ruff check headroom/cli/wrap.py headroom/memory/mcp_server.py tests/_mcp_stub.py tests/test_ccr_mcp_server.py tests/test_cli/test_wrap_codex.py tests/test_mcp_stub.py tests/test_memory/test_mcp_server.py All checks passed! uv run ruff format headroom/cli/wrap.py headroom/memory/mcp_server.py tests/_mcp_stub.py tests/test_ccr_mcp_server.py tests/test_cli/test_wrap_codex.py tests/test_mcp_stub.py tests/test_memory/test_mcp_server.py --check 7 files already formatted ``` ## Real Behavior Proof - Environment: isolated temp project directories, a temp Codex home, the real `wrap codex` and `unwrap codex` CLI commands under pytest, a mocked missing-`codex` launch path for the failed-wrap cleanup case, and shared MCP-SDK stubs for the memory MCP and CCR MCP test modules so CI still exercises those paths without a real `mcp` install. - Exact command / steps: run `uv run pytest tests/test_ccr_mcp_server.py tests/test_memory/test_mcp_server.py tests/test_cli/test_wrap_codex.py tests/test_mcp_stub.py`; prove the persisted config shape with `TestCodexMemoryMcpConfig::test_inject_omits_db_and_replaces_existing_memory_block`; prove prepare-only wrap cleanup with `test_wrap_codex_memory_prepare_only_unwrap_removes_memory_mcp_without_prior_config`; prove failed-wrap cleanup with `test_wrap_codex_memory_launch_failure_unwrap_cleans_memory_only_config`; guard pre-existing named Codex MCP preservation with `test_memory_only_wrap_restores_preexisting_named_mcp_block` and `test_memory_only_wrap_without_backup_preserves_named_mcp_block`; prove the startup diagnostic classifications with `test_memory_mcp_startup_context_reports_dynamic_project_db` and `test_memory_mcp_startup_context_reports_static_external_db`; prove the shared-store CCR retrieval path with `test_mcp_uses_shared_singleton_store` and `test_mcp_retrieves_proxy_stored_content`; prove stub import cleanup with `test_import_module_with_mcp_stub_imports_target_and_cleans_up`, `test_import_module_with_mcp_stub_reimports_target_and_restores_originals`, and `test_import_module_with_mcp_stub_cleans_up_dotted_target_attribute`. - Observed result: the persisted global `headroom_memory` block now keeps `--user` but omits `--db`; prepare-only memory setup still bootstraps the current project's `.headroom/memory.db`; `headroom unwrap codex --no-stop-proxy` now removes both the prepare-only generated config and the failed-wrap memory-only config instead of leaving `[mcp_servers.headroom_memory]` behind; pre-existing named Codex MCP blocks remain restorable across both normal and no-backup memory-only unwrap paths because only wrap-owned markers suppress backups or trigger named-block cleanup; the memory MCP server now logs whether its DB path came from the cwd default or an explicit static path, along with the resolved path and scope it will open; CI can exercise both MCP test modules even when the `mcp` package is absent from the shard environment, and the shared stub now re-imports target modules under the stubbed SDK while restoring both dependency and dotted parent-package target-module import state after cleanup. - Not tested: full end-to-end interactive Codex CLI launch. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [ ] 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 code change stays narrowly scoped to Codex memory config persistence, cleanup, and startup observability. It does not widen into larger memory-routing redesign or startup-failure recovery logic. |
||
|
Rudimar Ronsoni
|
b4571cc346
|
feat: headroom wrap opencode / unwrap opencode CLI (#1105)
## Summary This PR implements transparent `headroom wrap opencode` support without asking users to edit OpenCode provider URLs, choose an extra CLI flag, or maintain a static provider list. The wrapper now lives at the runtime transport boundary: OpenCode keeps its user/provider config, while Headroom intercepts outbound provider traffic in-process and routes it through the local Headroom proxy. ## What changed ### Transparent OpenCode wrapping - `headroom wrap opencode` injects the `headroom-opencode` plugin through `OPENCODE_CONFIG_CONTENT`. - Existing OpenCode provider URLs are preserved. We do not rewrite user config URLs to point at Headroom. - Existing `OPENAI_BASE_URL` and `ANTHROPIC_BASE_URL` env vars are preserved. - Local OpenCode traffic, localhost traffic, and Headroom proxy traffic bypass the shim to avoid loops. ### Runtime transport interception - Added an OpenCode plugin transport shim that wraps: - `globalThis.fetch` - `http.request` / `http.get` - `https.request` / `https.get` - External provider calls are routed to the local Headroom proxy. - The original upstream origin is passed through `x-headroom-base-url`, so the proxy can forward to the real provider without changing OpenCode config. - External `http2.connect` is blocked loudly instead of allowing direct provider traffic to leak outside Headroom. ### Live provider additions Provider coverage is no longer based on a static config scan. Because routing happens at outbound request time, providers added mid-session are routed through Headroom automatically as long as they use the covered Node transport paths. ### Subagent and child-process coverage - The parent OpenCode plugin sets a packaged Node preload shim through `NODE_OPTIONS=--import=.../hook-shim/handler.js`. - The transport shim patches `child_process.spawn`, `exec`, `execFile`, and `fork` so child Node processes receive the Headroom preload even when OpenCode passes a custom `env`. - The child-process shim fails closed if it loads without `HEADROOM_OPENCODE_TRANSPORT_PROXY_URL`. - This closes the subagent leak path where a child Node process could otherwise start without Headroom transport interception. ## Why this goes beyond PR #1089 PR #1089 improves OpenCode provider registration, but it still focuses on provider config shape. This PR moves the enforcement boundary to runtime transport interception. This PR goes further because: - No provider URL rewriting is required. - New providers added mid-session are covered automatically. - Subagents and child Node processes inherit the Headroom transport shim. - Direct external HTTP/2 paths fail loudly instead of leaking. - The wrap remains transparent to the user's OpenCode provider config. - The wrapper is fail-closed for unsupported child-process preload state. ## Additional robustness fixes While validating the change in Docker, the full Python suite exposed unrelated Linux/container robustness issues. These are fixed in this PR so the suite is green: - Binary cache handling now treats cache paths under a non-writable existing parent as unavailable, including when tests run as root in Docker. - `release_version.py` honors `MANUAL_VER` before git calls so direct script execution works outside a `.git` checkout. - Test logger isolation now resets relevant Headroom child loggers so proxy logging setup cannot poison later `caplog` tests. - The scanner missing-path test now uses a guaranteed missing `tmp_path` child instead of relying on `/nonexistent/path`. ## Validation All implementation validation was run inside Docker. - Full Python suite from a fresh Docker copy: `6605 passed, 523 skipped`. - Ruff on changed Python/OpenCode paths: passed. - OpenCode plugin typecheck: passed. - OpenCode plugin tests: `9 passed`. - OpenCode plugin build: passed. - Hook shim preload smoke test: passed. ## Notes This PR intentionally does not add a CLI option. `headroom wrap opencode` means full wrap. Either Headroom wraps OpenCode transparently, or the path fails loudly instead of silently leaking provider traffic. --------- Co-authored-by: Rudimar Ronsoni <6081613+rudironsoni@users.noreply.github.com> |
||
|
Logan Kang
|
c71592d421
|
feat(memory): add opt-in Apple-GPU (MPS) embedding runtime (#766)
## Description
On Apple-Silicon Macs — especially fanless models like the MacBook Air
(M5) — running the proxy with memory context injection can pin the CPU
while embedding. The embedding work runs an uncapped session on the CPU,
saturating multiple cores, which starves the proxy's asyncio loop and
leads to request timeouts.
This PR adds an **opt-in** runtime that offloads the memory embedder to
the Apple GPU (MPS). Setting `HEADROOM_EMBEDDER_RUNTIME=pytorch_mps`
routes embedding through the torch `sentence-transformers` backend on
MPS instead of the default ONNX CPU embedder, moving the work off the
CPU and keeping the proxy responsive.
The default behavior is unchanged — the feature is strictly opt-in,
env-var only, and falls through to the existing default embedder
selection (with a warning) whenever MPS or the torch dependencies are
unavailable.
Fixes: N/A — no tracking issue (surfaced while running codex auto-review
through
the proxy on a fanless MacBook Air (M5)).
## 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)
- [ ] Documentation update
- [x] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- **Runtime selection** (`headroom/proxy/memory_handler.py`): read
`HEADROOM_EMBEDDER_RUNTIME`; when set to `pytorch_mps` **and** MPS is
actually available, route the memory embedder to the torch
`sentence-transformers` backend (Apple GPU).
- If MPS is unavailable or torch/sentence-transformers is not installed,
log a warning and fall through to the existing default embedder
selection (ONNX when available, else the pre-existing local
sentence-transformers fallback) — no crash. The default (env var unset)
is unchanged. Env-var only
- **MPS serialization** (`headroom/memory/adapters/embedders.py`):
`LocalEmbedder` now funnels every `encode()` through a dedicated
single-worker `ThreadPoolExecutor` when the resolved device is MPS.
torch-MPS is not thread-safe, and the existing `run_in_executor(None,
...)` dispatch would otherwise let concurrent proxy requests call MPS
from multiple threads. CPU/CUDA keep the shared default executor
(behavior unchanged). `close()` also drops the cached model so re-use
after close re-initializes cleanly.
- **Packaging** (`pyproject.toml`): new `pytorch-mps` extra (`torch` +
`sentence-transformers`), **platform-gated to macOS** (`; sys_platform
== 'darwin'`) since MPS is Apple-Silicon-only. Deliberately left out of
`[all]` (its deps already arrive via `[ml]`/`[memory]`).
- **Tests** (`tests/test_memory/test_embedder_mps_serialization.py`):
regression coverage for the serialized executor, concurrency safety (no
SIGABRT), CPU-path default behavior, and close/re-use re-initialization.
- **Docs**: `wiki/{configuration,memory,macos-deployment}.md`,
`docs/content/docs/{configuration,installation,memory}.mdx`,
`README.md`, `CHANGELOG.md`.
## 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 (CPU-offload + concurrency profiling on
Apple Silicon)
## Test Output
```
$ pytest -v tests/test_memory/test_embedder_mps_serialization.py
collected 4 items
tests/test_memory/test_embedder_mps_serialization.py::test_cpu_uses_shared_executor PASSED [ 25%]
tests/test_memory/test_embedder_mps_serialization.py::test_mps_creates_single_worker_executor PASSED [ 50%]
tests/test_memory/test_embedder_mps_serialization.py::test_mps_concurrent_embeds_do_not_crash PASSED [ 75%]
tests/test_memory/test_embedder_mps_serialization.py::test_mps_reembed_after_close_recreates_executor PASSED [100%]
============================== 4 passed in 6.92s ===============================
$ ruff check headroom/memory/adapters/embedders.py headroom/proxy/memory_handler.py tests/test_memory/test_embedder_mps_serialization.py
All checks passed!
$ mypy headroom/memory/adapters/embedders.py headroom/proxy/memory_handler.py
Success: no issues found in 2 source files
$ pytest -q tests/test_memory/ tests/test_memory_handler_concurrent_init.py tests/test_memory_handler_native_ops.py
553 passed, 1 skipped in 13.51s
```
## 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
**Why MPS (and not CoreML or a thread cap):** measured on an
Apple-Silicon Mac, the default uncapped CPU embedding session saturates
the cores; the same model on MPS runs at a fraction of the CPU (≈8x
lower sustained CPU utilization in profiling) while producing
**byte-identical embeddings** (cosine distance ≈ 0 across runtimes), so
relevance/ranking is unchanged. A CoreML execution-provider path was
evaluated and rejected: the default optimized ONNX model uses fused ops
that fall back to CPU under CoreML (no offload), and a full-precision
re-export was impractical (very low throughput + multi-GB memory). MPS
via `sentence-transformers` was the only practical GPU offload.
**Why serialization is mandatory:** torch-MPS is not thread-safe —
concurrent encode calls from a multi-worker executor abort with
`-[IOGPUMetalCommandBuffer validate]: failed assertion 'commit an
already committed command buffer'` (reproduced deterministically; a
single-worker executor resolves it).
Under concurrent load the serialized single-GPU-stream throughput meets
or exceeds the parallel CPU path while using a fraction of the cores.
**Scope / boundary:** this targets the Python **memory** embedder, which
is live on the proxy request path (memory context injection). The
Rust-backed SmartCrusher compression path is unaffected and remains
non-configurable from Python by design.
**Safety:** default behavior is unchanged (ONNX, no torch). The feature
is opt-in, env-var only, macOS-gated at the packaging layer, and
degrades gracefully (warn + the existing default embedder selection)
when MPS or the dependencies are unavailable.
|
||
|
chopratejas
|
0be0eede9e |
fix(memory): traffic_learner indexes system-reminder fragments as user preferences (refs #464)
`TrafficLearner._extract_preferences` ran three regex patterns over raw
user-message text and saved any match as a `User preference: <captured>`
memory. Two compounding bugs made ~10% of the reporter's saved memories
(187 of 1796) garbage:
1. **System-reminder content was matched.** Claude Code injects
`<system-reminder>…</system-reminder>` blocks into user-role
messages — scaffolding ("don't mention this reminder", "use colgrep
instead of Grep", "never bypass signing") that hits every correction
trigger. The learner happily persisted scaffolding as authoritative
user preferences.
2. **Capture groups were fixed-length windows.** `(.{10,100})` grabbed
the next 10–100 chars with no boundary awareness, producing
mid-word truncations like `User preference: of Grep, Glob. When
spawning agents, mention colgrep features a`.
This change rewrites `_extract_preferences` to be **regex-free** and
adds two layered defences:
- `_strip_system_reminders` (literal `str.find` scan, no regex)
removes `<system-reminder>…</system-reminder>` blocks from user
text before any pattern matching. Unclosed reminders drop to
end-of-string. Case-insensitive on the tag name only. ~95% of the
reporter's noise sample comes from this single layer.
- A token-based correction scanner replaces the three `re.compile`
patterns. It tokenises on whitespace (lowercasing once, up front),
matches trigger sequences as ordered token lists (`don't`, `do not`,
`stop`, `never`, `avoid`, `no use`, `no try`, `no do`, `instead`),
and captures the trailing content until a sentence terminator
(`.!?\n`) or end-of-input. Captures shorter than 10 chars are
rejected (stray triggers), and captures that hit the 78/98-char cap
without finding a terminator are rejected (rambling fragments). The
former noise — `colgrep instead of Grep, Glob. When spawning…` —
fails this gate; short complete user utterances
(`don't use git push, I'll push manually`) still pass because
end-of-input counts as a boundary.
Net regex count in this file: -3, +0.
`_hydrate_persisted_state` already runs in `start()` and seeds
`_saved_hashes`/`_persisted_ids` from prior rows, so cross-restart
dedup is already wired up — the reporter's "doesn't survive restarts"
note was partially outdated. The narrow remaining edge (in-process
`_dedup_window=100` eviction within a single very-long-running
process) self-heals on next restart and is left as a separate
follow-up.
Tests: 17 new across `TestStripSystemReminders`,
`TestExtractPreferencesSystemReminderFiltering`,
`TestExtractPreferencesRealCorrections`, and
`TestExtractPreferencesSentenceBoundary`. Full traffic_learner suite:
139 passing. ci-precheck green.
|
||
|
Tejas Chopra
|
5ceca13c65 | fix: harden learn path handling across platforms | ||
|
Garm
|
4512a0626e |
test(traffic-learner): cover helper edge cases + apply ruff format
CI flagged two issues on the rebased branch: 1. ruff format --check failed on server.py and test_traffic_learner.py after the rebase; line-collapse / trailing-whitespace nits. 2. Codecov reported 80% patch coverage with 20 lines missing in the matcher helpers — mostly branches not exercised by the high-level tests (empty Levenshtein inputs, source-prefix Bash parsing, env-var skip, equal-string short-circuit in binary match, the substantive- token path that beats the edit-distance gate, error_recovery patterns with non-canonical content in _drop_contradictions). Adds 16 targeted unit tests for those branches and applies ruff format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Garm
|
606131451b |
fix(traffic-learner): tighten matchers and drop contradictions
The recovery matchers paired any failed and successful tool call within a 5-call window with no semantic check that the pair was actually a retry. This produced confidently-wrong rules like: File `state.rs` does not exist. The correct path is `lib.rs`. …where the user simply read two unrelated files in the same directory. Across sessions the same user can also typo in opposite directions, producing directly contradictory rules side by side. This commit adds three structural checks: 1. Read recovery: require the failed and successful basenames to be identical or close in Levenshtein distance. Rejects the "same dir, different file" case that was the most common noise source. 2. Bash recovery: require both commands to share a binary (allowing path-prefixed variants and short prefix-versions like `python` ↔ `python3`) AND either have low normalized edit distance or share a substantive non-flag token. Rejects pairs that share only the binary name but differ in every meaningful argument. 3. Contradiction filter on flush: detect A→B and B→A pairs in error_recovery patterns and drop both. They almost always indicate opposite-direction typos in different sessions, not stable advice. Also: stash failed_path in metadata so the contradiction filter and downstream consumers can reason about pairs without parsing content. Tests: 13 new tests covering the heuristics directly. Existing tests exercising legitimate recoveries (`python`→`python3`, `ruff`→`.venv/bin/ruff`, `pip install`→success) continue to pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Garm
|
a8ebf9ac5e |
test(traffic-learner): regression test for shutdown evidence gate
Asserts that stop()'s final flush_to_file does not bypass the evidence threshold. Earlier behavior collapsed the gate to 1 at shutdown, persisting every singleton pattern. This guards against that change sneaking back in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Garm
|
290238f398 |
fix(traffic-learner): raise min-evidence default and make it configurable
The traffic learner was emitting one-shot error_recovery patterns that contradicted each other and bloated MEMORY.md with low-signal noise. Two issues drove this: 1. The shutdown flush bypassed the evidence gate: the in-memory _min_evidence was set to 2, but on stop() the gate dropped to 1, so every singleton pattern got persisted at session end. This is the opposite of how evidence thresholding should work — singletons are the least trustworthy patterns, not the most. 2. The default min_evidence of 2 is too low to filter noise from the matchers, which pair up failed/successful tool calls within a small sliding window without a strong semantic check that the calls are actually related. Changes: - Raise default min_evidence from 2 to 5 in TrafficLearner. - Remove the shutdown-relaxation in flush_to_files; require self._min_evidence at all times, including on stop(). - Add traffic_learning_min_evidence to ProxyConfig (default 5). - Add --min-evidence CLI flag with HEADROOM_MIN_EVIDENCE envvar so users and embedded clients (desktop apps, plugins) can tune the threshold without source changes. - Thread the config value through HeadroomProxy into TrafficLearner. - Tests: cover default propagation and custom value flow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
ipapapa
|
d3c37d7098 |
feat(memory): resolve Qdrant connection from HEADROOM_QDRANT_* env vars (#31)
Adds `HEADROOM_QDRANT_URL`, `_HOST`, `_PORT`, `_API_KEY`, `_HTTPS`,
`_PREFER_GRPC`, `_GRPC_PORT` support across the memory stack:
- `headroom/memory/qdrant_env.py`: shared resolver helper with
explicit-arg > env > default precedence (URL wins over host/port;
booleans parsed via standard truthy set).
- `memory/easy.py`, `backends/{mem0,direct_mem0}.py`,
`proxy/memory_handler.py`: call the resolver so
`Memory(backend="qdrant-neo4j")`, `Mem0Config`, and the proxy's
`MemoryConfig` all honor the same env keys.
- `proxy/models.py` + `proxy/server.py`: `ProxyConfig` picks up the
same keys so hosted Qdrant (e.g. Qdrant Cloud) works without code
changes.
- `cli/proxy.py`: adds `--memory-qdrant-{url,host,port,api-key}`
flags that override the env when present.
- `tests/test_memory/test_qdrant_env.py`: unit coverage for
precedence, URL-vs-host/port, boolean parsing, and unset defaults.
- `CHANGELOG.md`: documented under [Unreleased] / Added.
Explicit constructor arguments still win; unset env keeps the existing
localhost:6333 defaults, so this is backwards-compatible.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Tejas Chopra
|
6dede0c2b4
|
Merge pull request #262 from gglucass/fix/traffic-learner-error-recovery
fix(memory): collapse and decay error_recovery patterns in MEMORY.md |
||
|
Tejas Chopra
|
32152f5202
|
Merge pull request #246 from Kayzo/fix/memory-batch-onnx-sqlitevec
fix(memory): batch onnx embeddings and sqlite-vec ops |
||
|
Garm
|
ac493cba1e |
test(memory): raise patch coverage from 83% to 98% on error_recovery fixes
26 new tests covering: - TestNormalizeBashForHash — empty string, no-suffix, head/tail strip, trailing context flags, stderr redirect, chain-boundary truncation - TestParseIsoTimestamp — None, empty, non-string, invalid format, naive (assumed UTC), tz-aware preserved - TestLoadPersistedPatternsTimestamps — reads first_seen_at/last_seen_at from metadata, falls back to created_at, collision-merges timestamps and bumps importance to max, handles malformed JSON and non-numeric importance cells gracefully - TestBumpPersistsLastSeenAt — verifies _bump_persisted_evidence writes $.last_seen_at into metadata JSON - TestHydrateLegacyRow — legacy rows without category, rows with unknown/invalid category, rows with empty content - TestCollectAllPatternsTimestamps — in-session re-sighting bumps last_seen_at past stale persisted timestamp - TestRefineErrorRecovery (additions) — refine-empties-section skips recommendation entirely, OSError during re-validation keeps the row, Read patterns without success_path skip re-validation cleanly Remaining uncovered lines in patch (4): defensive exception handlers in _hydrate_persisted_state (sqlite connect OperationalError, asyncio thread exception, JSONDecodeError on metadata) that require heavy mocking for marginal value. 91 tests pass, ruff + mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Garm
|
879064fea5 |
fix(memory): collapse and decay error_recovery patterns in MEMORY.md
The Learned: error recovery section was bloating with stale, near-duplicate, and contradictory entries because the dedup key was the literal rendered bullet text and there was no TTL or re-validation. - Normalize the hash key for error_recovery patterns. Read recoveries key on (basename(error_path), basename(success_path)); Bash recoveries strip volatile suffixes (| tail -N, 2>&1, etc.) and hash only the primary command before the first | or &&. Non-error-recovery categories keep literal-content hashing. - Stamp first_seen_at / last_seen_at on every pattern; bump both in _bump_persisted_evidence via json_set. Stored in metadata JSON — no schema change. - Refine at render time (error_recovery only): drop rows not re-observed in 21 days, re-validate Read success paths against the filesystem, collapse same-error_path-with-multiple-targets into one "use Glob/Grep first" bullet, rank by evidence_count * 0.5 ** (days/5), cap at 15 bullets. 15 new tests (TestNormalizedHash, TestRefineErrorRecovery). Full suite: 526 passed, 1 skipped. Ruff + mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Kayzo
|
f5cea7c51e |
fix(memory): batch onnx embeddings and sqlite-vec ops
Make the ONNX + sqlite-vec memory path truly batched. Batch ONNX embed_batch calls, batch sqlite-vec index/remove work under a single cached connection, and update MCP warm-up to use batch embed/save/index flows. Add focused regression tests for ONNX batching, sqlite-vec single-connection batch behavior, and MCP warm-up batching. Skip the MCP-specific test when optional MCP dependencies are not installed. Refs #240 |
||
|
Garm
|
b2536e602a |
test(learn): cover flush_to_file, backend edge cases, and hydrate/bump error paths
Adds 17 targeted tests to close the coverage gap on the new traffic_learner paths (codecov flagged ~51%). Exercises: - `flush_to_file` end-to-end with a fake learn plugin + writer: verifies anchored patterns are bucketed per project, recommendations are passed to the writer, writer exceptions are swallowed, and each early-return branch (no plugin, no patterns, discover_projects failure, un-anchored patterns) is hit without raising. - `_resolve_backend_db_path` on None backend, backend without `_config`, and backend with empty `db_path`. - `_collect_all_patterns` merging persisted + accumulator patterns by content_hash with summed evidence_count, plus the missing-DB branch. - `_hydrate_persisted_state` with backend=None and with a backend pointing at a non-existent DB file (both no-ops). - `_bump_persisted_evidence` with no backend, missing DB, and unknown memory id (all silent no-ops so the proxy hot path never blows up on malformed state). - `stop()` cancelling the flush task cleanly. All new tests use the existing `_FakeBackend` + `_init_db` helpers so they exercise real SQLite paths, not mocks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Garm
|
3e290b734b |
fix(learn): persist real evidence_count and bump on re-sighting
Before this change, every persisted traffic_learner row in memory.db
landed with evidence_count=1, causing two user-visible problems:
1. The live flush gate (evidence_count >= 2) filtered out every row, so
CLAUDE.md / MEMORY.md never received the patterns the learner saw
repeatedly.
2. _saved_hashes is in-memory only and reset on each proxy restart, so
a pattern seen once in session A then twice in session B would insert
a *duplicate* DB row instead of bumping the existing one. Users
accumulated many rows stuck at 1 instead of a few rows with high
evidence.
Root cause chain:
- _accumulate tracks a running count in the _pattern_counts tuple but
enqueues the ExtractedPattern dataclass with its default
evidence_count=1 intact.
- _save_worker writes pattern.evidence_count into metadata verbatim.
- After save, the hash goes into _saved_hashes and further sightings
are early-returned — never bumped.
- Next process start has empty _saved_hashes, so the same content goes
through the accumulator as fresh and gets re-saved.
Fix:
- _accumulate now sets pattern.evidence_count = count before enqueuing,
so DB rows reflect the real number of sightings at save time.
- _save_worker captures the Memory.id returned by save_memory and
records content_hash → id in a new _persisted_ids map.
- _accumulate's saved-hash branch now awaits
_bump_persisted_evidence(memory_id), which runs an atomic
json_set('$.evidence_count', existing + 1) UPDATE via
asyncio.to_thread to keep the proxy hot path non-blocking.
- start() calls a new _hydrate_persisted_state() that reads existing
traffic_learner rows' (id, content) pairs from the DB and pre-seeds
_saved_hashes + _persisted_ids. Cross-session re-sightings bump the
seeded row instead of inserting a duplicate.
- _load_persisted_patterns_from_sqlite and _hydrate_persisted_state
query by json_extract(metadata, '$.source') = 'traffic_learner'
instead of the prior LIKE on raw JSON — the bump path uses json_set,
which rewrites the metadata string without the default ": " spacing,
which would otherwise make the LIKE blind to bumped rows.
Adds TestEvidencePersistence with three cases:
- save persists the actual accumulated count (not the default 1)
- re-sightings bump the persisted row instead of creating duplicates
- a fresh learner hydrates _saved_hashes from DB, so cross-session
re-sightings bump the pre-existing row
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Garm
|
d9138a3ed8 |
feat(learn): live flush of traffic patterns to agent-native context files
Replaces the previous shutdown-only flush with a debounced, near-real-time dirty-flag flush worker that writes patterns into the correct CLAUDE.md / MEMORY.md bucket as traffic accumulates. - New FLUSH_DEBOUNCE_SECONDS gate (10s) prevents context-file thrash on bursty traffic while keeping updates "live" from the user's perspective. - TrafficLearner.start() now spawns a _flush_worker alongside the save worker; _accumulate() sets a dirty flag; _flush_worker() calls flush_to_file() when dirty and past the debounce window. - flush_to_file() now reads *both* persisted rows (memory.db) and the in-memory accumulator via _load_persisted_patterns_from_sqlite and _collect_all_patterns, so patterns survive proxy restarts and the agent-native files converge toward the full learned set. - Patterns are bucketed per-project via the learn plugin registry (plugin.discover_projects()) and anchored to project roots through longest-matching-path on content or entity_refs (_project_for_pattern). Un-anchored patterns are dropped. - Patterns are routed by PatternCategory to either CONTEXT_FILE (CLAUDE.md) or MEMORY_FILE (MEMORY.md) via _patterns_to_recommendations + _CATEGORY_TO_TARGET. - Live flushes require evidence_count >= 2; shutdown flushes accept single-evidence rows to avoid losing last-session signal. Adds tests for project routing, persisted-pattern loading, category routing, and the debounced flush worker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
chopratejas
|
5391761fe6 |
chore(memory): add EXTERNAL backend extension points
Mirrors the pattern used by headroom.ccr_backend so memory store,
vector index, and text index backends can be registered via setuptools
entry points.
- EXTERNAL enum value on StoreBackend, VectorBackend, TextBackend
- Optional *_backend_name fields on MemoryConfig
- entry_points(group=...) lookup in _create_{store,vector_index,text_index}
- New test_factory_external.py (7 tests) covering load / missing-name /
unknown-name paths
Default behavior (SQLITE + AUTO + FTS5) unchanged.
Extension groups:
headroom.memory_store
headroom.memory_vector
headroom.memory_text
|
||
|
chopratejas
|
d9cc4f3991 | Fix ruff lint errors in test files | ||
|
Tejas Chopra
|
0fd6dfcadb |
feat: add live traffic learning + cross-agent memory writers (--learn flag)
Live Traffic Learner extracts patterns from proxy traffic in real-time: - Error→recovery patterns (tool fails → next success teaches right approach) - Environment facts (working venv paths, test commands) - User preference signals (corrections, repeated choices) Agent-native memory writers export learned patterns to each agent's format: - Claude Code: MEMORY.md + per-topic files - Cursor: .cursor/rules/headroom-memory.mdc (YAML frontmatter) - Codex: AGENTS.md - Generic: plain markdown (Aider, Gemini, any agent) Memory Budget Manager handles token-optimized memory files: - Per-agent token budgets (2K Claude, 3K Cursor/Codex) - Temporal decay, staleness detection (git + filesystem) - Jaccard-similarity memory merging, dedup Opt-in via --learn flag on proxy/wrap commands: - headroom proxy --learn - headroom wrap claude --learn - --learn implies --memory; --no-learn overrides - compress() API completely unaffected (pure function) - Default behavior unchanged (no memory, no learning) |
||
|
Tejas Chopra
|
655df095fd |
feat(router): adaptive compression with Read lifecycle and context-pressure scaling
Enable ReadLifecycle by default so stale/superseded Read outputs are automatically replaced with compact CCR markers — these are provably safe to compress (file was edited or re-read). Replace static compression thresholds with adaptive parameters that scale with conversation length and context pressure: - protect_recent_reads_fraction: protects the most-recent 50% of messages from Read exclusion. Old Reads beyond this window become compressible, preventing the "28 excluded Read/Glob, 0 tokens saved" problem. - min_ratio_relaxed / min_ratio_aggressive: compression acceptance threshold interpolates linearly with context pressure (tokens / model limit). Low pressure → 0.85 (picky), high pressure → 0.65 (accept anything helpful). Eliminates the fixed 0.9 gate that was rejecting 20+ messages per request. Also adds --no-read-lifecycle CLI flag, and fixes a missing pytest.importorskip guard for sentence-transformers in memory tests. |
||
|
chopratejas
|
0adc39ab7a |
Fix CI: guard starlette imports, asyncio.run(), deprecate datetime.utcnow()
- Guard starlette imports in test_compress_api.py (skip ASGI tests without proxy deps) - Replace asyncio.get_event_loop().run_until_complete() with asyncio.run() (Python 3.13) - Replace datetime.utcnow() with datetime.now(timezone.utc).replace(tzinfo=None) everywhere |
||
|
chopratejas
|
7cf10675ea |
Add centralized ML model configuration
- Create headroom/models/config.py as single source of truth for all ML model defaults - Support environment variable overrides (HEADROOM_SENTENCE_TRANSFORMER, etc.) - Update all components to use ML_MODEL_DEFAULTS instead of hardcoded values - Switch LLMLingua default to smaller bert-base model (~350MB vs 1GB) - Total memory footprint reduced from ~1.6GB to ~980MB Updated files: - MLModelRegistry now resolves defaults from config - EmbeddingScorer, LocalEmbedder, TrainedRouter use config - All dataclass configs use field(default_factory=...) for consistency - Tests updated to handle auto-selected vector backends |
||
|
chopratejas
|
67d7db87cc | Fix test to expect VectorBackend.AUTO as default | ||
|
chopratejas
|
5e2186c42a |
Add multi-provider memory system with auto-detection
- Add MemoryToolAdapter for unified memory across providers - Anthropic: Uses native memory tool (memory_20250818) for subscription safety - OpenAI/Gemini/Others: Uses function calling format - All providers share the same semantic vector store backend - Simplify CLI to single --memory flag with auto-detection - Add proper resource cleanup (close methods) to fix test isolation - Update README with memory documentation |
||
|
Claude Code Bot
|
b6b8eed3bd |
fix(tests): skip memory tests when hnswlib not available
Add pytestmark skip conditions to memory test modules that depend on hnswlib (core_operations, factory, easy). The subprocess probe for hnswlib correctly detects unavailability on some platforms (like Python 3.13 CI runners), but these tests were still trying to run and failing with ImportError. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> |
||
|
chopratejas
|
5c740ea427 |
Add DiffCompressor and fix hnswlib SIGILL crash on CI
DiffCompressor: - Parse unified diff format and compress by reducing context lines - Preserve file headers and all +/- change lines - Score hunks by relevance (error keywords, query matches) - Add summary line: [N files, +X -Y lines] - Expected 30-50% savings on typical git diffs - Wire into content router for CompressionStrategy.DIFF - 30 tests covering parsing, compression, edge cases hnswlib SIGILL fix: - Move hnswlib import from module level to lazy loading - hnswlib crashes with SIGILL (Illegal Instruction) on CPUs without AVX support, before Python can catch the error - Now imports only when HNSWVectorIndex is actually used - HNSW_AVAILABLE is checked lazily via __getattr__ Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> |
||
|
chopratejas
|
2320405348 |
Fix asyncio event loop error in Python 3.10+ tests
Use asyncio.run() instead of asyncio.get_event_loop().run_until_complete() which raises RuntimeError in Python 3.10+ when no event loop exists. |