diff --git a/CHANGELOG.md b/CHANGELOG.md index f161cf555..f5f7811f7 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 +* **cache/semantic:** don't evict an unrelated entry when re-storing a key that is already cached. `SemanticCache.put` ran its at-capacity eviction loop before computing the entry's key, so overwriting a key that was already present (a duplicate or retried store) still evicted the LRU-oldest distinct entry even though an in-place update grows nothing. That silently dropped a live entry and turned a later lookup for it into a false cache miss. The key is now computed first and the eviction loop only runs when the key is genuinely new (mirroring `CompressionCache.store_compressed`, which deletes-then-inserts). * **tokenizers:** stop `TiktokenCounter.count_messages` from exploding on non-text content blocks. Its multi-part branch handled only `text` and OpenAI `image_url`; every other shape (Anthropic `image`/`tool_result`/`tool_use`, Strands blocks) fell through to `count_text(str(part))`, which json-stringified the base64 payload and tokenized it as text — a 1MB image counted as ~330K phantom tokens (~218x overcount in a standalone repro), corrupting every downstream budgeting/compression decision for multimodal OpenAI-model requests. Unknown block shapes now delegate to the base `_count_content_parts`, which prices images/documents by a bounded estimate (the overcount that helper already exists to prevent). * **install:** don't let a host env export override the manifest in persistent-docker deployments. `build_runtime_command` emitted the manifest's pinned `--env NAME=VALUE` pairs and then, for every host var matching a passthrough prefix, a bare `--env NAME`. Docker resolves duplicate `--env` last-wins, so a stale host export (e.g. `HEADROOM_BACKEND=anyllm`) that shared a passthrough prefix with a pinned manifest value (`HEADROOM_BACKEND=anthropic`) was appended after it and silently won, diverging the container from its deployment config. The bare passthrough is now skipped for any name the manifest already pins. * **memory:** honor explicit `store=false` on OpenAI `/v1/responses` requests by skipping Headroom memory-tool injection that depends on stored-response continuations. Memory context injection stays available, and requests no longer get rewritten to `store=true` behind the client's back ([#1944](https://github.com/headroomlabs-ai/headroom/issues/1944)). diff --git a/headroom/cache/semantic.py b/headroom/cache/semantic.py index bcf968a03..0ea2effba 100644 --- a/headroom/cache/semantic.py +++ b/headroom/cache/semantic.py @@ -192,15 +192,6 @@ class SemanticCache: """ self._cleanup_expired() - # Evict if at capacity - while len(self._cache) >= self.config.max_entries: - self._evict_oldest() - - # Generate embedding if available - embedding: list[float] = [] - if self._embedding_fn: - embedding = self._embedding_fn(query) - # Create cache key. Prefer the full-context hash: two requests that share # a trailing user message ("continue", "yes", "run the tests") but differ # in earlier context must NOT collide on one query-derived slot and @@ -208,6 +199,20 @@ class SemanticCache: # messages_hash is supplied (e.g. embedding-only usage). key = messages_hash or self._generate_key(query) + # Evict if adding a NEW key would exceed capacity. Overwriting a key that + # is already present is an in-place update that does not grow the map, so + # it must NOT evict — the old code ran the eviction loop before computing + # the key, so re-storing an existing entry at capacity dropped an + # unrelated live entry and turned a later lookup for it into a false miss. + # (Mirrors CompressionCache.store_compressed, which deletes-then-inserts.) + while key not in self._cache and len(self._cache) >= self.config.max_entries: + self._evict_oldest() + + # Generate embedding if available + embedding: list[float] = [] + if self._embedding_fn: + embedding = self._embedding_fn(query) + now = time.time() entry = CacheEntry( embedding=embedding, diff --git a/tests/test_cache/test_semantic.py b/tests/test_cache/test_semantic.py index 8cbb15da9..c24d261f9 100644 --- a/tests/test_cache/test_semantic.py +++ b/tests/test_cache/test_semantic.py @@ -96,6 +96,31 @@ class TestSemanticCache: assert cache.get("query3", messages_hash="h3") is not None assert cache.get("query4", messages_hash="h4") is not None + def test_update_at_capacity_does_not_evict_unrelated_entry(self): + """Re-storing an existing key at capacity must not drop another entry. + + The eviction loop used to run before the cache key was computed, so + overwriting a key that was already present (a retried/duplicate store) + still evicted the LRU-oldest distinct entry even though the update grows + nothing. That silently dropped a live entry and turned a later lookup for + it into a false miss. + """ + config = SemanticCacheConfig(max_entries=2) + cache = SemanticCache(config) + + cache.put("query1", "response1", messages_hash="h1") + cache.put("query2", "response2", messages_hash="h2") + + # Re-store the already-present h2 (e.g. a duplicate/retried request). + cache.put("query2", "response2b", messages_hash="h2") + + # h1 must still be there — updating h2 must not evict it. + got1 = cache.get("query1", messages_hash="h1") + assert got1 is not None and got1.response == "response1" + # h2 reflects the update. + got2 = cache.get("query2", messages_hash="h2") + assert got2 is not None and got2.response == "response2b" + def test_ttl_expiration(self): """Test TTL expiration.""" config = SemanticCacheConfig(ttl_seconds=1)