diff --git a/altk/core/llm/output_parser.py b/altk/core/llm/output_parser.py index 307319b..9de2af6 100644 --- a/altk/core/llm/output_parser.py +++ b/altk/core/llm/output_parser.py @@ -25,6 +25,35 @@ T = TypeVar("T") +#: Widest integer range still worth enumerating on the wire. A rating scale +#: fits comfortably; an unbounded id or a byte count does not, and spelling one +#: out would bloat the schema for no gain. +_MAX_INT_ENUM_SPAN = 24 + + +def _int_range_as_enum( + type_def: Union[str, List[str], None], prop_schema: Dict[str, Any] +) -> Optional[List[int]]: + """Return ``[minimum..maximum]`` when an integer property is narrowly bounded. + + Strict providers reject ``minimum``/``maximum`` on an ``integer`` ("For + 'integer' type, properties maximum, minimum are not supported") but accept + ``enum``, so enumerating a short range is how the bound survives onto the + wire and keeps being enforced by the provider instead of by a retry. + """ + types = type_def if isinstance(type_def, list) else [type_def] + if "integer" not in types: + return None + low, high = prop_schema.get("minimum"), prop_schema.get("maximum") + if not (isinstance(low, int) and isinstance(high, int)): + return None + # ``bool`` is an ``int`` subclass in Python; a true/false bound is not a range. + if isinstance(low, bool) or isinstance(high, bool): + return None + if not 0 <= high - low < _MAX_INT_ENUM_SPAN: + return None + return list(range(low, high + 1)) + def json_schema_to_pydantic_model( schema: Dict[str, Any], @@ -103,9 +132,15 @@ def _map_array_for_prop(prop_schema: Dict[str, Any]) -> Type: item_type = parse_type(items.get("type"), items) return List[item_type] # type: ignore[valid-type] - # ``enum`` becomes a real ``Literal`` type so the choices survive even - # inside ``items``, where a Field-level constraint could not reach. + # A small bounded integer range is expressed as an ``enum`` instead of + # ``minimum``/``maximum``. Strict providers reject those two keywords on + # an integer but accept ``enum``, so this is the only way to keep the + # range enforced *by the provider* — which is what stops an + # out-of-range score from costing a retry. Bounded scores (SPARC's 1-5 + # metrics) are exactly this shape. enum_values = prop_schema.get("enum") + if enum_values is None: + enum_values = _int_range_as_enum(type_def, prop_schema) if enum_values and all( isinstance(v, (str, int, bool)) or v is None for v in enum_values ): @@ -130,12 +165,15 @@ def _map_array_for_prop(prop_schema: Dict[str, Any]) -> Type: # JSON Schema keyword -> Pydantic ``Field`` argument. Carrying these over # keeps provider-native structured output faithful to the source schema - # (a dropped ``minimum``/``enum`` shows up later as a validation failure). + # (a dropped ``enum``/``maxLength`` shows up later as a validation failure). + # + # Numeric bounds (``minimum``/``maximum``/``exclusive*``) are deliberately + # NOT carried over. This model is only ever the wire ``response_format`` + # value, while local validation runs ``jsonschema`` against the *original* + # schema dict (see ``_validate``) — so the bounds buy no strictness here, + # and Bedrock hard-rejects them ("For 'integer' type, properties maximum, + # minimum are not supported") while strict OpenAI ignores them. _CONSTRAINT_ARGS = { - "minimum": "ge", - "maximum": "le", - "exclusiveMinimum": "gt", - "exclusiveMaximum": "lt", "minLength": "min_length", "maxLength": "max_length", "minItems": "min_length", @@ -156,8 +194,11 @@ def _map_array_for_prop(prop_schema: Dict[str, Any]) -> Type: model = create_model(model_name, **fields) # type: ignore # Mirror ``additionalProperties: false`` — providers with strict structured # output need it, and without it the model may invent extra keys that the - # original schema then rejects. - if schema.get("additionalProperties") is False: + # original schema then rejects. Forbid is the *default* rather than + # opt-in: nested sub-schemas rarely repeat the keyword, and a rendered + # object schema with no ``additionalProperties`` at all is what strict + # providers reject. Only an explicit ``true`` opts back out. + if schema.get("additionalProperties") is not True: model.model_config["extra"] = "forbid" return model @@ -173,14 +214,26 @@ def relax_freeform_object_schema(schema: Dict[str, Any]) -> Dict[str, Any]: helper widens those fields so the same schema accepts both object-literal and stringified forms. Schemas where the object has sub-``properties`` are left alone. + + The walk is recursive: ``json_schema_to_pydantic_model`` stringifies + free-form objects at *any* depth (nested ``properties``, ``items``, + ``$defs``), so widening only the top level would leave those replies + failing validation. """ import copy + def _widen(node: Any) -> None: + if isinstance(node, dict): + if node.get("type") == "object" and "properties" not in node: + node["type"] = ["object", "string"] + for value in node.values(): + _widen(value) + elif isinstance(node, list): + for item in node: + _widen(item) + relaxed = copy.deepcopy(schema) - for _prop, prop_schema in relaxed.get("properties", {}).items(): - t = prop_schema.get("type") - if t == "object" and "properties" not in prop_schema: - prop_schema["type"] = ["object", "string"] + _widen(relaxed) return relaxed @@ -188,6 +241,52 @@ class OutputValidationError(Exception): """Raised when LLM output cannot be validated against the provided schema.""" +#: Constructor kwargs that configure *validation* rather than the provider +#: call. Clients that stash ``**kwargs`` to replay on every request must strip +#: these, or they reach the provider as unknown request arguments ("Azure: +#: Unrecognized request arguments supplied: free_form_object_as_str"). +VALIDATION_KWARGS = ( + "free_form_object_as_str", + "prompt_based_validation", + "native_structured_output", + "default_generation_kwargs", +) + +#: Substrings that identify a provider refusing the *schema itself* rather than +#: disliking the model's answer. Matching one means retrying the same request is +#: pointless — the schema will be refused identically every time — so the call +#: downgrades to prompt-based injection instead of burning the retry budget. +_SCHEMA_REJECTION_MARKERS = ( + "response_format", + "additionalproperties", + "output_config.format.schema", + "invalid schema", + "json_schema", + "not supported", + "unsupported", + "unrecognized request argument", +) + + +def _is_schema_rejection(exc: BaseException) -> bool: + """Whether *exc* is the provider rejecting our native schema. + + Deliberately narrow: only a client error (HTTP 4xx / ``BadRequest``) whose + message also names a schema concern counts. A 500, a rate limit, or a plain + content-validation failure must stay retryable. + """ + text = str(exc).lower() + looks_client_side = ( + "badrequest" in type(exc).__name__.lower() + or "badrequest" in text + or "400" in text + or "422" in text + ) + if not looks_client_side: + return False + return any(marker in text for marker in _SCHEMA_REJECTION_MARKERS) + + def _is_truncated(raw: Any) -> bool: """Return ``True`` when *raw* was cut off by the token limit. @@ -232,6 +331,14 @@ class ValidatingLLMClient(BaseLLMClient, ABC): injected into the system prompt and no native ``response_format`` kwarg is forwarded. Use for providers that don't support OpenAI-style structured output (e.g. watsonx). + - ``native_structured_output``: tri-state override of the per-model + capability probe. ``None`` (default) auto-detects — providers that + can tell decide, and a model the provider has no data about is + treated as *not* supporting it, because prompt-based validation + works everywhere. Set ``True`` for a model the probe cannot see but + you know honors ``response_format`` (a gateway/proxy model string is + the common case), or ``False`` to force the prompt-based path for + one model without disabling native output client-wide. - ``default_generation_kwargs``: dict of kwargs merged into every ``generate``/``generate_async`` call (e.g. ``{"max_tokens": 8096, "temperature": 0}``). Caller-provided kwargs override the defaults. @@ -241,12 +348,14 @@ class ValidatingLLMClient(BaseLLMClient, ABC): # ``configure_validation`` / constructor kwargs. free_form_object_as_str: bool = False prompt_based_validation: bool = False + native_structured_output: Optional[bool] = None def __init__( self, *, free_form_object_as_str: Optional[bool] = None, prompt_based_validation: Optional[bool] = None, + native_structured_output: Optional[bool] = None, default_generation_kwargs: Optional[Dict[str, Any]] = None, **base_kwargs: Any, ) -> None: @@ -254,6 +363,8 @@ def __init__( self.free_form_object_as_str = free_form_object_as_str if prompt_based_validation is not None: self.prompt_based_validation = prompt_based_validation + if native_structured_output is not None: + self.native_structured_output = native_structured_output self.default_generation_kwargs: Dict[str, Any] = dict( default_generation_kwargs or {} ) @@ -261,6 +372,9 @@ def __init__( # token limit? Retries use it to grow ``max_tokens`` instead of # re-asking with a budget already known to be too small. self._last_response_truncated: bool = False + # Latched once a provider rejects the native schema, so the downgrade to + # prompt-based validation is paid for once per client, not per call. + self._native_schema_rejected: bool = False super().__init__(**base_kwargs) # Wrap the subclass's _parse_llm_response so empty / malformed LLM # outputs retry gracefully (the retry loop treats "" as invalid) @@ -276,6 +390,7 @@ def configure_validation( *, free_form_object_as_str: Optional[bool] = None, prompt_based_validation: Optional[bool] = None, + native_structured_output: Optional[bool] = None, default_generation_kwargs: Optional[Dict[str, Any]] = None, ) -> "ValidatingLLMClient": """Update the validation knobs after construction (chainable).""" @@ -283,6 +398,8 @@ def configure_validation( self.free_form_object_as_str = free_form_object_as_str if prompt_based_validation is not None: self.prompt_based_validation = prompt_based_validation + if native_structured_output is not None: + self.native_structured_output = native_structured_output if default_generation_kwargs is not None: self.default_generation_kwargs = dict(default_generation_kwargs) return self @@ -348,13 +465,45 @@ def provider_class(cls) -> Type[Any]: def supports_native_structured_output(self) -> bool: """Whether the target model honors a native structured-output kwarg. - Defaults to ``True`` (previous behavior). Providers that can tell which - models support it override this; when it returns ``False`` the schema is - injected into the system prompt instead, because a model that ignores - ``response_format`` cannot be constrained by it. + Defaults to ``True``: a schema the provider enforces costs one request, + while the same schema enforced by re-asking on a validation failure + costs several, so native output is worth attempting wherever it might + work. When this returns ``False`` the schema is injected into the system + prompt instead, because a model that ignores ``response_format`` cannot + be constrained by it. + + A wrong guess is self-correcting: a provider that refuses the schema, or + a model that answers a native request with empty content, downgrades the + call in flight and latches the answer for this client's remaining calls + (see ``_note_native_schema_rejected``) — one request, once. Set + ``native_structured_output=False`` to skip the attempt for a model known + to waste it. """ + if self.native_structured_output is not None: + return self.native_structured_output + if self._native_schema_rejected: + return False return True + def _note_native_schema_rejected(self, exc: BaseException) -> None: + """Remember that this provider refuses our native schema. + + Set when a request is rejected for the schema itself, so subsequent + calls on this client go straight to prompt-based injection rather than + spending a request rediscovering it. + """ + import logging as _logging + + if not self._native_schema_rejected: + _logging.getLogger("altk.core.llm.output_parser").warning( + "Provider rejected the native structured-output schema (%s); " + "falling back to prompt-based validation for this client. " + "Details: %s", + type(exc).__name__, + str(exc)[:300], + ) + self._native_schema_rejected = True + def _render_native_schema( self, schema: Union[Dict[str, Any], Type[BaseModel], Type[Any]] ) -> Any: @@ -565,7 +714,21 @@ def generate( if isinstance(raw, str): return self._validate(raw, schema) return raw - except (OutputValidationError, ValueError) as e: + except Exception as e: + # The provider refused the schema itself. Re-sending it would be + # refused identically, so switch this call to prompt-based + # injection and remember the answer for the client's later + # calls. Costs one request, not the whole retry budget. + if schema_field and _is_schema_rejection(e): + self._note_native_schema_rejected(e) + include_schema_in_system_prompt = True + kwargs.pop(schema_field, None) + schema_field = None + instr = self._make_instruction(schema) + current = self._inject_system(prompt, instr) + continue + if not isinstance(e, (OutputValidationError, ValueError)): + raise # ValueError covers providers whose ``_parse_llm_response`` # rejects an empty/contentless response ("No content or tool # calls found in response"). Without it, a single blank reply @@ -578,6 +741,17 @@ def generate( # would grow a conversation that several backends answer with # another empty response, burning every remaining attempt. if not (isinstance(raw, str) and raw.strip()): + # A model that silently ignores ``response_format`` answers + # it with empty content instead of an error, so there is + # nothing to detect but this. Re-asking with the kwarg still + # attached tends to return empty again, so drop it now and + # steer by the prompt — the schema is honored from here on. + if schema_field and not self._last_response_truncated: + self._note_native_schema_rejected(e) + include_schema_in_system_prompt = True + kwargs.pop(schema_field, None) + schema_field = None + instr = self._make_instruction(schema) # Truncated by the token limit? Re-asking with the same # budget yields the identical truncation, so grow it. This # is the common failure for reasoning models, whose @@ -671,7 +845,19 @@ async def generate_async( if isinstance(raw, str): return self._validate(raw, schema) return raw - except (OutputValidationError, ValueError) as e: + except Exception as e: + # See the sync path: a schema rejection is permanent, so + # downgrade to prompt-based injection instead of retrying it. + if schema_field and _is_schema_rejection(e): + self._note_native_schema_rejected(e) + include_schema_in_system_prompt = True + kwargs.pop(schema_field, None) + schema_field = None + instr = self._make_instruction(schema) + current = self._inject_system(prompt, instr) + continue + if not isinstance(e, (OutputValidationError, ValueError)): + raise # ValueError covers providers whose ``_parse_llm_response`` # rejects an empty/contentless response ("No content or tool # calls found in response"). Without it, a single blank reply @@ -684,6 +870,17 @@ async def generate_async( # would grow a conversation that several backends answer with # another empty response, burning every remaining attempt. if not (isinstance(raw, str) and raw.strip()): + # A model that silently ignores ``response_format`` answers + # it with empty content instead of an error, so there is + # nothing to detect but this. Re-asking with the kwarg still + # attached tends to return empty again, so drop it now and + # steer by the prompt — the schema is honored from here on. + if schema_field and not self._last_response_truncated: + self._note_native_schema_rejected(e) + include_schema_in_system_prompt = True + kwargs.pop(schema_field, None) + schema_field = None + instr = self._make_instruction(schema) # Truncated by the token limit? Re-asking with the same # budget yields the identical truncation, so grow it. This # is the common failure for reasoning models, whose diff --git a/altk/core/llm/providers/litellm/litellm.py b/altk/core/llm/providers/litellm/litellm.py index 9d7cd06..cba4967 100644 --- a/altk/core/llm/providers/litellm/litellm.py +++ b/altk/core/llm/providers/litellm/litellm.py @@ -9,7 +9,7 @@ from altk.core.llm.base import LLMClient, register_llm, Hook from altk.core.llm.types import GenerationMode, LLMResponse, ParameterMapper from pydantic import BaseModel -from altk.core.llm.output_parser import ValidatingLLMClient +from altk.core.llm.output_parser import VALIDATION_KWARGS, ValidatingLLMClient @register_llm("litellm") @@ -301,7 +301,13 @@ def __init__( lite_kwargs: Extra arguments passed when initializing the litellm client. """ self.model_path = model_name - self._lite_kwargs = lite_kwargs + # ``_lite_kwargs`` is replayed on every completion call, so the + # validation knobs must not travel with it — a provider rejects them + # as unknown request arguments ("Unrecognized request arguments + # supplied: free_form_object_as_str"). + self._lite_kwargs = { + k: v for k, v in lite_kwargs.items() if k not in VALIDATION_KWARGS + } super().__init__(client=None, hooks=hooks, **lite_kwargs) @classmethod @@ -316,23 +322,37 @@ def provider_class(cls) -> Type[Any]: def supports_native_structured_output(self) -> bool: """Whether this model honors a native ``response_format`` schema. - Uses litellm's per-model capability data, so newly supported models are - picked up without changes here. Models that lack it (e.g. gpt-oss on - watsonx, ollama, gemini) silently ignore ``response_format`` — some - return empty content when it is sent — so the caller falls back to - injecting the schema into the system prompt instead. + Native output is preferred wherever it works — the provider enforces the + schema in one request, where prompt-based validation is enforced by + re-asking. So a model litellm has *no* capability data for is attempted + natively: unknown covers gateway/proxy strings and + ``watsonx/mistral-large-2512``, which honor ``response_format`` fully, + and a wrong guess self-corrects after one request (see + ``_note_native_schema_rejected``). + + A model litellm *knows* to be unsupported is not attempted, and that + negative is worth trusting even though it is imperfect. Measured over + the SPARC metric schemas, forcing native on the smaller watsonx models + made them markedly worse, not better: constrained decoding drives + ``mistral-small-3-1-24b`` and ``llama-3-3-70b`` to emit thousands of + whitespace lines until they exhaust the token budget, taking them from + 7/7 and 6/7 down to 1/7 and 3/7. Some known-negative models do better + natively (``ibm/granite-4-h-small``: 1/14 prompt-based vs 13/14 native), + so this is a per-deployment trade-off rather than a rule — pass + ``native_structured_output=True`` for a model measured to prefer it. """ + if self.native_structured_output is not None: + return self.native_structured_output + if self._native_schema_rejected: + return False try: if litellm.supports_response_schema(model=self.model_path): return True # ``False`` is also what litellm returns for a model it has no - # metadata for, which would silently downgrade every unknown model. - # Only trust a negative answer when the model is actually known. + # metadata for, so only a *known* negative skips the native path. litellm.get_model_info(model=self.model_path) return False except Exception: - # Unknown model: assume native support and let validation + retries - # catch it, preserving the previous behavior. return True def _register_methods(self) -> None: diff --git a/altk/core/llm/providers/openai/openai.py b/altk/core/llm/providers/openai/openai.py index 34bb8ee..09d2823 100644 --- a/altk/core/llm/providers/openai/openai.py +++ b/altk/core/llm/providers/openai/openai.py @@ -81,6 +81,14 @@ def transform_min_tokens(value: Any, mode: Any) -> dict[str, Any]: class BaseValidatingOpenAIClient(ValidatingLLMClient): """Base class for validating OpenAI and Azure OpenAI clients with shared parameter mapping""" + #: ``strict: True`` requires ``additionalProperties: false`` on *every* + #: object schema, which a free-form object cannot satisfy while staying an + #: object ("'additionalProperties' is required to be supplied and to be + #: false"). Rendering those fields as JSON strings is the only shape this + #: API accepts; ``_validate`` widens the schema to match (see + #: ``relax_freeform_object_schema``). Override per instance to opt out. + free_form_object_as_str: bool = True + def _render_native_schema(self, schema: Any) -> Any: """Render *schema* as an OpenAI ``response_format`` payload. @@ -332,11 +340,17 @@ def generate( # type: ignore self, prompt: Union[str, List[Dict[str, str]]], schema: Optional[Any] = None, - schema_field: Optional[str] = None, + schema_field: Optional[str] = "response_format", retries: int = 3, **kwargs: Any, ) -> Union[str, LLMResponse]: - """Generate with OpenAI structured output support""" + """Generate with OpenAI structured output support. + + ``schema_field`` defaults to ``"response_format"`` so a schema is + enforced by OpenAI itself rather than by re-asking on a + validation failure. Pass ``schema_field=None`` to force the + prompt-based path. + """ # Convert string prompts to message format for chat if isinstance(prompt, str): prompt = [{"role": "user", "content": prompt}] @@ -392,11 +406,17 @@ async def generate_async( self, prompt: Union[str, List[Dict[str, str]]], schema: Optional[Any] = None, - schema_field: Optional[str] = None, + schema_field: Optional[str] = "response_format", retries: int = 3, **kwargs: Any, ) -> Any: - """Generate with OpenAI structured output support""" + """Generate with OpenAI structured output support. + + ``schema_field`` defaults to ``"response_format"`` so a schema is + enforced by OpenAI itself rather than by re-asking on a + validation failure. Pass ``schema_field=None`` to force the + prompt-based path. + """ # Convert string prompts to message format for chat if isinstance(prompt, str): prompt = [{"role": "user", "content": prompt}] @@ -516,11 +536,17 @@ def generate( self, prompt: Union[str, List[Dict[str, str]]], schema: Optional[Any] = None, - schema_field: Optional[str] = None, + schema_field: Optional[str] = "response_format", retries: int = 3, **kwargs: Any, ) -> Any: - """Generate with Azure OpenAI structured output support""" + """Generate with Azure OpenAI structured output support. + + ``schema_field`` defaults to ``"response_format"`` so a schema is + enforced by Azure OpenAI itself rather than by re-asking on a + validation failure. Pass ``schema_field=None`` to force the + prompt-based path. + """ # Convert string prompts to message format for chat if isinstance(prompt, str): prompt = [{"role": "user", "content": prompt}] @@ -576,11 +602,17 @@ async def generate_async( self, prompt: Union[str, List[Dict[str, str]]], schema: Optional[Any] = None, - schema_field: Optional[str] = None, + schema_field: Optional[str] = "response_format", retries: int = 3, **kwargs: Any, ) -> Any: - """Generate with Azure OpenAI structured output support""" + """Generate with Azure OpenAI structured output support. + + ``schema_field`` defaults to ``"response_format"`` so a schema is + enforced by Azure OpenAI itself rather than by re-asking on a + validation failure. Pass ``schema_field=None`` to force the + prompt-based path. + """ # Convert string prompts to message format for chat if isinstance(prompt, str): prompt = [{"role": "user", "content": prompt}] diff --git a/altk/pre_tool/sparc/README.md b/altk/pre_tool/sparc/README.md index 0904bdb..a20f46d 100644 --- a/altk/pre_tool/sparc/README.md +++ b/altk/pre_tool/sparc/README.md @@ -267,6 +267,45 @@ sparc = SPARCReflectionComponent(config=config, track=Track.SYNTAX) # TypeError: LLM client must be of type ValidatingLLMClient ``` +### Structured output: native vs prompt-based + +SPARC's metric schemas are nested, enum-bearing, and strict, so how they reach +the model matters. The client sends a native `response_format` schema whenever +the provider is expected to honour it — that way the provider enforces the +schema in a single request, instead of ALTK re-asking on a validation failure. +Measured over SPARC's seven runtime metrics, every model on the native path +needed exactly **one request per call**, and all retry overhead came from +prompt-based models. + +Two knobs cover the cases where the default guesses wrong: + +```python +CLIENT = get_llm("litellm.watsonx.output_val") + +# A model litellm reports as unsupported that is in fact far better natively +# (ibm/granite-4-h-small: 1/21 prompt-based vs 21/21 native on these schemas). +config = ComponentConfig( + llm_client=CLIENT(model_name="ibm/granite-4-h-small", + native_structured_output=True) +) + +# ...and the reverse: force the prompt-based path for a model that wastes the +# native attempt. +config = ComponentConfig( + llm_client=CLIENT(model_name="some/model", native_structured_output=False) +) +``` + +Both are also settable after construction via +`client.configure_validation(...)`. If a provider rejects the schema outright, +no configuration is needed — the call downgrades to the prompt-based path and +remembers the answer for that client, so the discovery costs one request. + +For providers whose structured output is strict about `additionalProperties` +(Azure OpenAI, Bedrock), also pass `free_form_object_as_str=True` so SPARC's +free-form `correction` fields are rendered in a shape those APIs accept. The +OpenAI and Azure clients default to this already. + ## Configuration ### Track-Based Configuration diff --git a/tests/core/test_litellm_structured_output.py b/tests/core/test_litellm_structured_output.py index dfc9c38..8720cae 100644 --- a/tests/core/test_litellm_structured_output.py +++ b/tests/core/test_litellm_structured_output.py @@ -51,26 +51,109 @@ def test_still_raises_when_nothing_usable(self): LiteLLMClientOutputVal._parse_llm_response(client, _response(content="")) +def _bare_client(cls, model_path): + """A client with only the attributes the capability gate reads. + + ``__new__`` skips ``__init__`` on purpose (no provider connection), so the + instance state the gate consults is set explicitly here. + """ + client = cls.__new__(cls) + client.model_path = model_path + client.native_structured_output = None + client._native_schema_rejected = False + return client + + class TestNativeStructuredOutputCapability: - """Native ``response_format`` is only used where the model honors it.""" + """Native ``response_format`` is preferred wherever it can work.""" @pytest.mark.parametrize( - "model_name, expected", + "cls, model_path", [ - # Reasoning model with no response-schema support: must fall back. - ("openai/gpt-oss-120b", False), - # Model litellm reports as supporting response schemas. - ("mistralai/mistral-large", True), + # No metadata at all: gateway/proxy strings, and the model from + # issue #119 that turned out to honor response_format fully. Native + # is attempted — it costs one request per call instead of several, + # and a refusal self-corrects. + (WatsonxLiteLLMClientOutputVal, "watsonx/mistral-large-2512"), + (LiteLLMClientOutputVal, "some-provider/not-a-real-model-xyz"), + (LiteLLMClientOutputVal, "openai/aws/claude-haiku-4-5"), + # Known and reported as supporting response schemas. + (WatsonxLiteLLMClientOutputVal, "watsonx/mistralai/mistral-large"), ], ) - def test_watsonx_capability_is_per_model(self, model_name, expected): - client = WatsonxLiteLLMClientOutputVal.__new__(WatsonxLiteLLMClientOutputVal) - client.model_path = f"watsonx/{model_name}" - assert client.supports_native_structured_output() is expected + def test_native_is_attempted_when_not_known_unsupported(self, cls, model_path): + assert _bare_client(cls, model_path).supports_native_structured_output() is True - def test_unknown_model_assumes_native_support(self): - """Unknown models keep the previous behavior rather than silently - switching every call to prompt-based validation.""" - client = LiteLLMClientOutputVal.__new__(LiteLLMClientOutputVal) - client.model_path = "some-provider/not-a-real-model-xyz" + @pytest.mark.parametrize( + "model_path", + [ + # Known-unsupported is trusted: forcing native on the smaller + # watsonx models made them worse, because constrained decoding + # drives them to emit whitespace until the token budget is gone. + "watsonx/openai/gpt-oss-120b", + "watsonx/mistralai/mistral-small-3-1-24b-instruct-2503", + ], + ) + def test_known_unsupported_skips_native(self, model_path): + client = _bare_client(WatsonxLiteLLMClientOutputVal, model_path) + assert client.supports_native_structured_output() is False + # ...but a caller who measured otherwise can still opt in. + client.native_structured_output = True assert client.supports_native_structured_output() is True + + def test_rejection_latches_off_native(self): + """Once a provider refuses the schema, stop offering it.""" + client = _bare_client(LiteLLMClientOutputVal, "openai/aws/claude-haiku-4-5") + assert client.supports_native_structured_output() is True + client._note_native_schema_rejected( + ValueError("BedrockException: output_config.format.schema not supported") + ) + assert client.supports_native_structured_output() is False + + def test_explicit_override_beats_a_recorded_rejection(self): + client = _bare_client(LiteLLMClientOutputVal, "openai/gpt-4o") + client._native_schema_rejected = True + client.native_structured_output = True + assert client.supports_native_structured_output() is True + + @pytest.mark.parametrize( + "model_path, override, expected", + [ + # Unknown proxy/gateway model the caller knows does honor it. + ("openai/some-gateway-model-xyz", True, True), + # Known-supporting model the caller wants steered by prompt anyway. + ("openai/gpt-4o", False, False), + ], + ) + def test_native_structured_output_override_wins( + self, model_path, override, expected + ): + client = _bare_client(LiteLLMClientOutputVal, model_path) + client.native_structured_output = override + assert client.supports_native_structured_output() is expected + + +class TestValidationKwargsDoNotReachTheProvider: + """Validation knobs configure ALTK, not the completion request. + + ``_lite_kwargs`` is replayed on every call, so a knob passed to the + constructor used to travel with it and the provider rejected the request: + "Unrecognized request arguments supplied: free_form_object_as_str, + native_structured_output". + """ + + def test_knobs_are_stripped_from_replayed_kwargs(self): + # No request is made here, so the client needs no credentials. + client = LiteLLMClientOutputVal( + model_name="openai/gpt-4o", + api_base="https://example.invalid/v1", + native_structured_output=True, + free_form_object_as_str=True, + prompt_based_validation=False, + default_generation_kwargs={"max_tokens": 32}, + ) + assert set(client._lite_kwargs) == {"api_base"} + # ...while still taking effect on the client itself. + assert client.native_structured_output is True + assert client.free_form_object_as_str is True + assert client.default_generation_kwargs == {"max_tokens": 32} diff --git a/tests/core/test_validating_llm_client.py b/tests/core/test_validating_llm_client.py index 9e0a665..d5e5c1c 100644 --- a/tests/core/test_validating_llm_client.py +++ b/tests/core/test_validating_llm_client.py @@ -117,6 +117,39 @@ def test_freeform_flag_recurses_into_nested_objects(self): assert issubclass(nested, BaseModel) assert set(nested.model_fields) == {"x"} + def test_nested_models_forbid_extras_without_repeating_the_keyword(self): + # Strict providers require ``additionalProperties: false`` on *every* + # object schema; nested sub-schemas rarely repeat the keyword, so + # forbid is the default. See issue #120. + rendered = json_schema_to_pydantic_model( + { + "type": "object", + "additionalProperties": False, + "properties": { + "sub": {"type": "object", "properties": {"x": {"type": "string"}}} + }, + } + ).model_json_schema() + assert rendered["additionalProperties"] is False + assert all( + d["additionalProperties"] is False for d in rendered["$defs"].values() + ) + + def test_explicit_additional_properties_true_is_respected(self): + rendered = json_schema_to_pydantic_model( + { + "type": "object", + "properties": { + "sub": { + "type": "object", + "additionalProperties": True, + "properties": {"x": {"type": "string"}}, + } + }, + } + ).model_json_schema() + assert "additionalProperties" not in next(iter(rendered["$defs"].values())) + # --------------------------------------------------------------------------- # relax_freeform_object_schema @@ -145,6 +178,36 @@ def test_deep_copy_does_not_mutate_input(self): _ = relax_freeform_object_schema(schema) assert schema["properties"]["a"]["type"] == "object" + _nested = { + "type": "object", + "properties": { + "issues": { + "type": "array", + "items": { + "type": "object", + "properties": {"corrected_value": {"type": "object"}}, + }, + } + }, + } + + def test_relaxes_freeform_objects_at_any_depth(self): + # ``json_schema_to_pydantic_model`` stringifies free-form objects at + # every depth, so widening only the top level left nested replies + # failing validation. See issue #120. + out = relax_freeform_object_schema(self._nested) + inner = out["properties"]["issues"]["items"]["properties"]["corrected_value"] + assert inner["type"] == ["object", "string"] + + def test_relaxed_validation_accepts_nested_json_string(self): + c = _FakeValidating(free_form_object_as_str=True, client=object()) + payload = '{"issues": [{"corrected_value": "{\\"p\\": 1}"}]}' + assert c._validate(payload, self._nested)["issues"][0]["corrected_value"] == ( + '{"p": 1}' + ) + # The object-literal form still validates against the same schema. + assert c._validate('{"issues": [{"corrected_value": {"p": 1}}]}', self._nested) + # --------------------------------------------------------------------------- # ValidatingLLMClient configuration surface @@ -319,18 +382,52 @@ def _parse_llm_response(self, raw): # will be wrapped class TestSchemaFidelity: - def test_numeric_bounds_survive(self): - m = json_schema_to_pydantic_model( + _bounded = { + "type": "object", + "properties": {"score": {"type": "integer", "minimum": 1, "maximum": 5}}, + "required": ["score"], + } + + def test_numeric_bounds_stripped_from_wire_schema(self): + # Bedrock rejects an ``integer`` carrying minimum/maximum outright + # ("For 'integer' type, properties maximum, minimum are not + # supported"), and this model is *only* the wire schema — so the + # bounds must not survive into it. See issue #118. + prop = json_schema_to_pydantic_model(self._bounded).model_json_schema()[ + "properties" + ]["score"] + assert "minimum" not in prop + assert "maximum" not in prop + assert prop["type"] == "integer" + + def test_out_of_range_value_is_still_rejected(self): + # Strictness lives in ``jsonschema`` against the *original* schema, + # not in the generated model, so dropping the bounds above costs + # nothing: an out-of-range score is still invalid and gets retried. + c = _FakeValidating(client=object()) + with pytest.raises(OutputValidationError): + c._validate('{"score": 9}', self._bounded) + assert c._validate('{"score": 3}', self._bounded) == {"score": 3} + + def test_string_and_array_constraints_still_survive(self): + # Only the numeric bounds are dropped; the rest keep the wire schema + # faithful to the source. + props = json_schema_to_pydantic_model( { "type": "object", "properties": { - "score": {"type": "integer", "minimum": 1, "maximum": 5} + "name": {"type": "string", "minLength": 2, "pattern": "^[a-z]+$"}, + "tags": { + "type": "array", + "items": {"type": "string"}, + "minItems": 1, + }, }, } - ) - prop = m.model_json_schema()["properties"]["score"] - assert prop["minimum"] == 1 - assert prop["maximum"] == 5 + ).model_json_schema()["properties"] + assert props["name"]["minLength"] == 2 + assert props["name"]["pattern"] == "^[a-z]+$" + assert props["tags"]["minItems"] == 1 def test_enum_survives_inside_array_items(self): # A Field-level constraint cannot reach into ``items``; only a real @@ -515,3 +612,250 @@ def supports_native_structured_output(self) -> bool: assert "response_format" not in observed[-1] # schema went into a system message instead assert observed[-1]["prompt"][0]["role"] == "system" + + +# --------------------------------------------------------------------------- +# Provider-strictness of the rendered wire schema, pinned against ALTK's own +# SPARC metric schemas — the shapes that issues #118 and #120 were filed on. +# --------------------------------------------------------------------------- + + +def _sparc_runtime_schemas() -> list[tuple[str, dict]]: + import glob + from pathlib import Path + + root = Path(__file__).resolve().parents[2] + pattern = str( + root / "altk/pre_tool/sparc/function_calling/metrics/*/*_runtime.json" + ) + out = [] + for path in sorted(glob.glob(pattern)): + for metric in json.loads(Path(path).read_text()): + out.append((metric["name"], metric["jsonschema"])) + return out + + +def _strictness_violations(node: Any, path: str = "$") -> list[str]: + """Report every object schema a strict provider would reject.""" + bad: list[str] = [] + if isinstance(node, dict): + if node.get("additionalProperties") is True: + bad.append(f"additionalProperties: true at {path}") + if node.get("type") == "object" and "additionalProperties" not in node: + bad.append(f"additionalProperties missing at {path}") + if "minimum" in node or "maximum" in node: + bad.append(f"numeric bound at {path}") + for key, value in node.items(): + bad += _strictness_violations(value, f"{path}.{key}") + elif isinstance(node, list): + for i, value in enumerate(node): + bad += _strictness_violations(value, f"{path}[{i}]") + return bad + + +class TestSparcSchemasAreProviderStrict: + @pytest.mark.parametrize("name, schema", _sparc_runtime_schemas()) + def test_rendered_schema_is_strict_safe(self, name, schema): + rendered = json_schema_to_pydantic_model( + schema, free_form_object_as_str=True + ).model_json_schema() + assert _strictness_violations(rendered) == [], name + + @pytest.mark.parametrize("name, schema", _sparc_runtime_schemas()) + def test_no_numeric_bounds_or_missing_keyword_by_default(self, name, schema): + # Without the free-form-as-string workaround, free-form objects still + # render as ``additionalProperties: true`` (that is the documented + # trade-off of the knob) — but the #118 bounds and the #120 missing + # keyword must be gone on the default path too. + rendered = json_schema_to_pydantic_model(schema).model_json_schema() + leftover = [v for v in _strictness_violations(rendered) if "true" not in v] + assert leftover == [], name + + +# --------------------------------------------------------------------------- +# Native-first: a provider that refuses the schema downgrades the client once, +# instead of spending the retry budget re-sending a schema it will refuse again. +# --------------------------------------------------------------------------- + + +class _Rejects(_FakeValidating): + """Fails every native attempt the way a strict provider does.""" + + def supports_native_structured_output(self) -> bool: + if self._native_schema_rejected: + return False + return True + + +class TestNativeSchemaDowngrade: + _schema = { + "type": "object", + "properties": {"a": {"type": "string"}}, + "required": ["a"], + } + + def _run(self, monkeypatch, error, scripted_ok='{"a": "ok"}'): + observed: list = [] + from altk.core.llm.base import BaseLLMClient + + def fake_generate(self, **kwargs): # noqa: ANN001 + observed.append(kwargs) + if "response_format" in kwargs: + raise error + return self._parse_llm_response(scripted_ok) + + monkeypatch.setattr(BaseLLMClient, "_generate", fake_generate, raising=True) + c = _Rejects(client=object()) + got = c.generate( + [{"role": "user", "content": "hi"}], + schema=self._schema, + schema_field="response_format", + retries=3, + ) + return c, observed, got + + def test_schema_rejection_retries_without_the_kwarg(self, monkeypatch): + err = Exception( + "litellm.BadRequestError: BedrockException - output_config.format.schema: " + "For 'integer' type, properties maximum, minimum are not supported" + ) + c, observed, got = self._run(monkeypatch, err) + assert got == {"a": "ok"} + assert len(observed) == 2, "one native attempt, then one prompt-based" + assert "response_format" in observed[0] + assert "response_format" not in observed[1] + # The schema moved into a system message on the second attempt. + assert observed[1]["prompt"][0]["role"] == "system" + + def test_rejection_is_remembered_for_later_calls(self, monkeypatch): + c, observed, _ = self._run( + monkeypatch, + Exception("Error code: 400 - Invalid schema for response_format"), + ) + assert c._native_schema_rejected is True + assert c.supports_native_structured_output() is False + before = len(observed) + c.generate( + [{"role": "user", "content": "again"}], + schema=self._schema, + schema_field="response_format", + retries=3, + ) + # The second call never tries native again — one request, no probe. + assert len(observed) == before + 1 + assert "response_format" not in observed[-1] + + def test_unrelated_provider_errors_still_propagate(self, monkeypatch): + # A 500 or a rate limit must not be mistaken for a schema rejection, + # or a transient outage would silently disable native output. + err = RuntimeError("InternalServerError: upstream connect error") + with pytest.raises(RuntimeError): + self._run(monkeypatch, err) + + def test_empty_content_under_native_drops_the_kwarg(self, monkeypatch): + """A model that ignores ``response_format`` answers with empty content + rather than an error — the only signal available (issue #119).""" + observed: list = [] + from altk.core.llm.base import BaseLLMClient + + def fake_generate(self, **kwargs): # noqa: ANN001 + observed.append(kwargs) + if "response_format" in kwargs: + return self._parse_llm_response("") + return self._parse_llm_response('{"a": "ok"}') + + monkeypatch.setattr(BaseLLMClient, "_generate", fake_generate, raising=True) + c = _Rejects(client=object()) + got = c.generate( + [{"role": "user", "content": "hi"}], + schema=self._schema, + schema_field="response_format", + retries=3, + ) + assert got == {"a": "ok"} + assert "response_format" in observed[0] + assert "response_format" not in observed[1] + + +# --------------------------------------------------------------------------- +# Bounded integers ride the wire as an enum, so the provider keeps enforcing +# the range without the keywords strict providers reject. +# --------------------------------------------------------------------------- + + +class TestBoundedIntegerAsEnum: + def test_small_range_becomes_an_enum(self): + prop = json_schema_to_pydantic_model( + { + "type": "object", + "properties": { + "output": {"type": "integer", "minimum": 1, "maximum": 5} + }, + } + ).model_json_schema()["properties"]["output"] + assert prop["enum"] == [1, 2, 3, 4, 5] + assert "minimum" not in prop and "maximum" not in prop + + def test_wide_range_is_left_as_a_plain_integer(self): + prop = json_schema_to_pydantic_model( + { + "type": "object", + "properties": { + "count": {"type": "integer", "minimum": 0, "maximum": 1_000_000} + }, + } + ).model_json_schema()["properties"]["count"] + assert "enum" not in prop + assert prop["type"] == "integer" + + @pytest.mark.parametrize( + "prop_schema", + [ + {"type": "number", "minimum": 0, "maximum": 1}, # confidence: not int + {"type": "integer", "minimum": 1}, # half-open + {"type": "integer", "maximum": 5}, + {"type": "boolean"}, + ], + ) + def test_shapes_that_must_not_be_enumerated(self, prop_schema): + prop = json_schema_to_pydantic_model( + {"type": "object", "properties": {"x": prop_schema}} + ).model_json_schema()["properties"]["x"] + assert "enum" not in prop + + def test_enum_value_is_accepted_and_out_of_range_rejected(self): + schema = { + "type": "object", + "properties": {"output": {"type": "integer", "minimum": 1, "maximum": 5}}, + "required": ["output"], + } + c = _FakeValidating(client=object()) + assert c._validate('{"output": 4}', schema) == {"output": 4} + with pytest.raises(OutputValidationError): + c._validate('{"output": 7}', schema) + + +# --------------------------------------------------------------------------- +# The OpenAI/Azure validating clients must reach for native structured output +# by default — they are the providers with the strongest support for it, and +# leaving it off meant every schema was enforced by retrying instead. +# --------------------------------------------------------------------------- + + +class TestOpenAIClientsDefaultToNative: + @pytest.mark.parametrize( + "registry_name, method", + [ + ("openai.sync.output_val", "generate"), + ("openai.async.output_val", "generate_async"), + ("azure_openai.sync.output_val", "generate"), + ("azure_openai.async.output_val", "generate_async"), + ], + ) + def test_schema_field_defaults_to_response_format(self, registry_name, method): + import inspect + + from altk.core.llm import get_llm + + sig = inspect.signature(getattr(get_llm(registry_name), method)) + assert sig.parameters["schema_field"].default == "response_format"