Skip to content

Commit 52d86de

Browse files
author
SpiliosDmk
committed
perf(client): cache compiled jsonschema validator per tool output schema
validate_tool_result() called jsonschema.validate() on every tools/call, which rebuilds the validator class, re-checks the schema, and re-crawls its \ each time — even though a tool's output schema doesn't change between calls. Cache the compiled validator per tool name instead, and invalidate the cache when list_tools() reports a changed schema for that tool. Error message selection (best_match) is unchanged. Fixes #3133
1 parent 837ef90 commit 52d86de

2 files changed

Lines changed: 89 additions & 8 deletions

File tree

src/mcp/client/session.py

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from functools import reduce
77
from operator import or_
88
from types import TracebackType
9-
from typing import Annotated, Any, Final, Literal, Protocol, cast, overload
9+
from typing import TYPE_CHECKING, Annotated, Any, Final, Literal, Protocol, cast, overload
1010

1111
import anyio
1212
import anyio.abc
@@ -57,6 +57,9 @@
5757
from mcp.shared.subscriptions import SUBSCRIPTION_ID_META_KEY, event_from_wire
5858
from mcp.shared.transport_context import TransportContext
5959

60+
if TYPE_CHECKING:
61+
from jsonschema.protocols import Validator
62+
6063
DEFAULT_CLIENT_INFO = types.Implementation(name="mcp", version="0.1.0")
6164
DISCOVER_TIMEOUT_SECONDS = 10.0
6265
_NOTIFICATION_QUEUE_SIZE: Final = 256
@@ -373,6 +376,7 @@ def __init__(
373376
self._logging_callback = logging_callback or _default_logging_callback
374377
self._message_handler = message_handler or _default_message_handler
375378
self._tool_output_schemas: dict[str, dict[str, Any] | None] = {}
379+
self._tool_output_validators: dict[str, Validator] = {}
376380
self._x_mcp_header_maps: dict[str, dict[tuple[str, ...], str]] = {}
377381
self._initialize_result: types.InitializeResult | None = None
378382
self._discover_result: types.DiscoverResult | None = None
@@ -1057,16 +1061,42 @@ async def validate_tool_result(self, name: str, result: types.CallToolResult) ->
10571061
logger.warning(f"Tool {name} not listed by server, cannot validate any structured content")
10581062

10591063
if output_schema is not None:
1060-
from jsonschema import SchemaError, ValidationError, validate
1064+
# jsonschema's stub omits best_match's return type, so its import (and any
1065+
# unannotated use of its result) is reported as partially-unknown under strict
1066+
# pyright; cast() the result below rather than suppressing checks on this file.
1067+
from jsonschema.exceptions import (
1068+
SchemaError,
1069+
best_match, # pyright: ignore[reportUnknownVariableType]
1070+
)
1071+
from jsonschema.exceptions import ValidationError as JSONSchemaValidationError
1072+
from jsonschema.validators import validator_for
10611073

10621074
if result.structured_content is None:
10631075
raise RuntimeError(f"Tool {name} has an output schema but did not return structured content")
1064-
try:
1065-
validate(result.structured_content, output_schema)
1066-
except ValidationError as e:
1067-
raise RuntimeError(f"Invalid structured content returned by tool {name}: {e}")
1068-
except SchemaError as e: # pragma: no cover
1069-
raise RuntimeError(f"Invalid schema for tool {name}: {e}") # pragma: no cover
1076+
1077+
validator = self._tool_output_validators.get(name)
1078+
if validator is None:
1079+
# Build and cache the validator once per (tool, schema) instead of on every
1080+
# call: jsonschema.validate() re-derives the validator class, re-checks the
1081+
# schema, and re-crawls its $refs each time it's invoked, which is wasted
1082+
# work once we already know the schema is valid and unchanged.
1083+
validator_cls = validator_for(output_schema)
1084+
try:
1085+
validator_cls.check_schema(output_schema)
1086+
except SchemaError as e: # pragma: no cover
1087+
raise RuntimeError(f"Invalid schema for tool {name}: {e}") # pragma: no cover
1088+
# The Validator protocol's stub requires `registry`, but every concrete
1089+
# validator (as returned by validator_for) defaults it, same as plain
1090+
# jsonschema.validate() relies on.
1091+
validator = validator_cls(output_schema) # pyright: ignore[reportCallIssue]
1092+
self._tool_output_validators[name] = validator
1093+
1094+
# Mirror jsonschema.validate()'s error selection (best_match over all errors)
1095+
# rather than Validator.validate()'s "raise the first one", so error messages
1096+
# are unchanged from before this caching was introduced.
1097+
error = cast(JSONSchemaValidationError | None, best_match(validator.iter_errors(result.structured_content)))
1098+
if error is not None:
1099+
raise RuntimeError(f"Invalid structured content returned by tool {name}: {error}")
10701100

10711101
async def list_prompts(self, *, params: types.PaginatedRequestParams | None = None) -> types.ListPromptsResult:
10721102
"""Send a prompts/list request.
@@ -1197,6 +1227,10 @@ def _absorb_tool_listing(self, result: types.ListToolsResult, *, complete: bool)
11971227

11981228
# Cache tool output schemas for future validation; cursor pages only ever add.
11991229
for tool in result.tools:
1230+
if tool.name not in self._tool_output_schemas or self._tool_output_schemas[tool.name] != tool.output_schema:
1231+
# Schema is new or changed since we last saw this tool: drop any compiled
1232+
# validator built from the old schema so validate_tool_result recompiles it.
1233+
self._tool_output_validators.pop(tool.name, None)
12001234
self._tool_output_schemas[tool.name] = tool.output_schema
12011235

12021236
if complete:
@@ -1205,6 +1239,7 @@ def _absorb_tool_listing(self, result: types.ListToolsResult, *, complete: bool)
12051239
names = {tool.name for tool in result.tools}
12061240
self._x_mcp_header_maps = {k: v for k, v in self._x_mcp_header_maps.items() if k in names}
12071241
self._tool_output_schemas = {k: v for k, v in self._tool_output_schemas.items() if k in names}
1242+
self._tool_output_validators = {k: v for k, v in self._tool_output_validators.items() if k in names}
12081243

12091244
return result
12101245

tests/client/test_session_promotions.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,49 @@ async def test_validate_tool_result_raises_on_schema_mismatch() -> None:
6464
# Stable SDK prefix only: the message tail is jsonschema text that shifts with the dependency.
6565
with pytest.raises(RuntimeError, match="Invalid structured content returned by tool t"):
6666
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": "no"}))
67+
68+
69+
@pytest.mark.anyio
70+
async def test_validate_tool_result_reuses_cached_validator() -> None:
71+
"""The compiled jsonschema validator is built once per tool and reused, not rebuilt on every call."""
72+
server = _make_server({"type": "object", "properties": {"x": {"type": "integer"}}, "required": ["x"]})
73+
async with Client(server) as client:
74+
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": 1}))
75+
first = client.session._tool_output_validators["t"] # pyright: ignore[reportPrivateUsage]
76+
77+
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": 2}))
78+
second = client.session._tool_output_validators["t"] # pyright: ignore[reportPrivateUsage]
79+
80+
assert first is second
81+
82+
83+
@pytest.mark.anyio
84+
async def test_validate_tool_result_rebuilds_validator_when_schema_changes() -> None:
85+
"""A fresh `list_tools()` that reports a different output schema drops the stale cached validator."""
86+
schema_holder: dict[str, dict[str, object]] = {
87+
"t": {"type": "object", "properties": {"x": {"type": "integer"}}, "required": ["x"]},
88+
}
89+
90+
async def on_list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult:
91+
return ListToolsResult(
92+
tools=[Tool(name="t", input_schema={"type": "object"}, output_schema=schema_holder["t"])]
93+
)
94+
95+
server = Server("test-server", on_list_tools=on_list_tools)
96+
async with Client(server) as client:
97+
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": 1}))
98+
stale_validator = client.session._tool_output_validators["t"] # pyright: ignore[reportPrivateUsage]
99+
100+
# The tool's output schema now requires a string instead of an integer.
101+
schema_holder["t"] = {"type": "object", "properties": {"x": {"type": "string"}}, "required": ["x"]}
102+
await client.session.list_tools()
103+
assert "t" not in client.session._tool_output_validators # pyright: ignore[reportPrivateUsage]
104+
105+
# The old value fails the new schema; a value matching the new schema passes,
106+
# and rebuilds (rather than reuses) the cached validator.
107+
with pytest.raises(RuntimeError, match="Invalid structured content returned by tool t"):
108+
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": 1}))
109+
await client.session.validate_tool_result("t", CallToolResult(content=[], structured_content={"x": "ok"}))
110+
111+
fresh_validator = client.session._tool_output_validators["t"] # pyright: ignore[reportPrivateUsage]
112+
assert fresh_validator is not stale_validator

0 commit comments

Comments
 (0)