From 1725cd1f8376828849c0e48fd9008afa25d24fff Mon Sep 17 00:00:00 2001
From: Abhay Singh <abhaysingh0293@gmail.com>
Date: Mon, 13 Jul 2026 20:24:16 +0530
Subject: [PATCH] fix(memory): key the embedder cache on ollama_base_url
 (#2109)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 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>
---
 CHANGELOG.md                                  |  1 +
 headroom/memory/factory.py                    |  8 +++
 .../test_factory_embedder_cache.py            | 52 +++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 tests/test_memory/test_factory_embedder_cache.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fd77ce059..13911c1bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Bug Fixes
 
+* **memory:** include the Ollama server URL in the embedder cache key so a second backend can't get an embedder bound to the wrong server. `_create_embedder` cached by `(backend, model)` only, but the Ollama embedder is constructed with `base_url=config.ollama_base_url`. Two configs in the same process that shared a backend and model but pointed at different Ollama servers (e.g. a per-project storage router) collided on one cache slot, so the second silently reused the first's embedder and embedded against the wrong host. The cache key now also includes `ollama_base_url`.
 * **tokenizers:** use `o200k_base` for the gpt-4.1 / gpt-4.5 / o4 families in `get_encoding_for_model`. `gpt-4.1*` and `gpt-4.5*` matched the broad `gpt-4` prefix and were encoded with `cl100k_base`, and `o4*` matched no prefix and fell through to the `cl100k_base` default — all three use `o200k_base`, so their token counts were computed with the wrong vocabulary. Added explicit `gpt-4.1`/`gpt-4.5` prefixes ahead of `gpt-4` and an `o4` prefix; `gpt-4` and `gpt-3.5` snapshots still resolve to `cl100k_base`.
 * **cache/ccr:** stop counting a successful eviction as a retrieval in the compression feedback learner, which inverted the learning signal. When an entry is evicted without ever being retrieved, `CompressionStore` emits a synthetic `retrieval_type="eviction_success"` event to mark that the compression was sufficient (the LLM never needed the original). `CompressionFeedback.record_retrieval` had no branch for it, so — because the type is not `"full"` — it was counted as a *search retrieval*, inflating the tool's `retrieval_rate`/`search_rate`. `get_compression_hints` reads a high retrieval rate as "compressing too aggressively" and backs off, so a compression that actually worked pushed the learner toward *less* compression (a standalone repro scores one successful eviction as a 100% retrieval rate). The event is now recognized and left out of the retrieval counters; the compression is still counted by `record_compression` at store time, so a never-retrieved entry correctly yields a low retrieval rate. Genuine retrievals are unaffected.
 * **proxy/anthropic:** don't launder a non-2xx upstream into HTTP 200 when enterprise security scans the response. On the non-streaming `/v1/messages` path the response-scan branch rebuilt the reply as `httpx.Response(status_code=200)` and returned it without checking the upstream status, so a rate-limit (429), overloaded (529), or other 4xx error whose JSON body was scanned reached the client as an HTTP 200 — the client's retry/backoff never fired and an error looked like success. The branch is now gated on a 200 upstream, matching the sibling CCR/cache/buffered-stream blocks in the same handler; non-2xx responses fall through and keep their real status.
diff --git a/headroom/memory/factory.py b/headroom/memory/factory.py
index 271279eed..94e7153d8 100644
--- a/headroom/memory/factory.py
+++ b/headroom/memory/factory.py
@@ -173,6 +173,14 @@ def _create_embedder(config: MemoryConfig) -> Embedder:
         if hasattr(config.embedder_backend, "value")
         else str(config.embedder_backend),
         config.embedder_model or "",
+        # The Ollama backend is built with ``base_url=config.ollama_base_url``,
+        # so two configs that share a backend and model but point at different
+        # Ollama servers must NOT share a cached embedder — otherwise the second
+        # caller silently gets an embedder bound to the first server. (The
+        # ``openai_api_key`` omission is handled by the up-front validation
+        # above; ``ollama_base_url`` has no such guard and would just resolve to
+        # the wrong host.)
+        config.ollama_base_url or "",
     )
 
     with _EMBEDDER_CACHE_LOCK:
diff --git a/tests/test_memory/test_factory_embedder_cache.py b/tests/test_memory/test_factory_embedder_cache.py
new file mode 100644
index 000000000..5a0ab9c0a
--- /dev/null
+++ b/tests/test_memory/test_factory_embedder_cache.py
@@ -0,0 +1,52 @@
+"""The embedder cache must not serve an embedder bound to the wrong server.
+
+Kept out of ``test_factory.py`` (which skips wholesale without hnswlib) because
+these cases only construct the lightweight Ollama embedder and need no vector
+index.
+"""
+
+from __future__ import annotations
+
+from headroom.memory.config import EmbedderBackend, MemoryConfig
+from headroom.memory.factory import _create_embedder, _reset_embedder_cache_for_tests
+
+
+def test_ollama_embedder_cache_keys_on_base_url():
+    """Two configs that share backend + model but differ in ollama_base_url must
+    not share a cached embedder — the second would otherwise get an embedder
+    bound to the first server."""
+    _reset_embedder_cache_for_tests()
+    try:
+        cfg1 = MemoryConfig(
+            embedder_backend=EmbedderBackend.OLLAMA,
+            embedder_model="nomic-embed-text",
+            ollama_base_url="http://gpu1:11434",
+        )
+        cfg2 = MemoryConfig(
+            embedder_backend=EmbedderBackend.OLLAMA,
+            embedder_model="nomic-embed-text",
+            ollama_base_url="http://gpu2:11434",
+        )
+
+        e1 = _create_embedder(cfg1)
+        e2 = _create_embedder(cfg2)
+
+        assert e1 is not e2
+        assert e1._base_url == "http://gpu1:11434"
+        assert e2._base_url == "http://gpu2:11434"
+    finally:
+        _reset_embedder_cache_for_tests()
+
+
+def test_ollama_embedder_cache_reuses_same_base_url():
+    """Same backend + model + base_url still hits the cache (one model load)."""
+    _reset_embedder_cache_for_tests()
+    try:
+        cfg = MemoryConfig(
+            embedder_backend=EmbedderBackend.OLLAMA,
+            embedder_model="nomic-embed-text",
+            ollama_base_url="http://gpu1:11434",
+        )
+        assert _create_embedder(cfg) is _create_embedder(cfg)
+    finally:
+        _reset_embedder_cache_for_tests()