mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(anthropic): keep both sides' constraints on a shared property, and serialise
Review of #43443 caught two issues, and chasing the first uncovered a third that was worse than either. Where a root $ref target and a sibling declare the same property, the sibling replaced the target's schema, so a constraint such as minLength vanished. Keys beside a $ref mean both schemas apply, which cannot be expressed by choosing one, so a property declared on both sides now becomes an allOf of the two. Anthropic accepts a combinator nested inside a property; only a root one is a problem. Identical declarations skip the wrapper. The properties map was a MappingProxyType, which json cannot encode. The sanitizer's outer dict() unwraps only the top level, so the proxy reached the request body and every root $ref tool call would have failed to serialise. It went unnoticed because the earlier proof used a root anyOf, which builds its properties through the flattener instead. Now a plain dict, matching the reasoning already recorded beside the flattener's own dicts. Drop the test that asserted a private helper's identities and length. The deduplication is a pure performance change with no observable behaviour, so a structural test was the wrong way to cover it; the behavioural equivalence test stays.
This commit is contained in:
parent
7320891a14
commit
6cba5304f2
2 changed files with 85 additions and 19 deletions
|
|
@ -11,7 +11,9 @@ from itertools import groupby, islice
|
|||
from os import PathLike
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Any, Final, Literal, TypeVar, cast
|
||||
from typing import TYPE_CHECKING, Any, Final, Literal, TypedDict, TypeVar, cast
|
||||
|
||||
from typing_extensions import ReadOnly
|
||||
|
||||
import litellm
|
||||
from litellm import verbose_logger
|
||||
|
|
@ -1211,15 +1213,35 @@ def _resolve_local_schema_ref(root: Mapping[str, object], ref: str) -> Mapping[s
|
|||
return target if isinstance(target, dict) else None
|
||||
|
||||
|
||||
class _AllOfSchema(TypedDict):
|
||||
allOf: ReadOnly[Sequence[object]]
|
||||
|
||||
|
||||
def _both_constraints_apply(target_property: object, sibling_property: object) -> object:
|
||||
"""One property schema satisfying both sides of a ``$ref`` merge.
|
||||
|
||||
A plain dict rather than a ``MappingProxyType``, because this value is
|
||||
serialised straight into the provider request and ``json`` cannot encode a
|
||||
mapping proxy.
|
||||
"""
|
||||
if target_property is None or target_property == sibling_property:
|
||||
return sibling_property
|
||||
both: Final = [target_property, sibling_property] # mutable-ok: allOf is a JSON array; a tuple is not one
|
||||
combined: Final[_AllOfSchema] = {"allOf": both}
|
||||
return combined
|
||||
|
||||
|
||||
def _inline_root_schema_ref(schema: Mapping[str, object]) -> Mapping[str, object]:
|
||||
"""Merge a root-level local ``$ref`` with the schema it points at.
|
||||
|
||||
Per JSON Schema, keys beside a ``$ref`` apply on top of what it references
|
||||
rather than being replaced by it, so ``properties`` and ``required`` union
|
||||
and the local key wins elsewhere. Dropping the siblings instead would lose
|
||||
constraints the caller stated here, such as ``additionalProperties: false``.
|
||||
A root without a ``$ref``, or one pointing outside the document or at a
|
||||
missing name, is returned as is.
|
||||
rather than being replaced by it. ``properties`` and ``required`` therefore
|
||||
union, and a property both sides declare becomes an ``allOf`` of the two so
|
||||
neither one's constraints are lost; Anthropic accepts a combinator nested
|
||||
inside a property, only a root one is a problem. Replacing the root outright
|
||||
would drop what the caller stated here, such as ``additionalProperties:
|
||||
false``. A root without a ``$ref``, or one pointing outside the document or
|
||||
at a missing name, is returned as is.
|
||||
"""
|
||||
ref: Final = schema.get("$ref")
|
||||
if not isinstance(ref, str):
|
||||
|
|
@ -1228,7 +1250,11 @@ def _inline_root_schema_ref(schema: Mapping[str, object]) -> Mapping[str, object
|
|||
if target is None:
|
||||
return schema
|
||||
siblings: Final = MappingProxyType({key: value for key, value in schema.items() if key != "$ref"})
|
||||
properties: Final = MappingProxyType({**_schema_properties(target), **_schema_properties(siblings)})
|
||||
target_properties: Final = _schema_properties(target)
|
||||
sibling_wins: Final = MappingProxyType({**target_properties, **_schema_properties(siblings)})
|
||||
properties: Final = { # mutable-ok: tool parameters are JSON dicts, and a mapping proxy will not serialise
|
||||
name: _both_constraints_apply(target_properties.get(name), value) for name, value in sibling_wins.items()
|
||||
}
|
||||
required: Final = sorted(_schema_required_names(target) | _schema_required_names(siblings))
|
||||
required_update: Final = MappingProxyType({"required": required}) if required else _EMPTY_SCHEMA
|
||||
return MappingProxyType({**target, **siblings, "properties": properties, **required_update})
|
||||
|
|
|
|||
|
|
@ -2061,6 +2061,43 @@ class TestSanitizeInputSchemaForAnthropic:
|
|||
f"a constraint stated beside the $ref must not be dropped, got {dict(result)}"
|
||||
)
|
||||
|
||||
def test_a_property_declared_on_both_sides_of_a_ref_keeps_both_constraints(self):
|
||||
"""Keys beside a ``$ref`` apply on top of it, so where both declare the
|
||||
same property neither constraint may be dropped. Anthropic accepts a
|
||||
combinator nested inside a property, so both can be carried."""
|
||||
from litellm.litellm_core_utils.prompt_templates.common_utils import (
|
||||
sanitize_input_schema_for_anthropic,
|
||||
)
|
||||
|
||||
result = sanitize_input_schema_for_anthropic(
|
||||
{
|
||||
"$defs": {"A": {"type": "object", "properties": {"a": {"type": "string", "minLength": 5}}}},
|
||||
"$ref": "#/$defs/A",
|
||||
"properties": {"a": {"type": "string", "maxLength": 9}},
|
||||
}
|
||||
)
|
||||
|
||||
assert result["properties"]["a"] == {
|
||||
"allOf": [{"type": "string", "minLength": 5}, {"type": "string", "maxLength": 9}]
|
||||
}, f"both sides' constraints on a shared property must survive, got {dict(result)}"
|
||||
|
||||
def test_a_property_identical_on_both_sides_is_not_wrapped(self):
|
||||
from litellm.litellm_core_utils.prompt_templates.common_utils import (
|
||||
sanitize_input_schema_for_anthropic,
|
||||
)
|
||||
|
||||
shared = {"type": "string"}
|
||||
|
||||
result = sanitize_input_schema_for_anthropic(
|
||||
{
|
||||
"$defs": {"A": {"type": "object", "properties": {"a": shared}}},
|
||||
"$ref": "#/$defs/A",
|
||||
"properties": {"a": shared},
|
||||
}
|
||||
)
|
||||
|
||||
assert result["properties"]["a"] == shared, f"identical declarations need no allOf wrapper, got {dict(result)}"
|
||||
|
||||
def test_ref_root_resolves_to_the_schema_it_points_at(self):
|
||||
from litellm.litellm_core_utils.prompt_templates.common_utils import (
|
||||
sanitize_input_schema_for_anthropic,
|
||||
|
|
@ -2134,22 +2171,25 @@ class TestSanitizeInputSchemaForAnthropic:
|
|||
|
||||
assert dict(many) == dict(once), f"repeating a branch must not change the result, got {dict(many)}"
|
||||
|
||||
def test_branches_that_resolved_to_one_object_collapse_to_one(self):
|
||||
"""Repeated ``$ref``s share a single memoised result, so the merge only
|
||||
needs to see it once. Collapsing them is what keeps a compact schema
|
||||
repeating one reference from costing work per branch."""
|
||||
@pytest.mark.parametrize(
|
||||
"name, root",
|
||||
[
|
||||
("ref root", {"$ref": "#/$defs/A"}),
|
||||
("union root", {"anyOf": [{"$ref": "#/$defs/A"}, {"$ref": "#/$defs/B"}]}),
|
||||
],
|
||||
)
|
||||
def test_the_sanitized_schema_can_be_serialized_onto_the_wire(self, name, root):
|
||||
"""The result is JSON-encoded into the provider request, so every value in
|
||||
it has to be encodable. A mapping proxy is not."""
|
||||
from litellm.litellm_core_utils.prompt_templates.common_utils import (
|
||||
_distinct_branches,
|
||||
sanitize_input_schema_for_anthropic,
|
||||
)
|
||||
|
||||
shared = {"type": "object", "properties": {"a": {"type": "string"}}}
|
||||
other = {"type": "object", "properties": {"b": {"type": "string"}}}
|
||||
result = sanitize_input_schema_for_anthropic({"$defs": {"A": self.A, "B": self.B}, **root})
|
||||
|
||||
result = _distinct_branches((shared, shared, other, shared))
|
||||
|
||||
assert len(result) == 2, f"identical objects should collapse, got {len(result)} branches"
|
||||
assert result[0] is shared, f"the first occurrence should be kept, got {result}"
|
||||
assert result[1] is other, f"distinct branches must survive in order, got {result}"
|
||||
assert json.loads(json.dumps(result))["properties"], (
|
||||
f"{name} must survive a JSON round trip, got {dict(result)}"
|
||||
)
|
||||
|
||||
def test_a_pydantic_union_tool_reaches_anthropic_with_its_arguments(self):
|
||||
"""The reporter's path: Pydantic emits a root ``anyOf`` over ``$defs``."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue