diff --git a/CHANGELOG.md b/CHANGELOG.md index c54c5bf19..5c4906462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [Unreleased] ### Fixed +- **Retry usage accounting**: Accumulate nested and newly added numeric usage fields across OpenAI and Anthropic retries, including prediction, cache-write, cache-creation, and server-tool counters, without treating boolean metadata as billable usage. ([#2493](https://github.com/567-labs/instructor/issues/2493), [#2500](https://github.com/567-labs/instructor/pull/2500)) +- **OpenAI Responses reask**: Add a fallback correction message when a `RESPONSES_TOOLS` response contains no tool calls (e.g. reasoning-only output), so retries carry validation feedback instead of resending the identical request. ([#2498](https://github.com/567-labs/instructor/pull/2498)) +- **v2 parallel tools**: Preserve raw iterable type hints through the sync and async patch wrappers so parallel tool schemas and results retain every requested model type. ([#2501](https://github.com/567-labs/instructor/pull/2501)) - **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)) diff --git a/instructor/v2/core/patch.py b/instructor/v2/core/patch.py index ac436d93e..676c5f9f7 100644 --- a/instructor/v2/core/patch.py +++ b/instructor/v2/core/patch.py @@ -209,13 +209,15 @@ def new_create_sync( # Get handlers from registry handlers = mode_registry.get_handlers(provider, mode) - if response_model is not None: + if response_model is not None and mode not in Mode.parallel_modes(): response_model = prepare_response_model(response_model) # Prepare request kwargs using registry handler - response_model, new_kwargs = handlers.request_handler( + prepared_model, new_kwargs = handlers.request_handler( response_model=response_model, kwargs=kwargs ) + if mode not in Mode.parallel_modes(): + response_model = prepared_model new_kwargs.pop("autodetect_images", None) if handlers.message_converter and "messages" in new_kwargs: new_kwargs["messages"] = handlers.message_converter( @@ -323,13 +325,15 @@ async def new_create_async( # Get handlers from registry handlers = mode_registry.get_handlers(provider, mode) - if response_model is not None: + if response_model is not None and mode not in Mode.parallel_modes(): response_model = prepare_response_model(response_model) # Prepare request kwargs using registry handler - response_model, new_kwargs = handlers.request_handler( + prepared_model, new_kwargs = handlers.request_handler( response_model=response_model, kwargs=kwargs ) + if mode not in Mode.parallel_modes(): + response_model = prepared_model new_kwargs.pop("autodetect_images", None) if handlers.message_converter and "messages" in new_kwargs: new_kwargs["messages"] = handlers.message_converter( diff --git a/instructor/v2/core/usage.py b/instructor/v2/core/usage.py index d220cf462..cc0946a1d 100644 --- a/instructor/v2/core/usage.py +++ b/instructor/v2/core/usage.py @@ -3,7 +3,10 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING, TypeVar +from numbers import Real +from typing import TYPE_CHECKING, TypeVar, cast + +from pydantic import BaseModel if TYPE_CHECKING: from anthropic.types import Usage as AnthropicUsage @@ -13,6 +16,56 @@ T_Response = TypeVar("T_Response") +def _field_names(model: BaseModel) -> set[str]: + """Return declared and Pydantic extra field names for a model.""" + return set(type(model).model_fields) | set(model.model_extra or {}) + + +def _all_field_names(*models: BaseModel) -> set[str]: + return {field_name for model in models for field_name in _field_names(model)} + + +def _is_numeric(value: object) -> bool: + return isinstance(value, Real) and not isinstance(value, bool) + + +def _zero_numeric_fields(model: BaseModel) -> BaseModel: + for field_name in _field_names(model): + value = getattr(model, field_name, None) + if isinstance(value, BaseModel): + _zero_numeric_fields(value) + elif _is_numeric(value): + setattr(model, field_name, 0) + return model + + +def _accumulate_models(response: BaseModel, total: BaseModel) -> None: + for field_name in _all_field_names(response, total): + response_value = getattr(response, field_name, None) + total_value = getattr(total, field_name, None) + if isinstance(response_value, BaseModel): + if not isinstance(total_value, BaseModel): + total_value = _zero_numeric_fields(response_value.model_copy(deep=True)) + setattr(total, field_name, total_value) + _accumulate_models(response_value, total_value) + setattr(response, field_name, total_value.model_copy(deep=True)) + elif isinstance(total_value, BaseModel): + setattr(response, field_name, total_value.model_copy(deep=True)) + elif _is_numeric(response_value): + response_number = cast(Real, response_value) + total_number = cast(Real, total_value) if _is_numeric(total_value) else 0 + value = total_number + response_number + setattr(total, field_name, value) + setattr(response, field_name, value) + elif _is_numeric(total_value): + setattr(response, field_name, total_value) + elif response_value is not None: + setattr(total, field_name, response_value) + setattr(response, field_name, response_value) + elif total_value is not None: + setattr(response, field_name, total_value) + + def update_total_usage( response: T_Response | None, total_usage: OpenAIUsage | AnthropicUsage, @@ -26,34 +79,7 @@ def update_total_usage( if isinstance(response_usage, _OpenAIUsage) and isinstance( total_usage, _OpenAIUsage ): - total_usage.completion_tokens += response_usage.completion_tokens or 0 - total_usage.prompt_tokens += response_usage.prompt_tokens or 0 - total_usage.total_tokens += response_usage.total_tokens or 0 - if (rtd := response_usage.completion_tokens_details) and ( - ttd := total_usage.completion_tokens_details - ): - ttd.audio_tokens = (ttd.audio_tokens or 0) + (rtd.audio_tokens or 0) - ttd.reasoning_tokens = (ttd.reasoning_tokens or 0) + ( - rtd.reasoning_tokens or 0 - ) - if (rpd := response_usage.prompt_tokens_details) and ( - tpd := total_usage.prompt_tokens_details - ): - tpd.audio_tokens = (tpd.audio_tokens or 0) + (rpd.audio_tokens or 0) - tpd.cached_tokens = (tpd.cached_tokens or 0) + (rpd.cached_tokens or 0) - response_usage.completion_tokens = total_usage.completion_tokens - response_usage.prompt_tokens = total_usage.prompt_tokens - response_usage.total_tokens = total_usage.total_tokens - response_usage.completion_tokens_details = ( - total_usage.completion_tokens_details.model_copy(deep=True) - if total_usage.completion_tokens_details is not None - else None - ) - response_usage.prompt_tokens_details = ( - total_usage.prompt_tokens_details.model_copy(deep=True) - if total_usage.prompt_tokens_details is not None - else None - ) + _accumulate_models(response_usage, total_usage) return response try: diff --git a/instructor/v2/providers/anthropic/usage.py b/instructor/v2/providers/anthropic/usage.py index 1bb27edd8..4442e2f32 100644 --- a/instructor/v2/providers/anthropic/usage.py +++ b/instructor/v2/providers/anthropic/usage.py @@ -4,6 +4,8 @@ from typing import Any +from instructor.v2.core.usage import _accumulate_models + def initialize_usage() -> Any: """Create an empty Anthropic usage accumulator.""" @@ -26,18 +28,5 @@ def update_total_usage(response_usage: Any, total_usage: Any) -> bool: ): return False - if not total_usage.cache_creation_input_tokens: - total_usage.cache_creation_input_tokens = 0 - if not total_usage.cache_read_input_tokens: - total_usage.cache_read_input_tokens = 0 - total_usage.input_tokens += response_usage.input_tokens or 0 - total_usage.output_tokens += response_usage.output_tokens or 0 - total_usage.cache_creation_input_tokens += ( - response_usage.cache_creation_input_tokens or 0 - ) - total_usage.cache_read_input_tokens += response_usage.cache_read_input_tokens or 0 - response_usage.input_tokens = total_usage.input_tokens - response_usage.output_tokens = total_usage.output_tokens - response_usage.cache_creation_input_tokens = total_usage.cache_creation_input_tokens - response_usage.cache_read_input_tokens = total_usage.cache_read_input_tokens + _accumulate_models(response_usage, total_usage) return True diff --git a/instructor/v2/providers/openai/handlers.py b/instructor/v2/providers/openai/handlers.py index bea9d8033..8afc5e8dd 100644 --- a/instructor/v2/providers/openai/handlers.py +++ b/instructor/v2/providers/openai/handlers.py @@ -260,6 +260,21 @@ def reask_responses_tools( } ) + if not reask_messages: + # Model produced no tool calls at all (e.g. a reasoning-only or plain + # message output). Fall back to a plain user correction so the retry + # carries feedback instead of resending the identical request, + # mirroring reask_tools and the Anthropic reask handler. + reask_messages.append( + { + "role": "user", + "content": ( + f"Validation Error found:\n{exception}\n" + "Recall the function correctly, fix the errors" + ), + } + ) + kwargs["messages"].extend(reask_messages) return kwargs diff --git a/tests/coverage/test_anthropic_support_coverage.py b/tests/coverage/test_anthropic_support_coverage.py index a96e741e2..d162b5e5d 100644 --- a/tests/coverage/test_anthropic_support_coverage.py +++ b/tests/coverage/test_anthropic_support_coverage.py @@ -14,6 +14,8 @@ import httpx import pytest from anthropic.types import Usage +from anthropic.types.cache_creation import CacheCreation +from anthropic.types.server_tool_usage import ServerToolUsage from pydantic import BaseModel, ValidationInfo, field_validator from instructor.v2.core.client import AsyncInstructor, Instructor @@ -397,3 +399,34 @@ def test_anthropic_usage_initializes_and_accumulates_sdk_usage() -> None: ) == (12, 10, 2, 5) assert update_total_usage(object(), total) is False assert update_total_usage(second, object()) is False + + +def test_anthropic_usage_accumulates_nested_provider_fields() -> None: + total = initialize_usage() + first = Usage( + input_tokens=100, + output_tokens=50, + cache_creation=CacheCreation( + ephemeral_1h_input_tokens=10, ephemeral_5m_input_tokens=20 + ), + server_tool_use=ServerToolUsage(web_fetch_requests=2, web_search_requests=3), + ) + second = Usage( + input_tokens=100, + output_tokens=50, + cache_creation=CacheCreation( + ephemeral_1h_input_tokens=10, ephemeral_5m_input_tokens=20 + ), + server_tool_use=ServerToolUsage(web_fetch_requests=2, web_search_requests=3), + ) + + assert update_total_usage(first, total) is True + assert update_total_usage(second, total) is True + assert total.cache_creation == CacheCreation( + ephemeral_1h_input_tokens=20, ephemeral_5m_input_tokens=40 + ) + assert total.server_tool_use == ServerToolUsage( + web_fetch_requests=4, web_search_requests=6 + ) + assert second.cache_creation == total.cache_creation + assert second.server_tool_use == total.server_tool_use diff --git a/tests/coverage/test_core_patch_retry_coverage.py b/tests/coverage/test_core_patch_retry_coverage.py index 5c0ee5c8b..3740a3e8f 100644 --- a/tests/coverage/test_core_patch_retry_coverage.py +++ b/tests/coverage/test_core_patch_retry_coverage.py @@ -31,7 +31,7 @@ retry_sync, retry_sync_v2, ) -from instructor.v2.core.usage import update_total_usage +from instructor.v2.core.usage import _accumulate_models, update_total_usage from instructor.v2.dsl.iterable import IterableModel from instructor.v2.dsl.response_list import ListResponse from instructor.v2.dsl.simple_type import ModelAdapter @@ -78,18 +78,28 @@ def test_openai_usage_adds_token_details_and_copies_totals_to_response() -> None prompt_tokens=6, total_tokens=10, completion_tokens_details=CompletionTokensDetails( - audio_tokens=1, reasoning_tokens=2 + accepted_prediction_tokens=3, + audio_tokens=1, + reasoning_tokens=2, + rejected_prediction_tokens=4, + ), + prompt_tokens_details=PromptTokensDetails.model_validate( + {"audio_tokens": 3, "cached_tokens": 4, "cache_write_tokens": 5} ), - prompt_tokens_details=PromptTokensDetails(audio_tokens=3, cached_tokens=4), ) response_usage = CompletionUsage( completion_tokens=7, prompt_tokens=11, total_tokens=18, completion_tokens_details=CompletionTokensDetails( - audio_tokens=5, reasoning_tokens=8 + accepted_prediction_tokens=7, + audio_tokens=5, + reasoning_tokens=8, + rejected_prediction_tokens=9, + ), + prompt_tokens_details=PromptTokensDetails.model_validate( + {"audio_tokens": 13, "cached_tokens": 21, "cache_write_tokens": 23} ), - prompt_tokens_details=PromptTokensDetails(audio_tokens=13, cached_tokens=21), ) response = _completion(1) response.usage = response_usage @@ -111,14 +121,20 @@ def test_openai_usage_adds_token_details_and_copies_totals_to_response() -> None assert total_prompt_details is not None assert response_completion_details is not None assert response_prompt_details is not None + assert total_completion_details.accepted_prediction_tokens == 10 assert total_completion_details.audio_tokens == 6 assert total_completion_details.reasoning_tokens == 10 + assert total_completion_details.rejected_prediction_tokens == 13 assert total_prompt_details.audio_tokens == 16 assert total_prompt_details.cached_tokens == 25 + assert total_prompt_details.model_extra == {"cache_write_tokens": 28} + assert response_completion_details.accepted_prediction_tokens == 10 assert response_completion_details.audio_tokens == 6 assert response_completion_details.reasoning_tokens == 10 + assert response_completion_details.rejected_prediction_tokens == 13 assert response_prompt_details.audio_tokens == 16 assert response_prompt_details.cached_tokens == 25 + assert response_prompt_details.model_extra == {"cache_write_tokens": 28} assert response_completion_details is not total_completion_details assert response_prompt_details is not total_prompt_details @@ -128,6 +144,125 @@ def test_openai_usage_adds_token_details_and_copies_totals_to_response() -> None assert response_prompt_details.cached_tokens == 25 +def test_openai_usage_accumulates_sdk_extra_numeric_fields() -> None: + total_usage = CompletionUsage.model_validate( + { + "completion_tokens": 1, + "prompt_tokens": 2, + "total_tokens": 3, + "future_tokens": 4, + "future_cost": 1.5, + } + ) + response_usage = CompletionUsage.model_validate( + { + "completion_tokens": 5, + "prompt_tokens": 6, + "total_tokens": 11, + "future_tokens": 7, + "future_cost": 2.25, + } + ) + response = _completion(1) + response.usage = response_usage + + update_total_usage(response, total_usage) + + assert total_usage.model_extra is not None + assert response_usage.model_extra is not None + assert total_usage.model_extra["future_tokens"] == 11 + assert total_usage.model_extra["future_cost"] == 3.75 + assert response_usage.model_extra["future_tokens"] == 11 + assert response_usage.model_extra["future_cost"] == 3.75 + + +def test_usage_carries_existing_non_numeric_fields_to_response() -> None: + total_usage = CompletionUsage.model_validate( + { + "completion_tokens": 1, + "prompt_tokens": 2, + "total_tokens": 3, + "service_tier": "scale", + } + ) + response_usage = CompletionUsage.model_validate( + { + "completion_tokens": 5, + "prompt_tokens": 6, + "total_tokens": 11, + } + ) + response = _completion(1) + response.usage = response_usage + + update_total_usage(response, total_usage) + + assert response_usage.model_extra is not None + assert response_usage.model_extra["service_tier"] == "scale" + + +def test_usage_response_non_numeric_fields_still_win() -> None: + total_usage = CompletionUsage.model_validate( + { + "completion_tokens": 1, + "prompt_tokens": 2, + "total_tokens": 3, + "service_tier": "scale", + } + ) + response_usage = CompletionUsage.model_validate( + { + "completion_tokens": 5, + "prompt_tokens": 6, + "total_tokens": 11, + "service_tier": "default", + } + ) + response = _completion(1) + response.usage = response_usage + + update_total_usage(response, total_usage) + + assert total_usage.model_extra is not None + assert total_usage.model_extra["service_tier"] == "default" + assert response_usage.model_extra is not None + assert response_usage.model_extra["service_tier"] == "default" + + +def test_usage_accumulates_recursive_models_across_sdk_shape_changes() -> None: + class Counters(BaseModel): + billed: int + label: str | None = None + + class Details(BaseModel): + counters: Counters + region: str + + class Usage(BaseModel): + total: int + details: Details | str | None = None + + total = Usage(total=0, details="legacy-shape") + first = Usage( + total=2, + details=Details(counters=Counters(billed=4, label="new"), region="us"), + ) + + _accumulate_models(first, total) + + assert isinstance(total.details, Details) + assert total.details.counters.billed == 4 + assert total.details.counters.label == "new" + assert total.details.region == "us" + + second = Usage(total=3) + _accumulate_models(second, total) + + assert second.total == 5 + assert second.details == total.details + assert second.details is not total.details + + def test_patch_requires_a_target_and_supports_a_create_callable() -> None: patch_without_target = cast(Callable[[], object], patch) with pytest.raises(ValueError, match="Either client or create must be provided"): diff --git a/tests/test_openai_responses_tools.py b/tests/test_openai_responses_tools.py index 5de9068ca..651e58938 100644 --- a/tests/test_openai_responses_tools.py +++ b/tests/test_openai_responses_tools.py @@ -141,6 +141,38 @@ def test_reask_responses_tools_none_arguments() -> None: assert "MUST populate ALL required fields" in msg +def test_reask_responses_tools_no_tool_calls_adds_fallback_message() -> None: + """Reask must add corrective feedback even when the output has no tool calls. + + Reasoning models can return only reasoning/message items instead of the + forced function call. Without a fallback the retry resends the identical + request with no feedback at all. + """ + reasoning_item = MagicMock() + reasoning_item.type = "reasoning" + + message_item = MagicMock() + message_item.type = "message" + + response = MagicMock() + response.output = [reasoning_item, message_item] + + error = ValueError( + "1 validation error for ResponseToolModel\nname\n Field required" + ) + + result = reask_responses_tools( + {"messages": [{"role": "user", "content": "extract"}]}, response, error + ) + + assert len(result["messages"]) == 2 + fallback = result["messages"][-1] + assert fallback["role"] == "user" + assert "Validation Error found" in fallback["content"] + assert "Field required" in fallback["content"] + assert "Recall the function correctly" in fallback["content"] + + def test_responses_tools_overrides_text_type_format() -> None: _, kwargs = OpenAIResponsesToolsHandler().prepare_request( ResponseToolModel, diff --git a/tests/v2/test_parallel_tools_wrapper.py b/tests/v2/test_parallel_tools_wrapper.py new file mode 100644 index 000000000..b2ae07ddf --- /dev/null +++ b/tests/v2/test_parallel_tools_wrapper.py @@ -0,0 +1,103 @@ +"""Regression tests for PARALLEL_TOOLS through the v2 patch wrapper. + +The existing handler-level tests call ``handlers.request_handler(...)`` directly +and bypass ``patch_v2``; these exercise the wrapper path ``from_openai`` uses. +""" + +from __future__ import annotations + +from collections.abc import Iterable +from typing import Any, Union, get_args, get_origin + +import pytest +from pydantic import BaseModel + +from instructor.v2.core.mode import Mode +from instructor.v2.core.patch import patch_v2 +from instructor.v2.core.providers import Provider +from tests.coverage._openai import chat_completion, tool_call + + +class A(BaseModel): + a: str + + +class B(BaseModel): + b: str + + +def _member_types(response_model: Any) -> tuple[type[BaseModel], ...]: + inner = get_args(response_model)[0] + origin = get_origin(inner) + if origin is Union: + return get_args(inner) + return (inner,) + + +def _parallel_completion(response_model: Any) -> Any: + members = _member_types(response_model) + payload = {"A": {"a": "alpha"}, "B": {"b": "beta"}} + return chat_completion( + tool_calls=[ + tool_call( + m.__name__, payload[m.__name__], call_id=f"call_{m.__name__.lower()}" + ) + for m in members + ], + finish_reason="tool_calls", + ) + + +def _assert_parallel_result(result: Any, response_model: Any) -> None: + members = _member_types(response_model) + expected_names = [m.__name__ for m in members] + items = list(result) + assert [type(x).__name__ for x in items] == expected_names + assert items[0].a == "alpha" + if B in members: + assert items[1].b == "beta" + + +@pytest.mark.parametrize( + "response_model", + [ + pytest.param(Iterable[A], id="Iterable[A]"), + pytest.param(Iterable[Union[A, B]], id="Iterable[Union[A,B]]"), + ], +) +def test_parallel_tools_sync_wrapper(response_model: Any) -> None: + calls: list[dict[str, Any]] = [] + + def create(**kwargs: Any) -> Any: + calls.append(kwargs) + return _parallel_completion(response_model) + + patched = patch_v2(create, Provider.OPENAI, Mode.PARALLEL_TOOLS) + result = patched( + response_model=response_model, + messages=[{"role": "user", "content": "run both"}], + ) + + assert calls, "create was never called" + _assert_parallel_result(result, response_model) + tool_names = {t["function"]["name"] for t in calls[0]["tools"]} + assert tool_names == {m.__name__ for m in _member_types(response_model)} + + +@pytest.mark.asyncio +async def test_parallel_tools_async_wrapper() -> None: + response_model = Iterable[Union[A, B]] + calls: list[dict[str, Any]] = [] + + async def create(**kwargs: Any) -> Any: + calls.append(kwargs) + return _parallel_completion(response_model) + + patched = patch_v2(create, Provider.OPENAI, Mode.PARALLEL_TOOLS) + result = await patched( + response_model=response_model, + messages=[{"role": "user", "content": "run both"}], + ) + + assert calls, "create was never called" + _assert_parallel_result(result, response_model)