fix(cache/semantic): key entries by context hash, not query text (#2022)

## Description

`SemanticCache` (`headroom/cache/semantic.py`) derives each entry's key
from the **query text
only** — where `query` is just the trailing user message — and its
exact-match lookup returns
the slot without checking the stored entry's `messages_hash`:

```python
# put()
key = self._generate_key(query)          # sha256(query)[:16]
self._cache[key] = entry
if messages_hash:
    self._hash_index[messages_hash] = key

# get() — exact-match branch
key = self._hash_index.get(messages_hash)
if key and key in self._cache:
    entry = self._cache[key]
    ...
    return entry                         # never checks entry.messages_hash
```

So two requests that share a trailing user message but differ in earlier
context map to the
**same** key. The second `put` overwrites the first, and the first
request's `messages_hash`
still points at that (now overwritten) slot — so it is served the
**other conversation's**
cached response.

Trailing messages like `"continue"`, `"yes"`, `"fix it"`, `"run the
tests"` are extremely
common in agentic/coding sessions, so this collides constantly. It's
independent of the
proxy-level `_compute_key` fix (that's about what goes *into*
`messages_hash`; here the entry
is stored under a query-only key regardless of how good the hash is).
This `SemanticCache` is
the one used by the SDK client's `enable_semantic_cache` path.

Concretely:
1. `put("run the tests", A, messages_hash=HA)` → key `K = sha256("run
the tests")`; `_cache[K]=A`.
2. `put("run the tests", B, messages_hash=HB)` → same `K`; `_cache[K]`
overwritten with `B`.
3. `get("run the tests", HA)` → `_hash_index[HA]=K`, `K in _cache` →
returns **B**.

Closes: no issue filed — found while auditing the cache key derivation.

## Fix

1. Key entries by the full-context `messages_hash` when present, falling
back to the query hash
   only when no hash is supplied:
   ```python
   key = messages_hash or self._generate_key(query)
   ```
2. Defensively verify `entry.messages_hash == messages_hash` in the
exact-match branch of `get`,
   so any residual stale mapping becomes a miss rather than wrong data.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `headroom/cache/semantic.py`: key `put` entries by `messages_hash`
when present; verify `entry.messages_hash` in the `get` exact-match
branch.
- `tests/test_cache/test_semantic.py`: add
`test_same_query_different_context_does_not_collide` and
`test_exact_match_verifies_messages_hash`.

## Testing

- [x] New regression tests added (`tests/test_cache/test_semantic.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/cache/semantic.py tests/test_cache/test_semantic.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 verified the `put`/`get`
logic with a dependency-free script and left the full pytest to CI.
- Exact command / steps: stored responses A and B under the same query
`"run the tests"` with different `messages_hash`, then read each hash
back — through both the old (query-keyed) and new (hash-keyed) logic.
- Observed result: the old logic serves B's response to request A; the
new logic isolates them:

```text
OLD: A->RESPONSE_B  B->RESPONSE_B
NEW: A->RESPONSE_A  B->RESPONSE_B
SEMANTIC CACHE COLLISION FIX VERIFIED (OLD served B to A; NEW isolates)
```

- Not tested: the full SDK `HeadroomClient` round-trip with
`enable_semantic_cache=True` (needs the heavy stack). The fix is
confined to `SemanticCache.put`/`get` and the new tests drive them
directly. 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 logic check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

- Small, contained fix — the key derivation plus a verification guard,
no new dependencies.
- @JerrettDavis tagging you — this one can serve one conversation's
cached response to another when the last message matches, so it seemed
worth surfacing. Thanks!
This commit is contained in:
Abhay Singh 2026-07-11 20:41:09 +05:30 committed by GitHub
parent f8431240b9
commit d8783ab89b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 35 additions and 5 deletions

View file

@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Bug Fixes
* **cache/semantic:** key entries by the full-context hash, not the trailing query text. `SemanticCache.put` stored each response under `sha256(query)[:16]` where `query` is only the last user message, and the exact-match branch of `get` returned the slot without checking the stored entry's `messages_hash`. Two requests that share a trailing message ("continue", "yes", "run the tests") but differ in earlier context therefore collided on one slot — the second overwrote the first, and the first's hash then resolved to the second's cached response (wrong data served). Entries are now keyed by `messages_hash` when present, and `get` verifies `entry.messages_hash` before returning.
* **proxy/openai:** stop PRE_SEND from reintroducing `tools: []` after the direct #728 fix. The OpenAI request handler now mirrors the existing `tools or _original_tools is not None` body-write guard during PRE_SEND write-back, so providers that reject empty tool arrays no longer see a tools field when the client omitted it, while explicit client `tools: []` remains preserved ([#1983](https://github.com/headroomlabs-ai/headroom/issues/1983)).
* **proxy/openai:** keep the exact Responses function name `terminal` resident during OpenAI tool-search deferral so cache-mode optimization stops forwarding `terminal.terminal` and triggering the reserved-namespace 400 on Codex Responses ([#1946](https://github.com/headroomlabs-ai/headroom/issues/1946)).
* **proxy/openai:** thread the savings-profile kwargs into the live `/v1/chat/completions` compression path. The chat handler called `openai_pipeline.apply()` without `proxy_pipeline_kwargs(config)`, so `HEADROOM_SAVINGS_PROFILE=agent-90` (and the individual `compress_user_messages`/`target_ratio`/`min_tokens_to_compress`/... knobs) were silently dropped — OpenAI-compatible clients like OpenCode kept protecting user messages and missed the configured profile. Both the token-mode and non-token chat branches now pass the profile kwargs, matching `handlers/anthropic.py` and the dedicated OpenAI compress endpoint ([#1534](https://github.com/headroomlabs-ai/headroom/issues/1534)).

View file

@ -152,9 +152,13 @@ class SemanticCache:
key = self._hash_index.get(messages_hash)
if key and key in self._cache:
entry = self._cache[key]
self._touch(key)
self._hits += 1
return entry
# Verify the stored entry really belongs to this request. Guards
# against a stale index mapping ever pointing at an entry that was
# overwritten by a different conversation sharing the same key.
if entry.messages_hash == messages_hash:
self._touch(key)
self._hits += 1
return entry
# Try semantic similarity if we have embedding function
if self._embedding_fn:
@ -197,8 +201,12 @@ class SemanticCache:
if self._embedding_fn:
embedding = self._embedding_fn(query)
# Create cache key
key = self._generate_key(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
# overwrite each other. Fall back to the query hash only when no
# messages_hash is supplied (e.g. embedding-only usage).
key = messages_hash or self._generate_key(query)
now = time.time()
entry = CacheEntry(

View file

@ -51,6 +51,27 @@ class TestSemanticCache:
entry = cache.get("Unknown query", messages_hash="unknown")
assert entry is None
def test_same_query_different_context_does_not_collide(self, cache):
"""Two requests that share a trailing user message but differ in earlier
context (distinct messages_hash) must not overwrite each other. Before the
fix both were keyed by sha256(query), so the second clobbered the first and
the first's hash resolved to the second's response."""
cache.put("run the tests", {"text": "response A"}, messages_hash="ctxA")
cache.put("run the tests", {"text": "response B"}, messages_hash="ctxB")
got_a = cache.get("run the tests", messages_hash="ctxA")
got_b = cache.get("run the tests", messages_hash="ctxB")
assert got_a is not None and got_a.response == {"text": "response A"}
assert got_b is not None and got_b.response == {"text": "response B"}
def test_exact_match_verifies_messages_hash(self, cache):
"""A stored entry is only returned when its messages_hash matches the
looked-up hash never another conversation's cached response."""
cache.put("continue", {"text": "A"}, messages_hash="hA")
# A lookup for a hash that isn't stored is a miss, not a wrong hit.
assert cache.get("continue", messages_hash="hB") is None
def test_lru_eviction(self):
"""Test LRU eviction when at capacity."""
config = SemanticCacheConfig(max_entries=3)