fix(cache/semantic): don't evict an unrelated entry on an update at capacity (#2094)

## Description

`SemanticCache.put` can evict a perfectly good, unrelated entry when it
merely updates a key that is already cached.

The method runs its at-capacity eviction loop *before* it computes the
entry's key:

```python
self._cleanup_expired()

# Evict if at capacity
while len(self._cache) >= self.config.max_entries:
    self._evict_oldest()
...
key = messages_hash or self._generate_key(query)
...
self._cache[key] = entry
```

So when the same key is stored again while the cache is full (a
duplicate store, or a retried request that produces the same
`messages_hash`), the loop fires because `len == max_entries`, evicts
the LRU-oldest *distinct* entry, and only then overwrites the existing
key in place. Writing to an already-present key does not grow the map,
so nothing needed to be evicted — but an unrelated live entry is now
gone, and the next `get` for it is a false miss.

Concretely, with `max_entries=2` and keys `[h1, h2]`, re-storing `h2`
evicts `h1`, leaving `[h2]` even though only two distinct keys were ever
stored.

The sibling `CompressionCache.store_compressed` gets this right: it
deletes the existing key first, inserts, and only then trims — so
re-storing a present key never drops an unrelated entry.

## Fix

Compute the key first, then run the eviction loop only while the key is
genuinely new:

```python
key = messages_hash or self._generate_key(query)

while key not in self._cache and len(self._cache) >= self.config.max_entries:
    self._evict_oldest()
```

An in-place update of an existing key no longer evicts anything; adding
a new key still trims to make room exactly as before.

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/cache/semantic.py`: move the cache-key computation above the
eviction loop and gate the loop on `key not in self._cache` so an
in-place update never evicts.
- `tests/test_cache/test_semantic.py`: add
`test_update_at_capacity_does_not_evict_unrelated_entry`.
- `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/cache/semantic.py tests/test_cache/test_semantic.py
All checks passed!
$ python -m py_compile headroom/cache/semantic.py tests/test_cache/test_semantic.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 eviction logic with a
dependency-free script that replicates the `OrderedDict` +
`_evict_oldest` (popitem last=False) behavior for the old vs new loop,
and left the full pytest to CI.
- Exact command / steps: with `max_entries=2`, store `h1` then `h2`,
then re-store the already-present `h2`, under both the old loop (evict
before key dedup) and the new loop (evict only when key is new).
- Observed result: old loop leaves `['h2']` and `get(h1)` returns `None`
(h1 wrongly evicted); new loop leaves `['h1', 'h2']` with `get(h1)`
intact and `h2` updated. The regression test asserts h1 survives and h2
reflects the update.
- Not tested: a live embedding-backed cache round-trip; 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 is a localized reordering of two existing
statements plus a loop guard, verified by the standalone proof and the
new regression test for CI. This is a different defect from the earlier
messages-hash keying fix — that one was about which slot a request maps
to; this one is about eviction dropping a live entry on an in-place
update.

Co-authored-by: Tejas Chopra <chopratejas@gmail.com>

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

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,

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)