scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-06 02:48:13 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 103

fix: prevent in-place schema mutation from breaking cache keys (#18784)

Browse source 
_remove_strict_from_schema() and _remove_additional_properties() mutate
tool schemas in-place via `del schema[key]`. Because the cache key is
computed from the same kwargs dict before (GET) and after (SET) the
provider call, the mutation causes the SET key to differ from the GET
key — making the disk cache permanently miss for any request with tools
containing `strict: true` or `additionalProperties: false`.

The fix wraps both functions to deep-copy before mutating. Internal
`_in_place` helpers preserve the recursive logic for the already-copied
data. All existing callers use the `x = func(x)` return pattern, so no
caller changes are needed.

Affected providers: hosted_vllm, watsonx, vertex_ai/gemini.

Note: the Vertex AI team already works around this with a defensive
deepcopy at their call site (vertex_and_google_ai_studio_gemini.py:648).
This fix makes that defensive copy redundant but harmless.

Prior art: PR #9693 attempted this identical fix 14 months ago but went
stale due to CLA paperwork, not technical rejection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Rolando Bosch 2026-02-28 15:19:24 -08:00
parent bc9c28eb80
commit fad3355368
3 changed files with 380 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
litellm/utils.py
View file

@ -3453,44 +3453,58 @@ def _remove_additional_properties(schema):
"""
clean out 'additionalProperties = False'. Causes vertexai/gemini OpenAI API Schema errors - https://github.com/langchain-ai/langchainjs/issues/5240
Returns a deep copy with 'additionalProperties' removed — the original schema
is not mutated. This is critical for cache-key stability: the cache key is
computed from the caller's original tools dict, so in-place mutation would
cause the key at GET time to differ from the key at SET time.
Relevant Issues: https://github.com/BerriAI/litellm/issues/6136, https://github.com/BerriAI/litellm/issues/6088
See also: https://github.com/BerriAI/litellm/issues/18784
"""
schema = copy.deepcopy(schema)
_remove_additional_properties_in_place(schema)
return schema
def _remove_additional_properties_in_place(schema):
"""In-place recursive helper — only called on an already-copied schema."""
if isinstance(schema, dict):
# Remove the 'additionalProperties' key if it exists and is set to False
if "additionalProperties" in schema and schema["additionalProperties"] is False:
del schema["additionalProperties"]
# Recursively process all dictionary values
for key, value in schema.items():
_remove_additional_properties(value)
_remove_additional_properties_in_place(value)
elif isinstance(schema, list):
# Recursively process all items in the list
for item in schema:
_remove_additional_properties(item)
return schema
_remove_additional_properties_in_place(item)
def _remove_strict_from_schema(schema):
"""
Remove 'strict' keys from a JSON schema structure.
Returns a deep copy with 'strict' removed — the original schema is not
mutated. This is critical for cache-key stability: the cache key is
computed from the caller's original tools dict, so in-place mutation would
cause the key at GET time to differ from the key at SET time.
Relevant Issues: https://github.com/BerriAI/litellm/issues/6136, https://github.com/BerriAI/litellm/issues/6088
See also: https://github.com/BerriAI/litellm/issues/18784
"""
schema = copy.deepcopy(schema)
_remove_strict_from_schema_in_place(schema)
return schema
def _remove_strict_from_schema_in_place(schema):
"""In-place recursive helper — only called on an already-copied schema."""
if isinstance(schema, dict):
# Remove the 'additionalProperties' key if it exists and is set to False
if "strict" in schema:
del schema["strict"]
# Recursively process all dictionary values
for key, value in schema.items():
_remove_strict_from_schema(value)
_remove_strict_from_schema_in_place(value)
elif isinstance(schema, list):
# Recursively process all items in the list
for item in schema:
_remove_strict_from_schema(item)
return schema
_remove_strict_from_schema_in_place(item)
def _remove_json_schema_refs(schema, max_depth=10):

2
tests/code_coverage_tests/recursive_detector.py
View file

@ -4,7 +4,9 @@ import os
IGNORE_FUNCTIONS = [
"_format_type",
"_remove_additional_properties",
"_remove_additional_properties_in_place",
"_remove_strict_from_schema",
"_remove_strict_from_schema_in_place",
"filter_schema_fields",
"text_completion",
"_check_for_os_environ_vars",

346
tests/local_testing/test_schema_mutation_cache_key.py Normal file
View file

@ -0,0 +1,346 @@
"""
Tests for https://github.com/BerriAI/litellm/issues/18784
_remove_strict_from_schema and _remove_additional_properties must NOT mutate
their input, because the caller's original tools dict is used to compute cache
keys both before and after the LLM provider call. If the schema is mutated
in-place, the cache key at GET time differs from the key at SET time and the
disk cache is effectively broken.
"""
import copy
import tempfile
import pytest
import litellm
from litellm.caching.caching import Cache, LiteLLMCacheType
from litellm.utils import (
_remove_additional_properties,
_remove_additional_properties_in_place,
_remove_strict_from_schema,
_remove_strict_from_schema_in_place,
)
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_tools_with_strict():
"""Return a realistic tools list with strict=True and additionalProperties."""
return [
{
"type": "function",
"function": {
"name": "get_weather",
"parameters": {
"type": "object",
"properties": {
"location": {"type": "string"},
},
"additionalProperties": False,
},
"strict": True,
},
}
]
# ---------------------------------------------------------------------------
# Core: in-place mutation must not happen
# ---------------------------------------------------------------------------
class TestRemoveStrictDoesNotMutate:
"""_remove_strict_from_schema must return a new object, leaving the input untouched."""
def test_original_unchanged_after_remove_strict(self):
tools = _make_tools_with_strict()
original = copy.deepcopy(tools)
_remove_strict_from_schema(tools)
assert tools == original, (
"_remove_strict_from_schema mutated the input in-place — "
"this breaks cache-key computation (issue #18784)"
)
def test_return_value_has_strict_removed(self):
tools = _make_tools_with_strict()
result = _remove_strict_from_schema(tools)
# 'strict' should be gone from the returned copy
assert "strict" not in result[0]["function"]
def test_original_still_has_strict(self):
tools = _make_tools_with_strict()
_remove_strict_from_schema(tools)
# The original must still have 'strict'
assert tools[0]["function"]["strict"] is True
class TestRemoveAdditionalPropertiesDoesNotMutate:
"""_remove_additional_properties must return a new object, leaving the input untouched."""
def test_original_unchanged_after_remove_additional_properties(self):
tools = _make_tools_with_strict()
original = copy.deepcopy(tools)
_remove_additional_properties(tools)
assert tools == original, (
"_remove_additional_properties mutated the input in-place — "
"this breaks cache-key computation (issue #18784)"
)
def test_return_value_has_additional_properties_removed(self):
tools = _make_tools_with_strict()
result = _remove_additional_properties(tools)
assert "additionalProperties" not in result[0]["function"]["parameters"]
def test_original_still_has_additional_properties(self):
tools = _make_tools_with_strict()
_remove_additional_properties(tools)
assert tools[0]["function"]["parameters"]["additionalProperties"] is False
# ---------------------------------------------------------------------------
# Integration: cache key must be stable across remove_strict calls
# ---------------------------------------------------------------------------
class TestCacheKeyStability:
"""The cache key must be identical before and after _remove_strict_from_schema
is applied to the tools, because the cache GET uses the original tools and
the cache SET uses whatever is in kwargs after the provider call."""
def test_cache_key_matches_before_and_after_remove_strict(self):
tools = _make_tools_with_strict()
with tempfile.TemporaryDirectory() as cache_dir:
cache = Cache(type=LiteLLMCacheType.DISK, disk_cache_dir=cache_dir)
key_before = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
# Simulate what the provider does internally
_remove_strict_from_schema(tools)
key_after = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
assert key_before == key_after, (
f"Cache key changed after _remove_strict_from_schema: "
f"{key_before!r} != {key_after!r} — disk cache is broken (issue #18784)"
)
def test_cache_key_matches_before_and_after_remove_additional_properties(self):
tools = _make_tools_with_strict()
with tempfile.TemporaryDirectory() as cache_dir:
cache = Cache(type=LiteLLMCacheType.DISK, disk_cache_dir=cache_dir)
key_before = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
# Simulate what the provider does internally
_remove_additional_properties(tools)
key_after = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
assert key_before == key_after, (
f"Cache key changed after _remove_additional_properties: "
f"{key_before!r} != {key_after!r} — disk cache is broken (issue #18784)"
)
# ---------------------------------------------------------------------------
# Edge cases
# ---------------------------------------------------------------------------
class TestEdgeCases:
"""Edge cases for the deep-copy fix."""
def test_nested_strict_in_parameters(self):
"""strict can appear at multiple nesting levels."""
schema = {
"strict": True,
"properties": {
"inner": {
"strict": True,
"type": "object",
}
},
}
original = copy.deepcopy(schema)
result = _remove_strict_from_schema(schema)
# Original preserved
assert schema == original
# Result cleaned
assert "strict" not in result
assert "strict" not in result["properties"]["inner"]
def test_empty_schema(self):
assert _remove_strict_from_schema({}) == {}
assert _remove_additional_properties({}) == {}
def test_none_passthrough(self):
"""Non-dict/list inputs are returned as-is."""
assert _remove_strict_from_schema(None) is None
assert _remove_strict_from_schema("hello") == "hello"
assert _remove_strict_from_schema(42) == 42
def test_list_of_tools(self):
"""The top-level input is often a list of tool dicts."""
tools = _make_tools_with_strict()
original = copy.deepcopy(tools)
result = _remove_strict_from_schema(tools)
assert tools == original
assert "strict" not in result[0]["function"]
def test_deeply_nested_additional_properties(self):
"""additionalProperties can appear at multiple nesting levels."""
schema = {
"type": "object",
"additionalProperties": False,
"properties": {
"outer": {
"type": "object",
"additionalProperties": False,
"properties": {
"inner": {
"type": "object",
"additionalProperties": False,
}
},
}
},
}
original = copy.deepcopy(schema)
result = _remove_additional_properties(schema)
# Original preserved at all levels
assert schema == original
# Result cleaned at all levels
assert "additionalProperties" not in result
assert "additionalProperties" not in result["properties"]["outer"]
assert "additionalProperties" not in result["properties"]["outer"]["properties"]["inner"]
# ---------------------------------------------------------------------------
# Sequential calls: production pattern (hosted_vllm, watsonx call both)
# ---------------------------------------------------------------------------
class TestSequentialCalls:
"""In production, providers like hosted_vllm and watsonx call both
_remove_additional_properties and _remove_strict_from_schema in sequence
on the same tools list. The original must survive both calls."""
def test_original_preserved_after_both_removals(self):
tools = _make_tools_with_strict()
original = copy.deepcopy(tools)
cleaned = _remove_additional_properties(tools)
cleaned = _remove_strict_from_schema(cleaned)
assert tools == original, "Original tools mutated by sequential removal calls"
assert "additionalProperties" not in cleaned[0]["function"]["parameters"]
assert "strict" not in cleaned[0]["function"]
def test_cache_key_stable_after_sequential_calls(self):
"""Cache key must be identical before and after both functions run."""
tools = _make_tools_with_strict()
with tempfile.TemporaryDirectory() as cache_dir:
cache = Cache(type=LiteLLMCacheType.DISK, disk_cache_dir=cache_dir)
key_before = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
# Simulate the exact production call sequence
cleaned = _remove_additional_properties(tools)
cleaned = _remove_strict_from_schema(cleaned)
key_after = cache.get_cache_key(
model="hosted_vllm/test",
messages=[{"role": "user", "content": "test"}],
tools=tools,
)
assert key_before == key_after, (
f"Cache key changed after sequential removal calls: "
f"{key_before!r} != {key_after!r}"
)
# ---------------------------------------------------------------------------
# In-place helpers: direct coverage
# ---------------------------------------------------------------------------
class TestInPlaceHelpers:
"""Direct tests for the _in_place internal helpers.
These are called only on already-copied data, but we verify they
actually perform the mutations they promise."""
def test_remove_strict_in_place_deletes_strict(self):
schema = {"strict": True, "properties": {"a": {"strict": True}}}
_remove_strict_from_schema_in_place(schema)
assert "strict" not in schema
assert "strict" not in schema["properties"]["a"]
def test_remove_strict_in_place_on_list(self):
schemas = [{"strict": True}, {"strict": True, "nested": {"strict": True}}]
_remove_strict_from_schema_in_place(schemas)
assert "strict" not in schemas[0]
assert "strict" not in schemas[1]
assert "strict" not in schemas[1]["nested"]
def test_remove_additional_properties_in_place_deletes(self):
schema = {
"additionalProperties": False,
"properties": {"a": {"additionalProperties": False}},
}
_remove_additional_properties_in_place(schema)
assert "additionalProperties" not in schema
assert "additionalProperties" not in schema["properties"]["a"]
def test_remove_additional_properties_in_place_keeps_true(self):
"""Only additionalProperties: false is removed; true is kept."""
schema = {"additionalProperties": True}
_remove_additional_properties_in_place(schema)
assert schema["additionalProperties"] is True