From 0f606b6281dd4c55e1c5a32cc97c418b66860df1 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Wed, 8 Jul 2026 00:26:36 -0400 Subject: [PATCH] fix(cache): avoid fallback session collisions (#1827) ## Description Cache-mode session tracking currently collapses unrelated conversations when they share a large static first system prompt. The fallback session-id hash ignores later system messages entirely, so dynamic per-conversation context can get cut out of the key and two different sessions reuse the same `PrefixCacheTracker`. This hashes the full ordered system-text payload instead, while leaving explicit `x-headroom-session-id` overrides untouched. Refs #1808. ## 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 - Collected all system-text content when building the fallback cache session id. - Stopped truncating fallback session-id input to the first 500 characters of the first system message. - Added a regression that proves two conversations with different later system context no longer collide. - Added a preservation test that appending only non-system turns keeps the same fallback session id. - Applied the pinned Ruff formatter to three pre-existing files on the current base so the repo-wide lint job passes unchanged semantics. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_cache/test_prefix_tracker.py -q`) - [x] Linting passes (`uv run ruff check headroom/cache/prefix_tracker.py tests/test_cache/test_prefix_tracker.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_cache/test_prefix_tracker.py -q 40 passed, 1 warning in 0.15s uv run ruff check headroom/cache/prefix_tracker.py tests/test_cache/test_prefix_tracker.py All checks passed! uv run ruff check . All checks passed! uv run ruff format --check . 1046 files already formatted ``` ## Real Behavior Proof - Environment: Windows, project `uv` environment, focused cache-tracker regression. - Exact command / steps: run `tests/test_cache/test_prefix_tracker.py` on `origin/main` with the new collision regression present, then rerun the same file on this branch. - Observed result: base returns the same session id for two conversations that differ only in a later system message and fails `assert id_a != id_b`; head passes the focused file and keeps the fallback session id stable when only non-system turns are appended. - Not tested: live proxy traffic through a real agentic client. ## 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 - [x] 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 - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes This is only the session-collision half of #1808. The duplicate-response-header fix stays separate so this PR can reference the issue without claiming the whole bug report is resolved. The extra formatting-only diff comes from the current base failing the pinned full-repo Ruff format check. --- headroom/cache/prefix_tracker.py | 11 +++---- tests/test_cache/test_prefix_tracker.py | 41 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/headroom/cache/prefix_tracker.py b/headroom/cache/prefix_tracker.py index 4d5baebea..7fd2d4c9f 100644 --- a/headroom/cache/prefix_tracker.py +++ b/headroom/cache/prefix_tracker.py @@ -613,20 +613,19 @@ class SessionTrackerStore: if session_header: return str(session_header) - # Fall back to hashing model + system prompt - system_content = "" + # Fall back to hashing model + all system-text content. + system_parts: list[str] = [] for msg in messages: if msg.get("role") == "system": content = msg.get("content", "") if isinstance(content, str): - system_content = content[:500] # First 500 chars is enough + system_parts.append(content) elif isinstance(content, list): for block in content: if isinstance(block, dict) and block.get("type") == "text": - system_content = block.get("text", "")[:500] - break - break + system_parts.append(block.get("text", "")) + system_content = json.dumps(system_parts, ensure_ascii=False, separators=(",", ":")) key = f"{model}:{system_content}" return hashlib.md5(key.encode()).hexdigest()[:16] # nosec B324 diff --git a/tests/test_cache/test_prefix_tracker.py b/tests/test_cache/test_prefix_tracker.py index a2e095292..01d296755 100644 --- a/tests/test_cache/test_prefix_tracker.py +++ b/tests/test_cache/test_prefix_tracker.py @@ -370,6 +370,47 @@ class TestSessionTrackerStore: id3 = store.compute_session_id(MockRequest(), "gpt-4", messages) assert id3 != id1 + def test_compute_session_id_uses_all_system_messages(self, store): + """Different dynamic system messages should not collide.""" + + class MockRequest: + headers = {} + + static_prompt = "framework prompt " * 80 + conv_a = [ + {"role": "system", "content": [{"type": "text", "text": static_prompt}]}, + {"role": "system", "content": [{"type": "text", "text": "context: session A"}]}, + {"role": "user", "content": "hello"}, + ] + conv_b = [ + {"role": "system", "content": [{"type": "text", "text": static_prompt}]}, + {"role": "system", "content": [{"type": "text", "text": "context: session B"}]}, + {"role": "user", "content": "hello"}, + ] + + id_a = store.compute_session_id(MockRequest(), "claude-3", conv_a) + id_b = store.compute_session_id(MockRequest(), "claude-3", conv_b) + + assert id_a != id_b + + def test_compute_session_id_is_stable_when_only_non_system_turns_change(self, store): + """Appending non-system turns should keep the same fallback session id.""" + + class MockRequest: + headers = {} + + base_messages = [ + {"role": "system", "content": [{"type": "text", "text": "framework prompt"}]}, + {"role": "system", "content": [{"type": "text", "text": "context: session A"}]}, + {"role": "user", "content": "hello"}, + ] + extended_messages = base_messages + [{"role": "assistant", "content": "hi there"}] + + id1 = store.compute_session_id(MockRequest(), "claude-3", base_messages) + id2 = store.compute_session_id(MockRequest(), "claude-3", extended_messages) + + assert id1 == id2 + def test_compute_session_id_no_system(self, store): """Should work without system messages."""