fix(cache): include request-shaping parameters in the prompt cache key (#90) (#91)

The prompt cache key was sha256(model + [role, content]) while the request
surface forwarded upstream also carries tools, tool_choice, response_format,
reasoning_effort, thinking, max_tokens, temperature, top_p, n, and arbitrary
provider extras. Two requests with the same messages therefore shared a cache
entry, so a plain chat answer could be served to a later request that asked
for structured output or tool calls, and a response generated under a high
max_tokens could be served to a request that asked for a low one. The cache
sits in front of the provider call, so this silently overrides the client's
request contract.

- request_cache_params() collects the declared response-shaping fields plus
  everything in model_extra except stream, and PromptCache.get/put take it as
  an optional params argument folded into the key.
- Message normalization now keeps model_extra (tool_call_id, name,
  tool_calls), so tool-result turns with identical text no longer collide.
- json.dumps uses default=str so a non-serializable extra degrades the key
  instead of raising.

Both call sites in server.py pass the same params dict, so a lookup and the
insert that follows it cannot drift.

Co-authored-by: Nadir <info@getnadir.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 3 people

CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to NadirClaw will be documented in this file.
 
 ## [Unreleased]
 
+### Fixed
+- **The prompt cache ignored every request parameter except the model and message text, so it could answer a request with a response that does not satisfy it** (#90). The key was `sha256(model + [role, content])`, while the request surface forwarded upstream also carries `tools`, `tool_choice`, `response_format`, `reasoning_effort`, `thinking`, `max_tokens`, `temperature`, `top_p`, `n`, and arbitrary provider extras. Two requests with the same messages therefore shared a cache entry: a plain chat answer could be served to a later request asking for `response_format: {"type": "json_schema"}` or for `tool_calls`, and a response generated under a high `max_tokens` could be served to a request that asked for a low one — silently overriding the client's contract, since the cache sits in front of the provider call. The key now includes every response-shaping field (`request_cache_params()` collects the declared ones plus everything in `model_extra` except `stream`), and message normalization keeps `tool_call_id` / `name` / `tool_calls` so tool-result turns with identical text no longer collide. Non-serializable extras fall back to `repr` rather than raising.
+
 ## [0.23.0] - 2026-08-31
 
 ### Added

nadirclaw/cache.py

@@ -31,19 +31,53 @@ def _cache_max_size() -> int:
     return int(os.getenv("NADIRCLAW_CACHE_MAX_SIZE", "1000"))
 
 
-def _make_cache_key(model: str, messages: list) -> str:
-    """Build a deterministic cache key from model + messages (ignoring temperature/stream)."""
-    # Normalize messages to just role + content
-    normalized = []
-    for m in messages:
-        if hasattr(m, "role"):
-            normalized.append({"role": m.role, "content": m.text_content() if hasattr(m, "text_content") else str(m.content)})
-        elif isinstance(m, dict):
-            normalized.append({"role": m.get("role", ""), "content": m.get("content", "")})
-        else:
-            normalized.append(str(m))
-
-    blob = json.dumps({"model": model or "", "messages": normalized}, sort_keys=True)
+# Declared request fields that change the upstream response and so must be part
+# of the cache key. Everything else the client sends (tools, tool_choice,
+# response_format, reasoning_effort, thinking, provider extras) arrives in
+# model_extra and is included wholesale -- see request_cache_params.
+_KEYED_FIELDS = ("temperature", "top_p", "max_tokens", "n")
+
+
+def request_cache_params(request: Any) -> Dict[str, Any]:
+    """Extract every request field that shapes the response, for the cache key.
+
+    Only ``stream`` is ignored: it changes the transport, not the content, and
+    the cache is consulted on non-streaming requests only.
+    """
+    params: Dict[str, Any] = {f: getattr(request, f, None) for f in _KEYED_FIELDS}
+    extra = getattr(request, "model_extra", None) or {}
+    params.update({k: v for k, v in extra.items() if k != "stream"})
+    return {k: v for k, v in params.items() if v is not None}
+
+
+def _normalize_message(m: Any) -> Any:
+    """Reduce a message to the parts that affect the response."""
+    if hasattr(m, "role"):
+        norm = {
+            "role": m.role,
+            "content": m.text_content() if hasattr(m, "text_content") else str(m.content),
+        }
+        # tool_call_id / name / tool_calls ride along in model_extra
+        norm.update(getattr(m, "model_extra", None) or {})
+        return norm
+    if isinstance(m, dict):
+        norm = dict(m)
+        norm["content"] = m.get("content", "")
+        return norm
+    return str(m)
+
+
+def _make_cache_key(model: str, messages: list, params: Optional[Dict[str, Any]] = None) -> str:
+    """Build a deterministic cache key from model + messages + response-shaping params."""
+    blob = json.dumps(
+        {
+            "model": model or "",
+            "messages": [_normalize_message(m) for m in messages],
+            "params": params or {},
+        },
+        sort_keys=True,
+        default=str,
+    )
     return hashlib.sha256(blob.encode()).hexdigest()
 
 
@@ -58,9 +92,11 @@ def __init__(self, max_size: int | None = None, ttl: int | None = None):
         self._hits = 0
         self._misses = 0
 
-    def get(self, model: str, messages: list) -> Optional[Dict[str, Any]]:
+    def get(
+        self, model: str, messages: list, params: Optional[Dict[str, Any]] = None
+    ) -> Optional[Dict[str, Any]]:
         """Look up a cached response. Returns None on miss or expiry."""
-        key = _make_cache_key(model, messages)
+        key = _make_cache_key(model, messages, params)
         with self._lock:
             if key in self._cache:
                 ts, data = self._cache[key]
@@ -76,9 +112,15 @@ def get(self, model: str, messages: list) -> Optional[Dict[str, Any]]:
             self._misses += 1
             return None
 
-    def put(self, model: str, messages: list, response: Dict[str, Any]) -> None:
+    def put(
+        self,
+        model: str,
+        messages: list,
+        response: Dict[str, Any],
+        params: Optional[Dict[str, Any]] = None,
+    ) -> None:
         """Store a response in the cache."""
-        key = _make_cache_key(model, messages)
+        key = _make_cache_key(model, messages, params)
         with self._lock:
             if key in self._cache:
                 self._cache.move_to_end(key)

nadirclaw/server.py

@@ -1587,12 +1587,13 @@ async def chat_completions(
         # ------------------------------------------------------------------
         # Prompt cache — check before calling the model
         # ------------------------------------------------------------------
-        from nadirclaw.cache import _cache_enabled, get_prompt_cache
+        from nadirclaw.cache import _cache_enabled, get_prompt_cache, request_cache_params
 
         prompt_cache = get_prompt_cache()
+        cache_params = request_cache_params(request)
         cache_hit = False
         if _cache_enabled() and not request.stream:
-            cached_response = prompt_cache.get(selected_model, request.messages)
+            cached_response = prompt_cache.get(selected_model, request.messages, cache_params)
             if cached_response is not None:
                 response_data = cached_response
                 cache_hit = True
@@ -1683,7 +1684,7 @@ async def _true_stream_wrapper():
 
             # Store in prompt cache
             if _cache_enabled():
-                prompt_cache.put(selected_model, request.messages, response_data)
+                prompt_cache.put(selected_model, request.messages, response_data, cache_params)
         else:
             elapsed_ms = int((time.time() - start_time) * 1000)
             total_tokens = response_data["prompt_tokens"] + response_data["completion_tokens"]

tests/test_cache.py

@@ -2,7 +2,7 @@
 
 import time
 
-from nadirclaw.cache import PromptCache, _make_cache_key
+from nadirclaw.cache import PromptCache, _make_cache_key, request_cache_params
 
 
 class TestMakeCacheKey:
@@ -28,6 +28,66 @@ def test_key_is_hex_string(self):
         assert isinstance(key, str)
         assert len(key) == 64  # sha256 hex
 
+    def test_response_shaping_params_change_key(self):
+        """Params that change the upstream response must change the key (issue #90)."""
+        msgs = [{"role": "user", "content": "Return a city name"}]
+        base = _make_cache_key("gpt-4", msgs)
+        for params in (
+            {"response_format": {"type": "json_object"}},
+            {"tools": [{"type": "function", "function": {"name": "get_weather"}}]},
+            {"tool_choice": "required"},
+            {"max_tokens": 16},
+            {"temperature": 0.9},
+            {"top_p": 0.1},
+            {"n": 3},
+            {"reasoning_effort": "high"},
+            {"thinking": {"type": "enabled", "budget_tokens": 1024}},
+        ):
+            assert _make_cache_key("gpt-4", msgs, params) != base, params
+
+    def test_same_params_same_key(self):
+        msgs = [{"role": "user", "content": "hello"}]
+        params = {"max_tokens": 100, "tools": [{"type": "function"}]}
+        assert _make_cache_key("gpt-4", msgs, params) == _make_cache_key("gpt-4", msgs, dict(params))
+
+    def test_unserializable_param_does_not_raise(self):
+        msgs = [{"role": "user", "content": "hello"}]
+        assert len(_make_cache_key("gpt-4", msgs, {"weird": object()})) == 64
+
+    def test_tool_call_id_changes_key(self):
+        """Tool-result messages with identical text must not collide."""
+        k1 = _make_cache_key("gpt-4", [{"role": "tool", "content": "ok", "tool_call_id": "a"}])
+        k2 = _make_cache_key("gpt-4", [{"role": "tool", "content": "ok", "tool_call_id": "b"}])
+        assert k1 != k2
+
+
+class TestRequestCacheParams:
+    def test_extracts_declared_and_extra_fields(self):
+        class FakeRequest:
+            temperature = 0.5
+            top_p = None
+            max_tokens = 128
+            n = None
+            model_extra = {
+                "tools": [{"type": "function"}],
+                "response_format": {"type": "json_object"},
+                "stream": False,
+            }
+
+        params = request_cache_params(FakeRequest())
+        assert params == {
+            "temperature": 0.5,
+            "max_tokens": 128,
+            "tools": [{"type": "function"}],
+            "response_format": {"type": "json_object"},
+        }
+
+    def test_missing_attributes_are_tolerated(self):
+        class Bare:
+            pass
+
+        assert request_cache_params(Bare()) == {}
+
 
 class TestPromptCache:
     def test_put_and_get(self):
@@ -39,6 +99,16 @@ def test_put_and_get(self):
         result = cache.get("gpt-4", msgs)
         assert result == response
 
+    def test_different_params_do_not_share_entry(self):
+        """A plain chat response must not be served to a structured-output request."""
+        cache = PromptCache(max_size=10, ttl=60)
+        msgs = [{"role": "user", "content": "Return a city name"}]
+        response = {"content": "Paris", "finish_reason": "stop", "prompt_tokens": 5, "completion_tokens": 1}
+
+        cache.put("gpt-4", msgs, response, {})
+        assert cache.get("gpt-4", msgs, {"response_format": {"type": "json_object"}}) is None
+        assert cache.get("gpt-4", msgs, {}) == response
+
     def test_miss_returns_none(self):
         cache = PromptCache(max_size=10, ttl=60)
         result = cache.get("gpt-4", [{"role": "user", "content": "hello"}])