Skip to content

Commit ab30192

Browse files
committed
fix: mcp parameter fields are being dropped
1 parent 56fcc74 commit ab30192

2 files changed

Lines changed: 208 additions & 2 deletions

File tree

‎src/sap_cloud_sdk/agentgateway/converters.py‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,44 @@
2525
"object": dict,
2626
}
2727

28+
# JSON Schema keys that map directly to a Pydantic Field kwarg (same semantics,
29+
# but camelCase → snake_case where needed).
30+
_FIELD_KWARGS: dict[str, str] = {
31+
"title": "title",
32+
"description": "description",
33+
"examples": "examples",
34+
"deprecated": "deprecated",
35+
"pattern": "pattern",
36+
"minLength": "min_length",
37+
"maxLength": "max_length",
38+
"minimum": "ge",
39+
"maximum": "le",
40+
"exclusiveMinimum": "gt",
41+
"exclusiveMaximum": "lt",
42+
"multipleOf": "multiple_of",
43+
}
44+
45+
# Everything else the MCP builder can emit that has no native Pydantic Field kwarg.
46+
# These are passed through via json_schema_extra so the LLM still sees them.
47+
_EXTRA_KEYS: frozenset[str] = frozenset(
48+
{
49+
"enum",
50+
"default",
51+
"example",
52+
"const",
53+
"format",
54+
"contentEncoding",
55+
"uniqueItems",
56+
"items",
57+
"properties",
58+
"required",
59+
"additionalProperties",
60+
"oneOf",
61+
"anyOf",
62+
"allOf",
63+
}
64+
)
65+
2866

2967
def _resolve_type(json_type: Any) -> tuple[type, bool]:
3068
"""Return (python_type, is_nullable) from a JSON Schema ``type`` value.
@@ -119,10 +157,23 @@ async def run(**kwargs) -> str:
119157
v = properties[orig]
120158
py_type, type_nullable = _resolve_type(v.get("type"))
121159
optional = orig not in required
160+
161+
field_kwargs: dict[str, Any] = {}
162+
extra: dict[str, Any] = {}
163+
for key, value in v.items():
164+
if key in ("type",):
165+
continue
166+
if key in _FIELD_KWARGS:
167+
field_kwargs[_FIELD_KWARGS[key]] = value
168+
elif key in _EXTRA_KEYS:
169+
extra[key] = value
170+
if extra:
171+
field_kwargs["json_schema_extra"] = extra
172+
122173
if optional or type_nullable:
123-
fields[safe] = (py_type | None, Field(default=None))
174+
fields[safe] = (py_type | None, Field(default=None, **field_kwargs))
124175
else:
125-
fields[safe] = (py_type, ...)
176+
fields[safe] = (py_type, Field(..., **field_kwargs))
126177
args_schema = create_model(f"{mcp_tool.name}_args", **fields) if fields else None
127178

128179
return StructuredTool.from_function(

‎tests/agentgateway/unit/test_converters.py‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,161 @@ async def test_optional_underscore_param_restored_when_supplied(self):
316316
assert "Product" not in kwargs
317317

318318

319+
class TestMcpToolToLangchainFieldMetadata:
320+
"""JSON Schema property metadata is forwarded to Pydantic Field.
321+
322+
Fields with a native Pydantic equivalent go there directly; everything
323+
else is preserved via json_schema_extra so the LLM still sees them.
324+
"""
325+
326+
def _tool(self, properties: dict, required: list[str] | None = None) -> MCPTool:
327+
return MCPTool(
328+
name="meta_tool",
329+
server_name="server",
330+
description="desc",
331+
input_schema={
332+
"type": "object",
333+
"required": required or [],
334+
"properties": properties,
335+
},
336+
url="https://example.com/mcp",
337+
)
338+
339+
# --- native Field kwargs ---
340+
341+
def test_description_preserved_on_required_field(self):
342+
lc_tool = mcp_tool_to_langchain(
343+
self._tool({"s": {"type": "string", "description": "The status"}}, required=["s"]),
344+
AsyncMock(), lambda: "token",
345+
)
346+
assert _schema_fields(lc_tool)["s"].description == "The status"
347+
348+
def test_description_preserved_on_optional_field(self):
349+
lc_tool = mcp_tool_to_langchain(
350+
self._tool({"s": {"type": "string", "description": "The status"}}),
351+
AsyncMock(), lambda: "token",
352+
)
353+
assert _schema_fields(lc_tool)["s"].description == "The status"
354+
355+
def test_title_preserved(self):
356+
lc_tool = mcp_tool_to_langchain(
357+
self._tool({"n": {"type": "string", "title": "OriginalName"}}, required=["n"]),
358+
AsyncMock(), lambda: "token",
359+
)
360+
assert _schema_fields(lc_tool)["n"].title == "OriginalName"
361+
362+
def test_examples_preserved_as_native_field_kwarg(self):
363+
lc_tool = mcp_tool_to_langchain(
364+
self._tool({"n": {"type": "string", "examples": ["Alice", "Bob"]}}, required=["n"]),
365+
AsyncMock(), lambda: "token",
366+
)
367+
assert _schema_fields(lc_tool)["n"].examples == ["Alice", "Bob"]
368+
369+
def test_deprecated_preserved(self):
370+
lc_tool = mcp_tool_to_langchain(
371+
self._tool({"n": {"type": "string", "deprecated": True}}, required=["n"]),
372+
AsyncMock(), lambda: "token",
373+
)
374+
assert _schema_fields(lc_tool)["n"].deprecated is True
375+
376+
def test_pattern_preserved(self):
377+
lc_tool = mcp_tool_to_langchain(
378+
self._tool({"n": {"type": "string", "pattern": "^[a-z]+$"}}, required=["n"]),
379+
AsyncMock(), lambda: "token",
380+
)
381+
assert _schema_fields(lc_tool)["n"].metadata # pattern lives in metadata
382+
383+
def test_min_max_length_preserved(self):
384+
lc_tool = mcp_tool_to_langchain(
385+
self._tool({"n": {"type": "string", "minLength": 2, "maxLength": 50}}, required=["n"]),
386+
AsyncMock(), lambda: "token",
387+
)
388+
schema = lc_tool.args_schema.model_json_schema()
389+
assert schema["properties"]["n"]["minLength"] == 2
390+
assert schema["properties"]["n"]["maxLength"] == 50
391+
392+
def test_minimum_maximum_preserved(self):
393+
lc_tool = mcp_tool_to_langchain(
394+
self._tool({"v": {"type": "integer", "minimum": 1, "maximum": 100}}, required=["v"]),
395+
AsyncMock(), lambda: "token",
396+
)
397+
schema = lc_tool.args_schema.model_json_schema()
398+
assert schema["properties"]["v"]["minimum"] == 1
399+
assert schema["properties"]["v"]["maximum"] == 100
400+
401+
# --- json_schema_extra bucket ---
402+
403+
def test_enum_preserved_in_json_schema_extra(self):
404+
lc_tool = mcp_tool_to_langchain(
405+
self._tool({"c": {"type": "string", "enum": ["red", "green", "blue"]}}, required=["c"]),
406+
AsyncMock(), lambda: "token",
407+
)
408+
schema = lc_tool.args_schema.model_json_schema()
409+
assert schema["properties"]["c"]["enum"] == ["red", "green", "blue"]
410+
411+
def test_default_preserved_in_json_schema_extra(self):
412+
lc_tool = mcp_tool_to_langchain(
413+
self._tool({"c": {"type": "string", "default": "active"}}, required=["c"]),
414+
AsyncMock(), lambda: "token",
415+
)
416+
schema = lc_tool.args_schema.model_json_schema()
417+
assert schema["properties"]["c"]["default"] == "active"
418+
419+
def test_example_preserved_in_json_schema_extra(self):
420+
lc_tool = mcp_tool_to_langchain(
421+
self._tool({"c": {"type": "string", "example": "hello"}}, required=["c"]),
422+
AsyncMock(), lambda: "token",
423+
)
424+
schema = lc_tool.args_schema.model_json_schema()
425+
assert schema["properties"]["c"]["example"] == "hello"
426+
427+
def test_format_preserved_in_json_schema_extra(self):
428+
lc_tool = mcp_tool_to_langchain(
429+
self._tool({"ts": {"type": "string", "format": "date-time"}}, required=["ts"]),
430+
AsyncMock(), lambda: "token",
431+
)
432+
schema = lc_tool.args_schema.model_json_schema()
433+
assert schema["properties"]["ts"]["format"] == "date-time"
434+
435+
def test_const_preserved_in_json_schema_extra(self):
436+
lc_tool = mcp_tool_to_langchain(
437+
self._tool({"v": {"type": "string", "const": "fixed"}}, required=["v"]),
438+
AsyncMock(), lambda: "token",
439+
)
440+
schema = lc_tool.args_schema.model_json_schema()
441+
assert schema["properties"]["v"]["const"] == "fixed"
442+
443+
def test_multiple_extra_keys_coexist(self):
444+
lc_tool = mcp_tool_to_langchain(
445+
self._tool(
446+
{"s": {"type": "string", "enum": ["a", "b"], "format": "uuid", "example": "a"}},
447+
required=["s"],
448+
),
449+
AsyncMock(), lambda: "token",
450+
)
451+
schema = lc_tool.args_schema.model_json_schema()
452+
assert schema["properties"]["s"]["enum"] == ["a", "b"]
453+
assert schema["properties"]["s"]["format"] == "uuid"
454+
assert schema["properties"]["s"]["example"] == "a"
455+
456+
def test_missing_metadata_produces_no_extra(self):
457+
lc_tool = mcp_tool_to_langchain(
458+
self._tool({"id": {"type": "string"}}, required=["id"]),
459+
AsyncMock(), lambda: "token",
460+
)
461+
field = _schema_fields(lc_tool)["id"]
462+
assert field.description is None
463+
assert field.json_schema_extra is None
464+
465+
def test_unknown_keys_are_silently_ignored(self):
466+
"""Keys not in either bucket (e.g. future JSON Schema extensions) must not raise."""
467+
lc_tool = mcp_tool_to_langchain(
468+
self._tool({"x": {"type": "string", "x-custom-ext": "value"}}, required=["x"]),
469+
AsyncMock(), lambda: "token",
470+
)
471+
assert "x" in _schema_fields(lc_tool)
472+
473+
319474
class TestMcpToolToLangchainInvocation:
320475
"""End-to-end invocation tests: verify what actually reaches call_tool."""
321476

0 commit comments

Comments
 (0)