fix(cache): isolate retry-loop kwargs so cache store key matches lookup key

CHANGELOG.md

@@ -10,6 +10,7 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
 ## [Unreleased]
 
 ### Fixed
+- **v2 caching**: Isolate retry-bound `kwargs` so reask-handler mutations to `messages` cannot leak back into the caller's `new_kwargs` and cause the cache lookup/store keys to diverge after any retry. Without this fix, `cache=` silently stops caching any request that needed at least one reask. ([#2454](https://github.com/567-labs/instructor/issues/2454))
 - **v2 message handling**: Preserve caller-owned message lists and nested content across request preparation and retries for OpenAI-compatible, Cohere, Mistral, OpenRouter, Writer, and xAI handlers. ([#2417](https://github.com/567-labs/instructor/issues/2417), [#2428](https://github.com/567-labs/instructor/issues/2428))
 - **v2 JSON extraction**: Prefer the final complete top-level JSON value in text responses and retain every JSON object when multiple objects arrive in one streaming chunk.
 - **v2 schemas**: Treat fields with Pydantic `default_factory` values as optional in generated OpenAI tool schemas.

instructor/v2/core/messages.py

@@ -38,6 +38,29 @@ def copy_messages_for_mutation(messages: list[dict[str, Any]]) -> list[dict[str,
     return copied
 
 
+def isolate_retry_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]:
+    """Return kwargs with a shallow copy of the messages/contents/chat_history list.
+
+    Reask handlers append/extend that list in place during the retry loop (see
+    `copy_messages_for_mutation` above for the equivalent problem one layer up, in
+    `prepare_request`). Passing the caller-facing kwargs dict straight into the retry
+    loop means those mutations land on the exact same list object the caller passed
+    in, which corrupts any code that reads it again afterward, e.g. computing a cache
+    key from it both before and after the retry loop runs.
+
+    Each of the three candidate keys is isolated independently rather than stopping
+    at the first match, so a kwargs dict carrying more than one of them (not used by
+    any current provider, but not precluded by the shape of kwargs either) doesn't
+    leave the others aliased to the caller's list.
+    """
+    isolated = dict(kwargs)
+    for key_name in ("messages", "contents", "chat_history"):
+        value = isolated.get(key_name)
+        if isinstance(value, list):
+            isolated[key_name] = list(value)
+    return isolated
+
+
 def dump_message(message: ChatCompletionMessage) -> ChatCompletionMessageParam:
     ret: ChatCompletionMessageParam = {
         "role": message.role,

instructor/v2/core/patch.py

@@ -20,6 +20,7 @@
 from instructor.v2.core.utils import is_async
 from instructor.v2.core.exceptions import RegistryValidationMixin
 from instructor.v2.core.registry import mode_registry
+from instructor.v2.core.messages import isolate_retry_kwargs
 from instructor.v2.core.response_model import prepare_response_model
 from instructor.v2.core.retry import retry_async_v2, retry_sync_v2
 
@@ -247,7 +248,9 @@ def new_create_sync(
                 if cached is not None:
                     return cached  # type: ignore[return-value]
 
-        # Use v2 retry logic with registry handlers
+        # Use v2 retry logic with registry handlers. Pass an isolated copy of the
+        # messages list so reask-handler mutations during the retry loop can't leak
+        # back into new_kwargs, which is read again below for the cache store key.
         response = retry_sync_v2(
             func=func,
             response_model=response_model,
@@ -256,7 +259,7 @@ def new_create_sync(
             context=context,
             max_retries=max_retries,
             args=args,
-            kwargs=new_kwargs,
+            kwargs=isolate_retry_kwargs(new_kwargs),
             strict=strict,
             hooks=hooks,
         )
@@ -359,7 +362,9 @@ async def new_create_async(
                 if cached is not None:
                     return cached  # type: ignore[return-value]
 
-        # Use v2 retry logic with registry handlers
+        # Use v2 retry logic with registry handlers. Pass an isolated copy of the
+        # messages list so reask-handler mutations during the retry loop can't leak
+        # back into new_kwargs, which is read again below for the cache store key.
         response = await retry_async_v2(
             func=func,
             response_model=response_model,
@@ -368,7 +373,7 @@ async def new_create_async(
             context=context,
             max_retries=max_retries,
             args=args,
-            kwargs=new_kwargs,
+            kwargs=isolate_retry_kwargs(new_kwargs),
             strict=strict,
             hooks=hooks,
         )

tests/cache/test_cache_integration.py

@@ -3,7 +3,7 @@
 import instructor
 from instructor.cache import AutoCache
 from openai.types.chat import ChatCompletionMessageParam
-from pydantic import BaseModel, Field  # type: ignore[import-not-found]
+from pydantic import BaseModel, Field, field_validator  # type: ignore[import-not-found]
 
 
 def test_auto_cache_prevents_duplicate_provider_calls(monkeypatch):
@@ -43,3 +43,61 @@ def fake_completion(*_args, **_kwargs):  # noqa: D401, ANN001
     # Second call with identical inputs – should hit cache, no new provider call
     _ = client.create(messages=list(messages), response_model=User, cache=cache)
     assert call_counter["n"] == 1, "Cache miss – provider was called again"
+
+
+def test_auto_cache_prevents_duplicate_calls_after_a_retry(monkeypatch):
+    _ = monkeypatch
+    """Regression test: a call that needed a retry must still be cacheable.
+
+    Reask handlers append/extend the request's messages list in place. If that
+    list is the same object patch.py reads again to compute the cache store key,
+    the store key ends up different from the lookup key computed before the
+    retry, so a later, identical call never hits the cache.
+    """
+
+    class Answer(BaseModel):
+        value: int
+
+        @field_validator("value")
+        @classmethod
+        def must_be_positive(cls, v: int) -> int:
+            if v < 0:
+                raise ValueError("value must be positive")
+            return v
+
+    call_counter = {"n": 0}
+
+    def fake_completion(*_args, **_kwargs):
+        call_counter["n"] += 1
+        # First call ever returns an invalid value, forcing exactly one retry.
+        value = -5 if call_counter["n"] == 1 else 42
+        content = Answer.model_construct(value=value).model_dump_json()
+        return types.SimpleNamespace(
+            choices=[
+                types.SimpleNamespace(
+                    message=types.SimpleNamespace(content=content),
+                    finish_reason="stop",
+                )
+            ],
+            usage={},
+        )
+
+    cache = AutoCache(maxsize=10)
+    client = instructor.from_litellm(fake_completion, mode=instructor.Mode.JSON)
+    messages: list[ChatCompletionMessageParam] = [
+        {"role": "user", "content": "what is 6 times 7?"}
+    ]
+
+    result1 = client.create(
+        messages=list(messages), response_model=Answer, max_retries=2, cache=cache
+    )
+    assert result1.value == 42
+    assert call_counter["n"] == 2, "First call should need exactly one retry"
+
+    result2 = client.create(
+        messages=list(messages), response_model=Answer, max_retries=2, cache=cache
+    )
+    assert result2.value == 42
+    assert call_counter["n"] == 2, (
+        "Second, identical call should hit the cache instead of calling the provider again"
+    )