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()