diff --git a/CHANGELOG.md b/CHANGELOG.md index dc92a35a0..c54c5bf19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [Unreleased] ### Fixed +- **Credential redaction**: Hide common OAuth and Google API credential aliases in nested v2 debug logging while preserving non-secret token configuration. ([#2490](https://github.com/567-labs/instructor/issues/2490), [#2491](https://github.com/567-labs/instructor/pull/2491)) +- **Retry and message integrity**: Preserve cache keys and caller-owned retry messages, retain empty-content legacy function calls, return Anthropic tool results for every parallel tool call, and handle missing OpenAI/Mistral tool calls as retryable parse failures. ([#2454](https://github.com/567-labs/instructor/issues/2454), [#2455](https://github.com/567-labs/instructor/pull/2455), [#2464](https://github.com/567-labs/instructor/issues/2464), [#2484](https://github.com/567-labs/instructor/pull/2484), [#2485](https://github.com/567-labs/instructor/issues/2485), [#2486](https://github.com/567-labs/instructor/pull/2486), [#2448](https://github.com/567-labs/instructor/pull/2448), [#2453](https://github.com/567-labs/instructor/pull/2453)) +- **Streaming and DSL correctness**: Isolate partial-model recursion guards, preserve partial nested models and explicit nulls, harden citation matching, derive useful Iterable union names, and continue scanning JSON streams after non-JSON or multiple balanced values. ([#2422](https://github.com/567-labs/instructor/issues/2422), [#2430](https://github.com/567-labs/instructor/pull/2430), [#2431](https://github.com/567-labs/instructor/issues/2431), [#2452](https://github.com/567-labs/instructor/pull/2452), [#2456](https://github.com/567-labs/instructor/pull/2456), [#2461](https://github.com/567-labs/instructor/issues/2461), [#2463](https://github.com/567-labs/instructor/pull/2463), [#2476](https://github.com/567-labs/instructor/pull/2476), [#2487](https://github.com/567-labs/instructor/pull/2487), [#2489](https://github.com/567-labs/instructor/pull/2489)) +- **Provider request handling**: Avoid mutating Gemini generation config and cached OpenAI schemas, disable Anthropic parallel calls for forced single-tool requests, forward Bedrock default models, and label OpenAI audio as WAV or MP3 without misrepresenting unsupported formats. ([#2450](https://github.com/567-labs/instructor/issues/2450), [#2451](https://github.com/567-labs/instructor/pull/2451), [#2465](https://github.com/567-labs/instructor/issues/2465), [#2467](https://github.com/567-labs/instructor/pull/2467), [#2477](https://github.com/567-labs/instructor/issues/2477), [#2478](https://github.com/567-labs/instructor/pull/2478), [#2447](https://github.com/567-labs/instructor/pull/2447), [#2415](https://github.com/567-labs/instructor/pull/2415)) +- **Batch, CLI, and citation runtime**: Accept valid empty batch objects, use typed OpenAI file attributes in the CLI, normalize `None` message content, and install `regex` as the direct dependency required by `CitationMixin`. ([#2473](https://github.com/567-labs/instructor/pull/2473), [#2441](https://github.com/567-labs/instructor/pull/2441), [#2440](https://github.com/567-labs/instructor/pull/2440), [#2443](https://github.com/567-labs/instructor/pull/2443)) +- **Provider documentation**: Refresh retired Cerebras model IDs, clarify current and deprecated Google provider prefixes, and fix the Vertex Google GenAI example so its default model is passed to `from_genai()`. ([#2494](https://github.com/567-labs/instructor/pull/2494), [#2289](https://github.com/567-labs/instructor/issues/2289), [#2343](https://github.com/567-labs/instructor/pull/2343), [#2416](https://github.com/567-labs/instructor/issues/2416), [#2475](https://github.com/567-labs/instructor/pull/2475)) - **Multimodal (Audio)**: Raise explicit `ValueError` or `FileNotFoundError` from `Audio.from_url()` and `Audio.from_path()` instead of relying on bare `assert` statements that can disappear under `python -O`. ([#2361](https://github.com/567-labs/instructor/pull/2361)) - **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. diff --git a/docs/blog/posts/introducing-structured-outputs-with-cerebras-inference.md b/docs/blog/posts/introducing-structured-outputs-with-cerebras-inference.md index 1d5d54f73..b06b15b03 100644 --- a/docs/blog/posts/introducing-structured-outputs-with-cerebras-inference.md +++ b/docs/blog/posts/introducing-structured-outputs-with-cerebras-inference.md @@ -65,7 +65,7 @@ Once you've done so, you can use the following code to get started. import instructor from pydantic import BaseModel -client = instructor.from_provider("cerebras/llama3.1-70b") +client = instructor.from_provider("cerebras/gpt-oss-120b") class Person(BaseModel): @@ -74,7 +74,7 @@ class Person(BaseModel): resp = client.create( - model="llama3.1-70b", + model="gpt-oss-120b", messages=[ { "role": "user", @@ -109,7 +109,7 @@ class Person(BaseModel): resp = client.create( - model="llama3.1-70b", + model="gpt-oss-120b", messages=[ { "role": "user", diff --git a/docs/integrations/cerebras.md b/docs/integrations/cerebras.md index b500b9379..8a9ccc03c 100644 --- a/docs/integrations/cerebras.md +++ b/docs/integrations/cerebras.md @@ -22,7 +22,7 @@ import instructor from cerebras.cloud.sdk import Cerebras from pydantic import BaseModel -client = instructor.from_provider("cerebras/llama3.1-70b") +client = instructor.from_provider("cerebras/gpt-oss-120b") class User(BaseModel): name: str @@ -51,7 +51,7 @@ from pydantic import BaseModel import asyncio client = instructor.from_provider( - "cerebras/llama3.1-70b", + "cerebras/gpt-oss-120b", async_client=True, ) @@ -84,7 +84,7 @@ from pydantic import BaseModel import instructor from cerebras.cloud.sdk import Cerebras -client = instructor.from_provider("cerebras/llama3.1-70b") +client = instructor.from_provider("cerebras/gpt-oss-120b") class Address(BaseModel): @@ -149,7 +149,7 @@ from pydantic import BaseModel from typing import Iterable client = instructor.from_provider( - "cerebras/llama3.1-70b", + "cerebras/gpt-oss-120b", mode=instructor.Mode.MD_JSON, ) @@ -187,7 +187,7 @@ from pydantic import BaseModel from typing import Iterable client = instructor.from_provider( - "cerebras/llama3.1-70b", + "cerebras/gpt-oss-120b", mode=instructor.Mode.MD_JSON, ) diff --git a/docs/integrations/google.md b/docs/integrations/google.md index 27abe0fcb..bf20ffd50 100644 --- a/docs/integrations/google.md +++ b/docs/integrations/google.md @@ -18,6 +18,12 @@ Master structured data extraction using Google's Gemini models with Instructor. Google's GenAI SDK is the recommended way to access Gemini models. It provides a unified interface for both the Gemini API and Vertex AI. This guide shows you how to use Instructor with Google's GenAI SDK for type-safe, validated responses. +!!! info "Choosing a provider prefix" + + - `google/` is recommended. It uses the current `google-genai` SDK for the Gemini API and, with `vertexai=True`, Vertex AI. + - `vertexai/` is deprecated. Migrate to `google/` with `vertexai=True`. + - `gemini/` is legacy. It uses the older `google-generativeai` package; migrate to `google/`. + ```bash pip install "instructor[google-genai]" ``` diff --git a/docs/integrations/vertex.md b/docs/integrations/vertex.md index 2813e780e..c63027d7b 100644 --- a/docs/integrations/vertex.md +++ b/docs/integrations/vertex.md @@ -269,8 +269,8 @@ client = from_genai( vertexai=True, project="your-project", location="us-central1", - model="gemini-3-flash" - ) + ), + model="gemini-3-flash", ) ``` diff --git a/instructor/batch/processor.py b/instructor/batch/processor.py index c659bbc3f..b7fc3c6a3 100644 --- a/instructor/batch/processor.py +++ b/instructor/batch/processor.py @@ -189,7 +189,7 @@ def parse_results(self, results_content: str) -> list[BatchResult]: custom_id = data.get("custom_id", "unknown") extracted_data = self._extract_from_response(data) - if extracted_data: + if extracted_data is not None: try: # Parse into response model result = self.response_model(**extracted_data) diff --git a/instructor/cli/files.py b/instructor/cli/files.py index c83d8929d..976c0c836 100644 --- a/instructor/cli/files.py +++ b/instructor/cli/files.py @@ -28,11 +28,11 @@ def generate_file_table(files: list[openai.types.FileObject]) -> Table: for file in files: table.add_row( - file["id"], - str(file["bytes"]), - str(datetime.fromtimestamp(file["created_at"])), - file["filename"], - file["purpose"], + file.id, + str(file.bytes), + str(datetime.fromtimestamp(file.created_at)), + file.filename, + file.purpose, ) return table @@ -61,7 +61,7 @@ def upload( file_purpose = cast(Literal["fine-tune", "assistants"], purpose) with open(filepath, "rb") as file: response = client.files.create(file=file, purpose=file_purpose) - file_id = response["id"] + file_id = response.id with console.status(f"Monitoring upload: {file_id}...") as status: status.spinner_style = "dots" while True: diff --git a/instructor/v2/auto_client.py b/instructor/v2/auto_client.py index 54335e960..f925f56a2 100644 --- a/instructor/v2/auto_client.py +++ b/instructor/v2/auto_client.py @@ -1023,6 +1023,7 @@ def _build_bedrock( client, mode=default_mode, async_client=async_client, + model=model_name, **kwargs, ) logger.info( diff --git a/instructor/v2/core/json.py b/instructor/v2/core/json.py index e9238119c..bb04a0d66 100644 --- a/instructor/v2/core/json.py +++ b/instructor/v2/core/json.py @@ -83,6 +83,8 @@ def extract_json_from_stream(chunks: Iterable[str]) -> Generator[str, None, None delimiter_stack: list[str] = [] buffer: list[str] = [] codeblock_buffer: list[str] = [] + last_invalid_candidate: str | None = None + emitted_valid_candidate = False for chunk in chunks: for char in chunk: @@ -142,9 +144,17 @@ def extract_json_from_stream(chunks: Iterable[str]) -> Generator[str, None, None delimiter_stack.pop() if not delimiter_stack: buffer.append(char) - yield from buffer + candidate = "".join(buffer) buffer = [] json_started = False + try: + json.loads(candidate) + except ValueError: + last_invalid_candidate = candidate + continue + emitted_valid_candidate = True + last_invalid_candidate = None + yield from candidate continue buffer.append(char) @@ -157,6 +167,8 @@ def extract_json_from_stream(chunks: Iterable[str]) -> Generator[str, None, None if json_started and buffer: yield from buffer + elif not emitted_valid_candidate and last_invalid_candidate is not None: + yield from last_invalid_candidate async def extract_json_from_stream_async( @@ -171,6 +183,8 @@ async def extract_json_from_stream_async( delimiter_stack: list[str] = [] buffer: list[str] = [] codeblock_buffer: list[str] = [] + last_invalid_candidate: str | None = None + emitted_valid_candidate = False async for chunk in chunks: for char in chunk: @@ -231,10 +245,18 @@ async def extract_json_from_stream_async( delimiter_stack.pop() if not delimiter_stack: buffer.append(char) - for buffered_char in buffer: - yield buffered_char + candidate = "".join(buffer) buffer = [] json_started = False + try: + json.loads(candidate) + except ValueError: + last_invalid_candidate = candidate + continue + emitted_valid_candidate = True + last_invalid_candidate = None + for buffered_char in candidate: + yield buffered_char continue buffer.append(char) @@ -248,3 +270,6 @@ async def extract_json_from_stream_async( if json_started and buffer: for buffered_char in buffer: yield buffered_char + elif not emitted_valid_candidate and last_invalid_candidate is not None: + for buffered_char in last_invalid_candidate: + yield buffered_char diff --git a/instructor/v2/core/messages.py b/instructor/v2/core/messages.py index 9a24c81ea..37a3a24f1 100644 --- a/instructor/v2/core/messages.py +++ b/instructor/v2/core/messages.py @@ -38,6 +38,16 @@ 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]: + """Copy request lists that reask handlers mutate during retries.""" + 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, @@ -45,11 +55,7 @@ def dump_message(message: ChatCompletionMessage) -> ChatCompletionMessageParam: } if hasattr(message, "tool_calls") and message.tool_calls is not None: ret["tool_calls"] = message.model_dump()["tool_calls"] - if ( - hasattr(message, "function_call") - and message.function_call is not None - and ret["content"] - ): + if hasattr(message, "function_call") and message.function_call is not None: if not isinstance(ret["content"], str): response_message = "" for content_message in ret["content"]: @@ -83,6 +89,8 @@ def merge_consecutive_messages(messages: list[dict[str, Any]]) -> list[dict[str, for message in messages: role = message.get("role", "user") new_content = message.get("content", "") + if new_content is None: + new_content = "" if not flat_string and isinstance(new_content, str): new_content = [{"type": "text", "text": new_content}] 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/instructor/v2/core/registry.py b/instructor/v2/core/registry.py index 66419ebd8..8501bda36 100644 --- a/instructor/v2/core/registry.py +++ b/instructor/v2/core/registry.py @@ -6,6 +6,7 @@ from __future__ import annotations +import threading from dataclasses import dataclass from typing import Callable @@ -80,6 +81,12 @@ def __init__(self) -> None: """Initialize empty registry.""" self._handlers: dict[tuple[Provider, Mode], ModeHandlers] = {} self._lazy_loaders: dict[tuple[Provider, Mode], Callable[[], ModeHandlers]] = {} + # Guards the lazy-load resolution (check -> pop -> import -> set) in + # get_handlers(). Without this, concurrent first-callers for the same + # mode_key race: one pops the loader, the others see neither dict + # populated yet and raise KeyError. Held for the whole resolution + # (not per-key) since lazy-loading only ever runs once per key. + self._lazy_load_lock = threading.Lock() def register( self, @@ -181,12 +188,22 @@ def get_handlers(self, provider: Provider, mode: Mode) -> ModeHandlers: if mode_key in self._handlers: return self._handlers[mode_key] - # Try lazy loading - if mode_key in self._lazy_loaders: - loader = self._lazy_loaders.pop(mode_key) - handlers = loader() - self._handlers[mode_key] = handlers - return handlers + # Try lazy loading. Locked because the pop -> import -> set sequence + # below is not atomic: without the lock, a thread that loses the race + # to pop self._lazy_loaders[mode_key] would find it already gone and + # self._handlers[mode_key] not yet set, and raise KeyError even + # though the mode genuinely is registered (just still resolving). + with self._lazy_load_lock: + # Re-check: another thread may have finished loading this key + # while we were waiting for the lock. + if mode_key in self._handlers: + return self._handlers[mode_key] + + if mode_key in self._lazy_loaders: + loader = self._lazy_loaders.pop(mode_key) + handlers = loader() + self._handlers[mode_key] = handlers + return handlers raise KeyError( f"Mode {mode_key} is not registered. " diff --git a/instructor/v2/core/response.py b/instructor/v2/core/response.py index edc99b385..07f6e06ce 100644 --- a/instructor/v2/core/response.py +++ b/instructor/v2/core/response.py @@ -70,7 +70,17 @@ class User(BaseModel): T = TypeVar("T") _SENSITIVE_KEYS: frozenset[str] = frozenset( - {"api_key", "api_secret", "authorization", "token", "x_api_key"} + { + "access_token", + "api_key", + "api_secret", + "authorization", + "client_secret", + "refresh_token", + "token", + "x_api_key", + "x_goog_api_key", + } ) diff --git a/instructor/v2/dsl/citation.py b/instructor/v2/dsl/citation.py index c5ccd4f04..f0b0bc805 100644 --- a/instructor/v2/dsl/citation.py +++ b/instructor/v2/dsl/citation.py @@ -68,6 +68,8 @@ def validate_sources(self, info: ValidationInfo) -> "CitationMixin": # Get the context from the info text_chunks = info.context.get("context", None) + if text_chunks is None: + return self # Get the spans of the substring_phrase in the context spans = list(self.get_spans(text_chunks)) @@ -80,12 +82,15 @@ def _get_span( ) -> Generator[tuple[int, int], None, None]: import regex - minor = quote + # Escape the quote so regex metacharacters in LLM-generated text + # (e.g. unbalanced parentheses or brackets) are matched literally + # instead of crashing the fuzzy search with a regex compile error. + minor = regex.escape(quote) major = context errs_ = 0 s = regex.search(f"({minor}){{e<={errs_}}}", major) - while s is None and errs_ <= errs: + while s is None and errs_ < errs: errs_ += 1 s = regex.search(f"({minor}){{e<={errs_}}}", major) diff --git a/instructor/v2/dsl/partial.py b/instructor/v2/dsl/partial.py index 464d32ca8..3e5695ae6 100644 --- a/instructor/v2/dsl/partial.py +++ b/instructor/v2/dsl/partial.py @@ -12,6 +12,7 @@ import types import warnings from collections.abc import AsyncGenerator, Callable, Generator, Iterable +from contextvars import ContextVar from copy import deepcopy from functools import cache from functools import reduce @@ -40,8 +41,11 @@ UNION_ORIGINS = (Union, UNION_TYPE) if UNION_TYPE is not None else (Union,) # Track models currently being processed to prevent infinite recursion -# with self-referential models (e.g., TreeNode with children: List["TreeNode"]) -_processing_models: set[type] = set() +# with self-referential models (e.g., TreeNode with children: List["TreeNode"]). +# Each top-level partial-model construction receives an isolated guard. +_processing_models: ContextVar[set[type] | None] = ContextVar( + "processing_models", default=None +) def _unwrap_optional_base_model(annotation: Any) -> type[BaseModel] | None: @@ -241,6 +245,12 @@ def _build_partial_list( result.append(_item_model.model_validate(item, **kwargs)) continue + if _item_model is not None and isinstance(item, dict): + result.append( + _build_partial_object(item, _item_model, tracker, item_path, **kwargs) + ) + continue + result.append(item) return result @@ -250,6 +260,15 @@ def _process_generic_arg( arg: Any, make_fields_optional: bool = False, ) -> Any: + if _processing_models.get() is None: + token = _processing_models.set(set()) + try: + return _process_generic_arg(arg, make_fields_optional=make_fields_optional) + finally: + _processing_models.reset(token) + + processing_models = _processing_models.get() + assert processing_models is not None arg_origin = get_origin(arg) if arg_origin is not None: @@ -269,9 +288,9 @@ def _process_generic_arg( return arg_origin[modified_nested_args] if isinstance(arg, type) and issubclass(arg, BaseModel): # Prevent infinite recursion for self-referential models - if arg in _processing_models: + if arg in processing_models: return arg # Already processing this model, return unwrapped - _processing_models.add(arg) + processing_models.add(arg) try: return ( _make_partial_type(arg, make_fields_optional=True) @@ -279,7 +298,7 @@ def _process_generic_arg( else Partial[arg] ) finally: - _processing_models.discard(arg) + processing_models.discard(arg) else: return arg @@ -439,13 +458,18 @@ def model_from_chunks( yield obj # Final validation: only validate if the JSON is structurally complete - # If JSON is incomplete (stream ended mid-object), skip validation + # If JSON is incomplete (stream ended mid-object), skip validation. + # Validate the accumulated JSON itself rather than a + # model_dump(exclude_none=True) round-trip, which would strip fields + # the model legitimately returned as null and make required-but-nullable + # fields fail re-validation as "missing". if final_obj is not None: original_model = getattr(cls, "_original_model", None) if original_model is not None: - if is_json_complete(potential_object.strip() or "{}"): + json_str = potential_object.strip() or "{}" + if is_json_complete(json_str): original_model.model_validate( - final_obj.model_dump(exclude_none=True), **kwargs + from_json(json_str.encode()), **kwargs ) @classmethod @@ -474,13 +498,18 @@ async def model_from_chunks_async( yield obj # Final validation: only validate if the JSON is structurally complete - # If JSON is incomplete (stream ended mid-object), skip validation + # If JSON is incomplete (stream ended mid-object), skip validation. + # Validate the accumulated JSON itself rather than a + # model_dump(exclude_none=True) round-trip, which would strip fields + # the model legitimately returned as null and make required-but-nullable + # fields fail re-validation as "missing". if final_obj is not None: original_model = getattr(cls, "_original_model", None) if original_model is not None: - if is_json_complete(potential_object.strip() or "{}"): + json_str = potential_object.strip() or "{}" + if is_json_complete(json_str): original_model.model_validate( - final_obj.model_dump(exclude_none=True), **kwargs + from_json(json_str.encode()), **kwargs ) @staticmethod @@ -607,6 +636,15 @@ def __class_getitem__( to support partially defined fields. """ + if _processing_models.get() is None: + token = _processing_models.set(set()) + try: + return cls.__class_getitem__(wrapped_class) + finally: + _processing_models.reset(token) + + processing_models = _processing_models.get() + assert processing_models is not None make_fields_optional = None if isinstance(wrapped_class, tuple): @@ -636,16 +674,16 @@ def _wrap_models(field: FieldInfo) -> tuple[object, FieldInfo]: # attributes to optionals. elif isinstance(annotation, type) and issubclass(annotation, BaseModel): # Prevent infinite recursion for self-referential models - if annotation in _processing_models: + if annotation in processing_models: tmp_field.annotation = ( annotation # Already processing, keep unwrapped ) else: - _processing_models.add(annotation) + processing_models.add(annotation) try: tmp_field.annotation = Partial[annotation] finally: - _processing_models.discard(annotation) + processing_models.discard(annotation) return tmp_field.annotation, tmp_field model_name = ( diff --git a/instructor/v2/providers/anthropic/handlers.py b/instructor/v2/providers/anthropic/handlers.py index bae39821a..798c4ff33 100644 --- a/instructor/v2/providers/anthropic/handlers.py +++ b/instructor/v2/providers/anthropic/handlers.py @@ -402,6 +402,11 @@ def prepare_request( new_kwargs["tool_choice"] = { "type": "tool", "name": getattr(response_model, "__name__", "response"), + # Without this, Anthropic may still emit multiple tool_use + # blocks for the same forced tool, which the single-model + # parser then rejects even though the model's response was + # otherwise valid. + "disable_parallel_tool_use": True, } return response_model, new_kwargs @@ -426,7 +431,10 @@ def handle_reask( return kwargs assistant_content = [] - tool_use_id = None + # Anthropic requires a tool_result for *every* tool_use in the prior turn. + # Collect all ids — overwriting a single id drops results for PARALLEL_TOOLS + # and makes the reask request fail with 400 (issue #2485). + tool_use_ids: list[str] = [] for content in response.content: try: dumped_content = content.model_dump(exclude_none=True) # type: ignore[attr-defined] @@ -434,10 +442,14 @@ def handle_reask( dumped_content = content.model_dump() # type: ignore[attr-defined] assistant_content.append(dumped_content) if content.type == "tool_use": - tool_use_id = content.id + tool_use_ids.append(content.id) reask_msgs = [{"role": "assistant", "content": assistant_content}] - if tool_use_id is not None: + if tool_use_ids: + error_content = ( + "Validation Error found:\n" + f"{exception}\nRecall the function correctly, fix the errors" + ) reask_msgs.append( { "role": "user", @@ -445,12 +457,10 @@ def handle_reask( { "type": "tool_result", "tool_use_id": tool_use_id, - "content": ( - "Validation Error found:\n" - f"{exception}\nRecall the function correctly, fix the errors" - ), + "content": error_content, "is_error": True, } + for tool_use_id in tool_use_ids ], } ) diff --git a/instructor/v2/providers/gemini/utils.py b/instructor/v2/providers/gemini/utils.py index 3a3df240a..aaca56b77 100644 --- a/instructor/v2/providers/gemini/utils.py +++ b/instructor/v2/providers/gemini/utils.py @@ -322,7 +322,8 @@ def update_gemini_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: result = kwargs.copy() if "generation_config" in result: - gen_config = result["generation_config"] + gen_config = result["generation_config"].copy() + result["generation_config"] = gen_config for openai_key, gemini_key in _OPENAI_TO_GEMINI_MAP.items(): if openai_key in gen_config: diff --git a/instructor/v2/providers/mistral/handlers.py b/instructor/v2/providers/mistral/handlers.py index 3360f8c33..3e2f084ce 100644 --- a/instructor/v2/providers/mistral/handlers.py +++ b/instructor/v2/providers/mistral/handlers.py @@ -28,7 +28,10 @@ from instructor.v2.core.mode import Mode from instructor.v2.core.providers import Provider -from instructor.v2.core.errors import IncompleteOutputException +from instructor.v2.core.errors import ( + IncompleteOutputException, + ResponseParsingError, +) from instructor.v2.dsl.iterable import IterableBase from instructor.v2.dsl.parallel import ParallelBase, get_types_array from instructor.v2.dsl.partial import PartialBase @@ -224,8 +227,19 @@ def _extract_tool_call_json(self, response: Any) -> str: Mistral returns tool call arguments as either a string or a dict, so we need to handle both cases. """ - tool_call = response.choices[0].message.tool_calls[0] - args = tool_call.function.arguments + message = response.choices[0].message + tool_calls = getattr(message, "tool_calls", None) or [] + if not tool_calls: + # The model replied in prose instead of calling the tool (e.g. + # models that don't honor tool_choice="any", refusals, or + # OpenAI-compatible gateways). Raise a retryable parsing error so + # the retry machinery re-asks instead of crashing with TypeError. + raise ResponseParsingError( + "No tool calls found in Mistral response", + mode=self.mode.name, + raw_response=response, + ) + args = tool_calls[0].function.arguments if isinstance(args, dict): return json.dumps(args) return args @@ -294,9 +308,25 @@ def handle_reask( ) -> dict[str, Any]: """Handle reask for tools mode.""" kwargs = kwargs.copy() - reask_msgs: list[Any] = [dump_message(response.choices[0].message)] + message = response.choices[0].message + reask_msgs: list[Any] = [dump_message(message)] + + tool_calls = message.tool_calls or [] + if not tool_calls: + # The model answered in prose without calling the tool, so there is + # no tool_call_id to attach a tool-role message to. Fall back to a + # plain user correction instead of iterating None. + reask_msgs.append( + { + "role": "user", + "content": ( + f"Validation Error found:\n{exception}\n" + "Recall the function correctly, fix the errors" + ), + } + ) - for tool_call in response.choices[0].message.tool_calls: + for tool_call in tool_calls: reask_msgs.append( { "role": "tool", @@ -347,7 +377,7 @@ def parse_response( type_registry = {t.__name__: t for t in the_types} def parallel_generator() -> Generator[BaseModel, None, None]: - for tool_call in response.choices[0].message.tool_calls: + for tool_call in response.choices[0].message.tool_calls or []: name = tool_call.function.name if name in type_registry: model_class = type_registry[name] diff --git a/instructor/v2/providers/openai/handlers.py b/instructor/v2/providers/openai/handlers.py index 364b90948..bea9d8033 100644 --- a/instructor/v2/providers/openai/handlers.py +++ b/instructor/v2/providers/openai/handlers.py @@ -174,8 +174,24 @@ def reask_tools( ) return kwargs - reask_msgs: list[Any] = [dump_message(response.choices[0].message)] - for tool_call in response.choices[0].message.tool_calls: + message = response.choices[0].message + reask_msgs: list[Any] = [dump_message(message)] + tool_calls = message.tool_calls or [] + if not tool_calls: + # Model replied without calling the tool (e.g. OpenAI-compatible + # providers that don't honor forced tool_choice). Fall back to a plain + # user correction instead of iterating None, mirroring the streaming + # branch and the GenAI reask handler. + reask_msgs.append( + { + "role": "user", + "content": ( + f"Validation Error found:\n{exception}\n" + "Recall the function correctly, fix the errors" + ), + } + ) + for tool_call in tool_calls: reask_msgs.append( { "role": "tool", @@ -648,7 +664,11 @@ def prepare_request( new_kwargs["tools"] = handle_parallel_model(cast(Any, response_model)) new_kwargs["tool_choice"] = "auto" else: - schema = generate_openai_schema(response_model) + # Shallow-copy to avoid mutating the lru_cache return value. + # generate_openai_schema caches the same dict object; writing + # "strict" into it would permanently affect every future call for + # this model, even when strict=False. + schema = dict(generate_openai_schema(response_model)) # Check for strict parameter use_strict = new_kwargs.pop("strict", False) diff --git a/instructor/v2/providers/openai/multimodal.py b/instructor/v2/providers/openai/multimodal.py index c18c2bf56..dd7b60cf5 100644 --- a/instructor/v2/providers/openai/multimodal.py +++ b/instructor/v2/providers/openai/multimodal.py @@ -39,7 +39,30 @@ def image_to_openai(image: Any, mode: Mode) -> dict[str, Any]: def audio_to_openai(audio: Any, mode: Mode) -> dict[str, Any]: if mode in RESPONSES_MODES: raise ValueError("OpenAI Responses doesn't support audio") - return {"type": "input_audio", "input_audio": {"data": audio.data, "format": "wav"}} + + # Chat Completions accepts only WAV or MP3. Reject other formats instead of + # sending mislabeled bytes that the API cannot decode. + media_type = (getattr(audio, "media_type", "") or "").lower() + format_by_media_type = { + "audio/mp3": "mp3", + "audio/mpeg": "mp3", + "audio/mpga": "mp3", + "audio/wav": "wav", + "audio/x-wav": "wav", + } + if media_type not in format_by_media_type: + raise ValueError( + f"Unsupported OpenAI audio format: {media_type or 'unknown'}. " + "Expected WAV or MP3." + ) + + return { + "type": "input_audio", + "input_audio": { + "data": audio.data, + "format": format_by_media_type[media_type], + }, + } def pdf_to_openai(pdf: Any, mode: Mode) -> dict[str, Any]: diff --git a/pyproject.toml b/pyproject.toml index eb847cf18..fa75c6842 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,7 @@ dependencies = [ "jiter>=0.6.1,<0.15", "jinja2<4.0.0,>=3.1.4", "requests<3.0.0,>=2.32.3", + "regex>=2023.0.0,<2027.0.0", ] name = "instructor" version = "1.15.5" diff --git a/requirements.txt b/requirements.txt index 29dcbcd4f..54979d0cc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -77,6 +77,8 @@ pydantic-core==2.41.5 # pydantic pygments==2.20.0 # via rich +regex==2026.7.19 + # via instructor (pyproject.toml) requests==2.32.5 # via instructor (pyproject.toml) rich==14.3.3 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" + ) diff --git a/tests/cli/test_files.py b/tests/cli/test_files.py new file mode 100644 index 000000000..978617693 --- /dev/null +++ b/tests/cli/test_files.py @@ -0,0 +1,26 @@ +"""Tests for instructor.cli.files.""" + +from openai.types import FileObject +import pytest + + +def test_generate_file_table_uses_attribute_access( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("OPENAI_API_KEY", "test-key-for-cli-import") + from instructor.cli.files import generate_file_table + + file = FileObject( + id="file-abc123", + bytes=1024, + created_at=1700000000, + filename="training.jsonl", + object="file", + purpose="fine-tune", + status="processed", + ) + + table = generate_file_table([file]) + + rendered_first_column = table.columns[0]._cells + assert list(rendered_first_column) == ["file-abc123"] diff --git a/tests/core/test_schema_utils.py b/tests/core/test_schema_utils.py index 994e6f851..71825c474 100644 --- a/tests/core/test_schema_utils.py +++ b/tests/core/test_schema_utils.py @@ -215,5 +215,24 @@ def test_anthropic_schema_uses_openai_base(): assert anthropic_schema["input_schema"] == TestModel.model_json_schema() +def test_cached_schema_same_object(): + """generate_openai_schema returns the same dict object on every call (lru_cache). + + This means callers MUST NOT mutate the returned dict directly; they should + copy it first. OpenAIToolsHandler.prepare_request does this by shallow-copying + before writing "strict" into the schema. + """ + + class ImmutableModel(BaseModel): + value: int + + s1 = generate_provider_openai_schema(ImmutableModel) + s2 = generate_provider_openai_schema(ImmutableModel) + + # Both calls return the exact same object — any direct mutation would + # affect all future callers, which is why handlers must copy before mutating. + assert s1 is s2 + + if __name__ == "__main__": pytest.main([__file__]) diff --git a/tests/coverage/test_anthropic_handlers_coverage.py b/tests/coverage/test_anthropic_handlers_coverage.py index 0bb034582..dd51240d6 100644 --- a/tests/coverage/test_anthropic_handlers_coverage.py +++ b/tests/coverage/test_anthropic_handlers_coverage.py @@ -408,7 +408,11 @@ def test_tools_prepare_request_serializes_messages_selects_tools_and_respects_ch } ] assert request["tools"][0]["name"] == "User" - assert request["tool_choice"] == {"type": "tool", "name": "User"} + assert request["tool_choice"] == { + "type": "tool", + "name": "User", + "disable_parallel_tool_use": True, + } no_model, passthrough = handler.prepare_request( None, @@ -580,7 +584,14 @@ def test_parallel_tools_prepare_reask_and_parse_real_tool_blocks() -> None: assert list(handler.parse_response(None, parallel_type)) == [] assert list(handler.parse_response(object(), parallel_type)) == [] reask = handler.handle_reask({"messages": []}, response, ValueError("bad job")) - assert reask["messages"][-1]["content"][0]["tool_use_id"] == "toolu_three" + # Anthropic requires a tool_result for *every* tool_use block (#2485). + reask_results = reask["messages"][-1]["content"] + assert [block["tool_use_id"] for block in reask_results] == [ + "toolu_one", + "toolu_two", + "toolu_three", + ] + assert all(block.get("is_error") is True for block in reask_results) def test_json_prepare_reask_and_parse_anthropic_and_openai_shaped_responses() -> None: diff --git a/tests/coverage/test_auto_client_coverage.py b/tests/coverage/test_auto_client_coverage.py index 5ad620ec0..86fe9598b 100644 --- a/tests/coverage/test_auto_client_coverage.py +++ b/tests/coverage/test_auto_client_coverage.py @@ -373,7 +373,12 @@ def create_instructor(client: object, **kwargs: Any) -> dict[str, Any]: "aws_session_token": "explicit-token", "region_name": "eu-west-1", } - assert result == {"mode": Mode.JSON, "async_client": True, "max_tokens": 19} + assert result == { + "mode": Mode.JSON, + "async_client": True, + "model": "amazon.titan", + "max_tokens": 19, + } def test_bedrock_reads_all_environment_credentials( diff --git a/tests/coverage/test_core_helpers_coverage.py b/tests/coverage/test_core_helpers_coverage.py index fd99020ac..40559e8e1 100644 --- a/tests/coverage/test_core_helpers_coverage.py +++ b/tests/coverage/test_core_helpers_coverage.py @@ -115,6 +115,18 @@ def test_dump_message_appends_function_call_to_text_content() -> None: ) +def test_dump_message_retains_function_call_when_content_is_empty() -> None: + message = ChatCompletionMessage( + role="assistant", + content=None, + function_call=FunctionCall(name="lookup", arguments='{"id":7}'), + ) + + result = dump_message(message) + + assert result["content"] == json.dumps({"arguments": '{"id":7}', "name": "lookup"}) + + def test_merge_consecutive_messages_checks_tail_for_non_string_content() -> None: messages: list[dict[str, Any]] = [ {"role": "user", "content": f"line {index}"} for index in range(10) diff --git a/tests/coverage/test_core_response_coverage.py b/tests/coverage/test_core_response_coverage.py index 05e48a2ad..d0a64f262 100644 --- a/tests/coverage/test_core_response_coverage.py +++ b/tests/coverage/test_core_response_coverage.py @@ -3,6 +3,7 @@ import importlib import json from collections.abc import Callable +from copy import deepcopy from types import SimpleNamespace from typing import Any, cast @@ -107,6 +108,45 @@ def test_redaction_handles_tuples_without_mutating_input() -> None: assert kwargs["metadata"][0]["api-secret"] == "private" +@pytest.mark.parametrize( + "credential_key", + [ + "access_token", + "refresh-token", + "X-Goog-Api-Key", + "Client-Secret", + "api_key", + "Authorization", + ], +) +def test_redaction_hides_common_credential_aliases( + credential_key: str, +) -> None: + kwargs = { + "extra_headers": { + credential_key: "private", + "x-request-id": "request-123", + } + } + original = deepcopy(kwargs) + + result = _redact_kwargs(kwargs) + + assert result["extra_headers"][credential_key] == "[redacted]" + assert result["extra_headers"]["x-request-id"] == "request-123" + assert kwargs == original + + +def test_redaction_preserves_non_secret_token_configuration() -> None: + kwargs = { + "max_tokens": 256, + "token_budget": 1_000, + "headers": {"content-type": "application/json"}, + } + + assert _redact_kwargs(kwargs) == kwargs + + def test_registry_load_failure_is_best_effort( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/coverage/test_dsl_partial_coverage.py b/tests/coverage/test_dsl_partial_coverage.py index 98ed7c553..03175638f 100644 --- a/tests/coverage/test_dsl_partial_coverage.py +++ b/tests/coverage/test_dsl_partial_coverage.py @@ -1,5 +1,7 @@ from collections.abc import AsyncGenerator, Callable, Generator, Iterable +from concurrent.futures import ThreadPoolExecutor from enum import Enum +from threading import Event, Lock from typing import Any, Optional, Protocol, Union, cast import typing @@ -127,7 +129,14 @@ def test_partial_builder_validates_complete_nested_values_and_keeps_open_values( assert result.metadata == {"source": "stream", "attempt": 2} assert result.featured == Item(number=7, state=State.READY) assert result.items[0] == Item(number=8, state=State.PENDING) - assert result.items[1] == {"number": "9", "state": "rea"} + # Open (incomplete) list items are recursed into partial model instances, + # consistent with how a singular open nested field is handled (see + # test_partial_builder_recurses_into_open_nested_model_and_handles_scalars): + # unvalidated/partial values are kept, but the item stays a typed instance + # instead of degrading to a raw dict. + assert isinstance(result.items[1], Item) + assert result.items[1].number == "9" + assert result.items[1].state == "rea" def test_partial_builder_recurses_into_open_nested_model_and_handles_scalars() -> None: @@ -444,3 +453,68 @@ class RecursiveNode(BaseModel): assert schema["$defs"]["RecursiveNode"]["properties"]["child"] == { "$ref": "#/$defs/RecursiveNode" } + + +def test_partial_processing_isolated_between_threads( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class Address(BaseModel): + street: str + + class Home(BaseModel): + addresses: list[Address] + + class Office(BaseModel): + addresses: list[Address] + + first_entered = Event() + second_entered = Event() + release_first = Event() + call_lock = Lock() + calls = 0 + partial_module_any = cast(Any, partial_module) + original_make_partial_type = cast( + Callable[..., type[BaseModel]], + partial_module_any._make_partial_type, + ) + + def block_first_address_conversion( + annotation: type[BaseModel], *, make_fields_optional: bool = False + ) -> type[BaseModel]: + nonlocal calls + if annotation is Address: + with call_lock: + calls += 1 + call_number = calls + + if call_number == 1: + first_entered.set() + assert release_first.wait(timeout=1) + elif call_number == 2: + second_entered.set() + + return original_make_partial_type( + annotation, make_fields_optional=make_fields_optional + ) + + monkeypatch.setattr( + partial_module, "_make_partial_type", block_first_address_conversion + ) + make_partial_type = cast( + Callable[..., type[BaseModel]], + partial_module_any._make_partial_type, + ) + + with ThreadPoolExecutor(max_workers=2) as executor: + first = executor.submit(make_partial_type, Home, make_fields_optional=True) + assert first_entered.wait(timeout=1) + + second = executor.submit(make_partial_type, Office, make_fields_optional=True) + assert second_entered.wait(timeout=1) + release_first.set() + + home_partial = first.result() + office_partial = second.result() + + assert "PartialAddress" in home_partial.model_json_schema()["$defs"] + assert "PartialAddress" in office_partial.model_json_schema()["$defs"] diff --git a/tests/coverage/test_dsl_small_coverage.py b/tests/coverage/test_dsl_small_coverage.py index 99564fff6..35da3d889 100644 --- a/tests/coverage/test_dsl_small_coverage.py +++ b/tests/coverage/test_dsl_small_coverage.py @@ -46,6 +46,17 @@ def test_citation_keeps_quotes_without_context_and_recovers_fuzzy_quotes() -> No assert list(cited.get_spans(context)) == [(21, 42)] +def test_citation_keeps_quotes_when_validation_context_lacks_context_key() -> None: + quotes = ["Jaxon is 20 years old"] + + cited = Claim.model_validate( + {"fact": "age", "substring_quotes": quotes}, + context={"request_id": "req_123"}, + ) + + assert cited.substring_quotes == quotes + + @pytest.mark.parametrize( ("value", "complete"), [("", False), (" \n\t", False), ('{"ok": true}', True), ('{"ok":', False)], diff --git a/tests/coverage/test_mistral_coverage.py b/tests/coverage/test_mistral_coverage.py index 266fb01b3..439f4428e 100644 --- a/tests/coverage/test_mistral_coverage.py +++ b/tests/coverage/test_mistral_coverage.py @@ -24,7 +24,7 @@ from pydantic import BaseModel import instructor.v2.providers.mistral.client as mistral_client -from instructor.v2.core.errors import ClientError, ModeError +from instructor.v2.core.errors import ClientError, ModeError, ResponseParsingError from instructor.v2.core.mode import Mode from instructor.v2.core.multimodal import PDF from instructor.v2.core.providers import Provider @@ -405,6 +405,38 @@ def test_parallel_tool_request_and_response_support_multiple_models() -> None: assert parsed == [User(name="Ada", age=36), Answer(answer=42.0)] +def test_tools_handler_survives_prose_response_without_tool_calls() -> None: + # Models that ignore tool_choice="any" (or refuse) return a plain assistant + # message with tool_calls=None. Parsing must raise a retryable + # ResponseParsingError instead of TypeError, reask must fall back to a user + # correction instead of iterating None, and the parallel generator must + # yield nothing instead of crashing. + handler = MistralToolsHandler() + prose = response(content="I cannot call the tool for that.") + + with pytest.raises(ResponseParsingError, match="No tool calls found"): + handler.parse_response(prose, User) + + reask_kwargs = handler.handle_reask( + {"messages": [{"role": "user", "content": "Extract the user"}]}, + prose, + ValueError("age must be an int"), + ) + assert len(reask_kwargs["messages"]) == 3 + assert reask_kwargs["messages"][1]["role"] == "assistant" + correction = reask_kwargs["messages"][-1] + assert correction["role"] == "user" + assert "age must be an int" in correction["content"] + assert "Recall the function correctly" in correction["content"] + + parallel_model, _ = handler.prepare_request( + cast(type[BaseModel], Iterable[Union[User, Answer]]), + {"messages": [{"role": "user", "content": "Extract all results"}]}, + ) + assert parallel_model is not None + assert list(handler.parse_response(prose, parallel_model)) == [] + + def test_tools_streaming_iterable_parser_uses_task_list_chunks() -> None: handler = MistralToolsHandler() model, request = handler.prepare_request( diff --git a/tests/coverage/test_openai_handlers_coverage.py b/tests/coverage/test_openai_handlers_coverage.py index 1c311d90b..08ea3e6d1 100644 --- a/tests/coverage/test_openai_handlers_coverage.py +++ b/tests/coverage/test_openai_handlers_coverage.py @@ -36,6 +36,7 @@ OpenAIToolsHandler, reask_default, reask_responses_tools, + reask_tools, ) from tests.coverage._openai import chat_chunk, chat_completion, tool_call from tests.coverage._streams import async_items @@ -149,6 +150,26 @@ def test_tools_reask_preserves_assistant_calls_and_adds_one_tool_error_per_call( ] +def test_tools_reask_without_tool_call_adds_a_user_correction() -> None: + # OpenAI-compatible providers that don't honor forced tool_choice (or a + # model that answers in prose) can return a plain assistant message whose + # tool_calls is None. reask_tools must fall back to a user correction rather + # than iterating None and raising TypeError. + response = chat_completion(content="The answer is many.") + + result = reask_tools( + {"messages": [{"role": "user", "content": "How many?"}]}, + response, + ValueError("answer must be an int"), + ) + + assert result["messages"][1]["role"] == "assistant" + correction = result["messages"][-1] + assert correction["role"] == "user" + assert "answer must be an int" in correction["content"] + assert "Recall the function correctly" in correction["content"] + + def test_responses_reask_uses_legacy_arguments_without_inventing_call_details() -> None: response = SimpleNamespace(output=[SimpleNamespace(arguments='{"name": 3}')]) diff --git a/tests/dsl/test_partial.py b/tests/dsl/test_partial.py index 2d242bd0e..cd344ade4 100644 --- a/tests/dsl/test_partial.py +++ b/tests/dsl/test_partial.py @@ -960,6 +960,71 @@ async def async_chunks(): assert "age" in str(exc_info.value) + def test_final_validation_accepts_explicit_null_for_required_nullable_field(self): + """Explicit JSON null for a required-but-nullable field should validate. + + The final validation must not confuse a field the model explicitly + returned as null with a field that was never streamed at all. + """ + + class ModelWithNullable(BaseModel): + name: str + email: Optional[str] # Required, but nullable + + PartialModel = Partial[ModelWithNullable] + + chunks = ['{"name": "Al', 'ice", "email"', ": null}"] + + results = list(_partial_api(PartialModel).model_from_chunks(iter(chunks))) + assert len(results) > 0 + final = results[-1] + assert final.name == "Alice" + assert final.email is None + + def test_final_validation_still_rejects_absent_required_nullable_field(self): + """A required nullable field genuinely absent from complete JSON still fails.""" + + class ModelWithNullable(BaseModel): + name: str + email: Optional[str] # Required, but nullable + + PartialModel = Partial[ModelWithNullable] + + chunks = ['{"name": "Alice"}'] # 'email' truly missing + + with pytest.raises(ValidationError) as exc_info: + list(_partial_api(PartialModel).model_from_chunks(iter(chunks))) + + assert "email" in str(exc_info.value) + + @pytest.mark.asyncio + async def test_async_final_validation_accepts_explicit_null_for_required_nullable_field( + self, + ): + """Async streaming should also accept explicit nulls for nullable fields.""" + + class ModelWithNullable(BaseModel): + name: str + email: Optional[str] # Required, but nullable + + PartialModel = Partial[ModelWithNullable] + + async def async_chunks(): + yield '{"name": "Al' + yield 'ice", "email"' + yield ": null}" + + results = [] + async for obj in _partial_api(PartialModel).model_from_chunks_async( + async_chunks() + ): + results.append(obj) + + assert len(results) > 0 + final = results[-1] + assert final.name == "Alice" + assert final.email is None + class TestRecursiveModels: """Test that Partial handles self-referential models without infinite recursion.""" @@ -1280,3 +1345,28 @@ class Outer(BaseModel): # Must remain None — not an empty Inner() instance assert obj.inner is None + + def test_partial_list_returns_model_instance_for_incomplete_trailing_item(self): + """The trailing (still-streaming) item in a list[BaseModel] field must be + a partial model instance, not a raw dict -- consistent with how a + singular open nested BaseModel field is already handled.""" + + class Item(BaseModel): + name: str + qty: int + + class Order(BaseModel): + items: list[Item] + + partial = Partial[Order] + + # First item complete, second item mid-stream (incomplete). + chunks = ['{"items": [{"name": "apple", "qty": 3}, {"name": "banana", "qty"'] + + results = list(_partial_api(partial).model_from_chunks(chunks)) + obj = results[-1] + + assert isinstance(obj.items[0], Item) + # Would be a raw dict before the fix, raising AttributeError on .name + assert isinstance(obj.items[-1], Item) + assert obj.items[-1].name == "banana" diff --git a/tests/processing/test_message_processing.py b/tests/processing/test_message_processing.py index cdc990aa9..6d3fe24b5 100644 --- a/tests/processing/test_message_processing.py +++ b/tests/processing/test_message_processing.py @@ -88,6 +88,27 @@ def test_multiple_consecutive(self): assert result[2]["role"] == "user" assert "I need help" in result[2]["content"] + def test_consecutive_none_content(self): + """Consecutive same-role messages with content=None (e.g. tool-call-only + assistant turns) must not crash the merge.""" + messages = [ + {"role": "user", "content": "hi"}, + { + "role": "assistant", + "content": None, + "tool_calls": [{"id": "call_1"}], + }, + { + "role": "assistant", + "content": None, + "tool_calls": [{"id": "call_2"}], + }, + ] + result = merge_consecutive_messages(messages) + assert len(result) == 2 + assert result[0]["role"] == "user" + assert result[1]["role"] == "assistant" + class TestGetMessageContent: """Test the get_message_content function.""" diff --git a/tests/test_batch_processor_coverage.py b/tests/test_batch_processor_coverage.py index 03b3bc603..7bc603169 100644 --- a/tests/test_batch_processor_coverage.py +++ b/tests/test_batch_processor_coverage.py @@ -23,6 +23,10 @@ class Person(BaseModel): age: int +class DefaultedResult(BaseModel): + status: str = "ok" + + class RecordingProvider: def __init__(self, results: str = "") -> None: self.results = results @@ -275,6 +279,39 @@ def test_openai_results_distinguish_success_validation_extraction_and_json_error assert results[3].raw_data == {"raw_line": "not-json"} +@pytest.mark.parametrize( + ("model", "content"), + [ + ("openai/gpt-4.1-mini", openai_result("empty", "{}")), + ( + "anthropic/claude-sonnet", + json.dumps( + { + "custom_id": "empty", + "result": { + "type": "succeeded", + "message": {"content": [{"type": "tool_use", "input": {}}]}, + }, + } + ), + ), + ], +) +def test_parse_results_accepts_empty_object_for_defaulted_model( + provider: RecordingProvider, + model: str, + content: str, +) -> None: + del provider + processor = BatchProcessor(model, DefaultedResult) + + results = processor.parse_results(content) + + assert results == [ + BatchSuccess(custom_id="empty", result=DefaultedResult(status="ok")) + ] + + def test_anthropic_results_support_tool_use_and_text_fallback( provider: RecordingProvider, ) -> None: diff --git a/tests/v2/test_anthropic_handlers.py b/tests/v2/test_anthropic_handlers.py new file mode 100644 index 000000000..158dbb390 --- /dev/null +++ b/tests/v2/test_anthropic_handlers.py @@ -0,0 +1,72 @@ +"""Unit tests for Anthropic v2 handlers. + +These tests verify handler behavior without requiring API keys, by calling +the request-preparation logic directly and inspecting the resulting kwargs. +""" + +from __future__ import annotations + +from collections.abc import Iterable + +import pytest +from pydantic import BaseModel + +from instructor import Mode, Provider +from instructor.v2.core.registry import mode_registry + + +class Answer(BaseModel): + """Simple answer model for testing.""" + + answer: float + + +@pytest.fixture +def handler(): + """Get the Anthropic tools handler from registry.""" + return mode_registry.get_handlers(Provider.ANTHROPIC, Mode.TOOLS) + + +class TestAnthropicToolsHandlerToolChoice: + """Regression tests for #2477: a forced single-tool tool_choice did not + set disable_parallel_tool_use, so Anthropic could still emit multiple + tool_use blocks for the same forced tool, which the single-model parser + then rejected even though the model's response was otherwise valid.""" + + def test_single_response_model_disables_parallel_tool_use(self, handler): + kwargs = { + "messages": [{"role": "user", "content": "What is 2+2?"}], + "max_tokens": 100, + } + + _, result_kwargs = handler.request_handler(Answer, kwargs) + + assert result_kwargs["tool_choice"] == { + "type": "tool", + "name": "Answer", + "disable_parallel_tool_use": True, + } + + def test_parallel_tools_are_not_forced_to_disable_parallel_use(self, handler): + # Iterable[T] is the parallel-tools case: the model is *expected* to + # emit multiple tool_use blocks here, so tool_choice must stay "auto" + # and must not gain disable_parallel_tool_use. + kwargs = { + "messages": [{"role": "user", "content": "List some answers"}], + "max_tokens": 100, + } + + _, result_kwargs = handler.request_handler(Iterable[Answer], kwargs) + + assert result_kwargs["tool_choice"] == {"type": "auto"} + + def test_explicit_tool_choice_is_not_overridden(self, handler): + kwargs = { + "messages": [{"role": "user", "content": "What is 2+2?"}], + "max_tokens": 100, + "tool_choice": {"type": "auto"}, + } + + _, result_kwargs = handler.request_handler(Answer, kwargs) + + assert result_kwargs["tool_choice"] == {"type": "auto"} diff --git a/tests/v2/test_citation.py b/tests/v2/test_citation.py new file mode 100644 index 000000000..19bfa870e --- /dev/null +++ b/tests/v2/test_citation.py @@ -0,0 +1,102 @@ +"""Tests for CitationMixin span resolution. + +Covers the regex-escaping behavior in ``_get_span``: LLM-generated quotes may +contain regex metacharacters, which must be matched literally rather than +compiled as a pattern. +""" + +from __future__ import annotations + +import pytest + +from instructor.v2.dsl.citation import CitationMixin + + +class Answer(CitationMixin): + pass + + +def test_quote_with_regex_metacharacters_does_not_crash() -> None: + """A quote with unbalanced parentheses must resolve, not raise regex.error.""" + context = "The margin is 50% (approx) this quarter." + + answer = Answer.model_validate( + {"substring_quotes": ["50% (approx"]}, + context={"context": context}, + ) + + # The span is found and normalized back to the exact context substring. + assert answer.substring_quotes == ["50% (approx"] + + +@pytest.mark.parametrize( + "quote", + [ + "cost [USD]", # brackets + "a+b*c", # quantifiers + "path\\to\\file", # backslashes + "who? (maybe)", # optional + parens + ], +) +def test_various_metacharacter_quotes_resolve(quote: str) -> None: + context = f"prefix {quote} suffix" + + answer = Answer.model_validate( + {"substring_quotes": [quote]}, + context={"context": context}, + ) + + assert answer.substring_quotes == [quote] + + +def test_non_matching_quote_is_dropped() -> None: + """Quotes absent from the context are removed rather than kept.""" + context = "Nothing relevant here." + + answer = Answer.model_validate( + {"substring_quotes": ["(totally unrelated)"]}, + context={"context": context}, + ) + + assert answer.substring_quotes == [] + + +def test_fuzzy_matching_still_works_after_escaping() -> None: + """Escaping the literal quote must not disable fuzzy (edit-distance) matching.""" + context = "The margin is 50% (approx) this quarter." + + # One-character typo ("aprox") within the default edit budget. + answer = Answer.model_validate( + {"substring_quotes": ["50% (aprox)"]}, + context={"context": context}, + ) + + assert answer.substring_quotes == ["50% (approx)"] + + +def test_no_context_leaves_quotes_untouched() -> None: + answer = Answer.model_validate({"substring_quotes": ["anything (raw"]}) + + assert answer.substring_quotes == ["anything (raw"] + + +def test_quote_within_error_tolerance_matches() -> None: + context = "0123456789" + + answer = Answer.model_validate( + {"substring_quotes": ["0123456ZZZ"]}, + context={"context": context}, + ) + + assert answer.substring_quotes == [context] + + +def test_quote_beyond_error_tolerance_is_dropped() -> None: + context = "0123456789" + + answer = Answer.model_validate( + {"substring_quotes": ["0123ZZZZZZ"]}, + context={"context": context}, + ) + + assert answer.substring_quotes == [] diff --git a/tests/v2/test_core_multimodal_runtime.py b/tests/v2/test_core_multimodal_runtime.py index 0ac0e63a2..332b029bb 100644 --- a/tests/v2/test_core_multimodal_runtime.py +++ b/tests/v2/test_core_multimodal_runtime.py @@ -256,3 +256,17 @@ def test_audio_from_path_raises_file_not_found_for_missing_file(tmp_path: Path) with pytest.raises(FileNotFoundError): Audio.from_path(missing_path) + + +def test_audio_to_openai_format_follows_media_type() -> None: + from instructor.v2.providers.openai.multimodal import audio_to_openai + + mp3 = Audio(source="clip.mp3", media_type="audio/mpeg", data="ZmFrZQ==") + assert audio_to_openai(mp3, Mode.TOOLS)["input_audio"]["format"] == "mp3" + + wav = Audio(source="clip.wav", media_type="audio/wav", data="ZmFrZQ==") + assert audio_to_openai(wav, Mode.TOOLS)["input_audio"]["format"] == "wav" + + aac = Audio(source="clip.aac", media_type="audio/aac", data="ZmFrZQ==") + with pytest.raises(ValueError, match="Expected WAV or MP3"): + audio_to_openai(aac, Mode.TOOLS) diff --git a/tests/v2/test_gemini_utils_deterministic.py b/tests/v2/test_gemini_utils_deterministic.py index c47a2bbb7..7060e9e0b 100644 --- a/tests/v2/test_gemini_utils_deterministic.py +++ b/tests/v2/test_gemini_utils_deterministic.py @@ -308,6 +308,18 @@ def test_handle_gemini_json_adds_schema_prompt(monkeypatch: pytest.MonkeyPatch) assert kwargs["generation_config"]["response_mime_type"] == "application/json" +def test_update_gemini_kwargs_does_not_mutate_generation_config( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(utils, "_default_safety_thresholds", lambda: None) + generation_config = {"max_tokens": 5} + + result = utils.update_gemini_kwargs({"generation_config": generation_config}) + + assert generation_config == {"max_tokens": 5} + assert result["generation_config"] == {"max_output_tokens": 5} + + def test_handle_gemini_json_guards_empty_or_missing_messages( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/v2/test_json_helpers.py b/tests/v2/test_json_helpers.py index 8683d8254..65ef8c004 100644 --- a/tests/v2/test_json_helpers.py +++ b/tests/v2/test_json_helpers.py @@ -121,6 +121,24 @@ def test_extract_json_from_stream_preserves_backticks_in_fenced_string() -> None assert "".join(extract_json_from_stream(chunks)) == '{"code":"`inline`"}' +def test_extract_json_from_stream_discards_non_json_brace_span_before_payload() -> None: + # MD_JSON prompts often produce a prose preamble before the actual payload, + # and that preamble can itself contain a balanced pair of braces that is not + # JSON (e.g. a parenthetical aside). A naive bracket-matching scan treats that + # span as "the JSON" once it balances and emits it verbatim, so the real + # payload that follows gets appended onto invalid JSON instead of replacing + # it. The extractor should recognize the span isn't valid JSON, drop it, and + # keep scanning for the real payload. + chunks = [ + "I'll pull out the fields now. ", + "{Note: keeping the original casing} ", + "Here is the result: ", + '{"name":"Ada","age":30}', + ] + + assert "".join(extract_json_from_stream(chunks)) == '{"name":"Ada","age":30}' + + @pytest.mark.asyncio async def test_extract_json_from_stream_async_preserves_backticks_in_string() -> None: async def chunks(): @@ -176,3 +194,22 @@ async def chunks(): "".join([chunk async for chunk in extract_json_from_stream_async(chunks())]) == '{"path":"C:\\\\","name":"Ada"}' ) + + +@pytest.mark.asyncio +async def test_extract_json_from_stream_async_discards_non_json_brace_span_before_payload() -> ( + None +): + async def chunks(): + for chunk in [ + "I'll pull out the fields now. ", + "{Note: keeping the original casing} ", + "Here is the result: ", + '{"name":"Ada","age":30}', + ]: + yield chunk + + assert ( + "".join([chunk async for chunk in extract_json_from_stream_async(chunks())]) + == '{"name":"Ada","age":30}' + ) diff --git a/tests/v2/test_mistral_handlers.py b/tests/v2/test_mistral_handlers.py index 1a33f9be1..02cb8a465 100644 --- a/tests/v2/test_mistral_handlers.py +++ b/tests/v2/test_mistral_handlers.py @@ -195,6 +195,52 @@ def test_handle_reask_adds_messages(self, handler): assert tool_msg["role"] == "tool" assert "Validation Error" in tool_msg["content"] + def test_parse_response_without_tool_calls_raises_retryable_error(self, handler): + """Test parsing a prose response with no tool calls raises ResponseParsingError. + + Some models ignore tool_choice="any" and reply with plain text, leaving + tool_calls as None. Parsing must raise a retryable ResponseParsingError + (not a bare TypeError) so the retry machinery can re-ask. + """ + from instructor.v2.core.errors import ResponseParsingError + + response = MockResponse(content="I cannot answer that as a tool call.") + + with pytest.raises(ResponseParsingError, match="No tool calls found"): + handler.response_parser(response, Answer) + + def test_handle_reask_without_tool_calls_adds_user_correction(self, handler): + """Test handle_reask falls back to a user message when tool_calls is None. + + There is no tool_call_id to attach a tool-role message to, so the + handler must append a plain user correction instead of iterating None. + """ + kwargs = {"messages": [{"role": "user", "content": "Original"}]} + response = MockResponse(content="The answer is four.") + exception = ValueError("Validation failed") + + result = handler.reask_handler(kwargs, response, exception) + + assert len(result["messages"]) == 3 + correction = result["messages"][-1] + assert correction["role"] == "user" + assert "Validation Error" in correction["content"] + assert "Validation failed" in correction["content"] + + def test_parallel_parse_response_without_tool_calls_yields_nothing(self, handler): + """Test the parallel generator tolerates tool_calls=None without crashing.""" + from collections.abc import Iterable + from typing import Union + + class Other(BaseModel): + value: int + + response = MockResponse(content="No tools were called.") + + result = handler.response_parser(response, Iterable[Union[Answer, Other]]) + + assert list(result) == [] + def test_tools_handler_preserves_extra_kwargs(self, handler): """Test TOOLS handler preserves extra kwargs.""" kwargs = { diff --git a/tests/v2/test_openai_compat_handlers.py b/tests/v2/test_openai_compat_handlers.py index 3ae621b90..b20f9dafd 100644 --- a/tests/v2/test_openai_compat_handlers.py +++ b/tests/v2/test_openai_compat_handlers.py @@ -139,3 +139,34 @@ def test_legacy_modes_remain_accepted(provider: Provider) -> None: for legacy_mode in spec.legacy_modes: assert normalize_mode(provider, legacy_mode) != legacy_mode assert mode_registry.is_registered(provider, legacy_mode) + + +def test_strict_mode_does_not_pollute_schema_cache() -> None: + """strict=True must not permanently corrupt the lru_cache for subsequent calls. + + generate_openai_schema uses lru_cache, so it returns the same dict object on + every call. If the TOOLS handler writes "strict" directly into that dict, all + future calls for the same model (even without strict=True) receive a schema + that already carries "strict": True. The fix is a shallow copy before mutation. + """ + + class StrictCacheModel(BaseModel): + x: int + + handler = _handlers(Provider.OPENAI, Mode.TOOLS) + + # First call with strict=True. + _, result_strict = handler.request_handler( + StrictCacheModel, + {"messages": [{"role": "user", "content": "test"}], "strict": True}, + ) + assert result_strict["tools"][0]["function"].get("strict") is True + + # Second call WITHOUT strict=True must not inherit the mutation. + _, result_plain = handler.request_handler( + StrictCacheModel, + {"messages": [{"role": "user", "content": "test"}]}, + ) + assert "strict" not in result_plain["tools"][0]["function"], ( + "lru_cache was poisoned: 'strict' key survived a non-strict call" + ) diff --git a/tests/v2/test_registry.py b/tests/v2/test_registry.py index 5ffc2142b..2c865933f 100644 --- a/tests/v2/test_registry.py +++ b/tests/v2/test_registry.py @@ -1,5 +1,7 @@ """Tests for v2 mode registry.""" +from typing import Any, cast + import pytest from instructor import Mode @@ -116,3 +118,71 @@ def test_registry_invalid_handler_type(provider: Provider, mode: Mode): """Test error for invalid handler type.""" with pytest.raises(ValueError, match="Invalid handler_type"): mode_registry.get_handler(provider, mode, "invalid_type") + + +def test_get_handlers_concurrent_first_access_does_not_race(): + """Regression test for #2422. + + Concurrent first callers for the same lazily-registered (provider, mode) + key must all get the same handlers back, never a KeyError, even when the + loader is slow (simulating a module import in flight). + + The loader sleeps briefly so every other thread has a chance to reach + get_handlers() while the first thread's load is still in progress, this + is the exact window in which the unlocked version raced: losing threads + would see the lazy loader already popped and self._handlers not yet set, + and raise KeyError for a mode that genuinely is registered. + """ + import threading + import time + + from instructor.v2.core.registry import ModeHandlers, ModeRegistry + + registry = ModeRegistry() + n_threads = 8 + started = threading.Barrier(n_threads, timeout=5) + load_count = 0 + load_count_lock = threading.Lock() + + def slow_loader() -> ModeHandlers: + nonlocal load_count + with load_count_lock: + load_count += 1 + # Simulate a slow module import: gives every other thread time to + # reach get_handlers() and start contending before this resolves. + time.sleep(0.2) + return ModeHandlers( + request_handler=cast(Any, lambda *_a, **_k: None), + reask_handler=cast(Any, lambda *_a, **_k: None), + response_parser=cast(Any, lambda *_a, **_k: None), + ) + + registry.register_lazy(Provider.DEEPSEEK, Mode.TOOLS, slow_loader) + + results: list[object] = [None] * n_threads + errors: list[BaseException] = [] + errors_lock = threading.Lock() + + def worker(idx: int) -> None: + # Line up all threads so they call get_handlers() as close to + # simultaneously as possible, maximizing contention. + started.wait(timeout=5) + try: + results[idx] = registry.get_handlers(Provider.DEEPSEEK, Mode.TOOLS) + except BaseException as exc: # noqa: BLE001 + with errors_lock: + errors.append(exc) + + threads = [threading.Thread(target=worker, args=(i,)) for i in range(n_threads)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=10) + + assert errors == [], f"get_handlers raised for concurrent callers: {errors}" + assert all(r is not None for r in results) + assert len({id(r) for r in results}) == 1, ( + "all callers should get the same ModeHandlers instance" + ) + # The loader must run exactly once, not once per racing thread. + assert load_count == 1 diff --git a/uv.lock b/uv.lock index c48c6cbcf..5eff0bc24 100644 --- a/uv.lock +++ b/uv.lock @@ -1771,6 +1771,7 @@ dependencies = [ { name = "openai" }, { name = "pydantic" }, { name = "pydantic-core" }, + { name = "regex" }, { name = "requests" }, { name = "rich" }, { name = "tenacity" }, @@ -1934,6 +1935,7 @@ requires-dist = [ { name = "pytest-xdist", marker = "extra == 'dev'", specifier = ">=3.8.0" }, { name = "python-dotenv", marker = "extra == 'dev'", specifier = ">=1.0.1" }, { name = "redis", marker = "extra == 'test-docs'", specifier = ">=5.0.1,<8.0.0" }, + { name = "regex", specifier = ">=2023.0.0,<2027.0.0" }, { name = "requests", specifier = ">=2.32.3,<3.0.0" }, { name = "rich", specifier = ">=13.7.0,<15.0.0" }, { name = "sqlmodel", marker = "extra == 'sqlmodel'", specifier = ">=0.0.22,<1.0.0" },