From da65335385063c812a3f6e522738cf689b959a98 Mon Sep 17 00:00:00 2001 From: ErenAta16 Date: Fri, 17 Jul 2026 23:49:57 +0300 Subject: [PATCH 1/3] Isolate retry-loop kwargs so cache store key matches lookup key patch.py computes a cache lookup key from new_kwargs["messages"] before retry_sync_v2/retry_async_v2 run, and recomputes the store key from the same new_kwargs["messages"] after they return, but passes new_kwargs into the retry call by reference. Reask handlers (reask_tools, reask_md_json, reask_default, reask_responses_tools, Anthropic's handle_reask) each do kwargs = kwargs.copy() then kwargs["messages"].append(...)/.extend(...): the shallow dict copy doesn't copy the messages list, so those handlers mutate the same list object patch.py still holds a reference to. Any request that needed at least one retry ends up with a store key computed from the post-retry, reask-polluted messages list, different from the lookup key computed from the pristine list, so the result gets cached under a key nothing will ever look up again. Add isolate_retry_kwargs() in messages.py, mirroring the existing copy_messages_for_mutation helper (added for #2417/#2428), and use it at both call sites where new_kwargs is handed to the retry layer, so mutations during the retry loop can never leak back into the kwargs dict used for the cache store key regardless of what an individual reask handler does. Adds a regression test, test_auto_cache_prevents_duplicate_calls_after_a_retry in tests/cache/test_cache_integration.py, next to the existing test_auto_cache_prevents_duplicate_provider_calls. Verified red (fails on the unfixed code) and green (passes on the fixed code). Fixes #2454. --- instructor/v2/core/messages.py | 17 ++++++++ instructor/v2/core/patch.py | 13 ++++-- tests/cache/test_cache_integration.py | 60 ++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/instructor/v2/core/messages.py b/instructor/v2/core/messages.py index 9a24c81ea..7822b48b9 100644 --- a/instructor/v2/core/messages.py +++ b/instructor/v2/core/messages.py @@ -38,6 +38,23 @@ 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. + """ + for key_name in ("messages", "contents", "chat_history"): + value = kwargs.get(key_name) + if isinstance(value, list): + return {**kwargs, key_name: list(value)} + return kwargs + + def dump_message(message: ChatCompletionMessage) -> ChatCompletionMessageParam: ret: ChatCompletionMessageParam = { "role": message.role, diff --git a/instructor/v2/core/patch.py b/instructor/v2/core/patch.py index 26aacc6ba..ac436d93e 100644 --- a/instructor/v2/core/patch.py +++ b/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, ) diff --git a/tests/cache/test_cache_integration.py b/tests/cache/test_cache_integration.py index afb8687a8..158d2329b 100644 --- a/tests/cache/test_cache_integration.py +++ b/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" + ) From 7af106850f365a5ffbc00cf75eb417aedc704c5f Mon Sep 17 00:00:00 2001 From: ErenAta16 Date: Sat, 18 Jul 2026 17:55:43 +0300 Subject: [PATCH 2/3] Isolate every candidate retry-loop key, not just the first match isolate_retry_kwargs stopped after copying the first of messages/contents/chat_history it found, so a kwargs dict carrying more than one would leave the others aliased to the caller's list. No current provider does this, but nothing in the kwargs shape rules it out either. Isolate each candidate independently instead. --- instructor/v2/core/messages.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/instructor/v2/core/messages.py b/instructor/v2/core/messages.py index 7822b48b9..b016f330f 100644 --- a/instructor/v2/core/messages.py +++ b/instructor/v2/core/messages.py @@ -47,12 +47,18 @@ def isolate_retry_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: 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 = kwargs.get(key_name) + value = isolated.get(key_name) if isinstance(value, list): - return {**kwargs, key_name: list(value)} - return kwargs + isolated[key_name] = list(value) + return isolated def dump_message(message: ChatCompletionMessage) -> ChatCompletionMessageParam: From ec6e6405efb0038f4afef87ce26697b5e5a8b226 Mon Sep 17 00:00:00 2001 From: ErenAta16 Date: Sun, 19 Jul 2026 15:31:31 +0300 Subject: [PATCH 3/3] Add CHANGELOG entry for the cache-key retry-divergence fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fcbd6f1b..31c6f9c31 100644 --- a/CHANGELOG.md +++ b/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.