diff --git a/pyproject.toml b/pyproject.toml index 749af47ab6..cb9990284b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,6 +30,7 @@ dependencies = [ # stderr (agronholm/anyio#816, fixed in 4.10). "anyio>=4.10; python_version >= '3.14'", "anyio>=4.9; python_version < '3.14'", + "griffe>=1.0.0", "httpx>=0.27.1,<1.0.0", "httpx-sse>=0.4", "pydantic>=2.12.0", diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index 4a76106371..39547a2344 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -1,6 +1,7 @@ import functools import inspect import json +import logging from collections.abc import Awaitable, Callable, Sequence from itertools import chain from types import GenericAlias @@ -9,6 +10,7 @@ import anyio import anyio.to_thread import pydantic_core +from griffe import Docstring, DocstringSectionKind, Parser from pydantic import BaseModel, ConfigDict, Field, PydanticUserError, WithJsonSchema, create_model from pydantic.fields import FieldInfo from pydantic.json_schema import GenerateJsonSchema, JsonSchemaWarningKind @@ -28,6 +30,55 @@ logger = get_logger(__name__) +# griffe emits its own logging when a docstring section is malformed (e.g. a +# documented parameter that isn't in the signature). That's noise for our use +# case - we only want whatever descriptions we can extract - so silence it. +logging.getLogger("griffe").setLevel(logging.ERROR) + + +def _extract_param_descriptions(func: Callable[..., Any]) -> dict[str, str]: + """Extract per-parameter descriptions from a function's docstring. + + Supports the Google, NumPy, and Sphinx docstring styles. The style is not + declared anywhere, so we parse with each supported parser and keep whichever + yields the most parameter descriptions. + + Returns a mapping of parameter name to description. Returns an empty mapping + if the function has no docstring or no documented parameters. + """ + doc = inspect.getdoc(func) + if not doc: + return {} + + best: dict[str, str] = {} + for parser in (Parser.google, Parser.numpy, Parser.sphinx): + try: + sections = Docstring(doc).parse(parser) + except Exception: # pragma: no cover - defensive: never fail tool registration + continue + descriptions: dict[str, str] = {} + for section in sections: + if section.kind is not DocstringSectionKind.parameters: + continue + for param in section.value: + if param.description: + descriptions[param.name] = param.description.strip() + if len(descriptions) > len(best): + best = descriptions + return best + + +def _param_has_description(annotation: Any) -> bool: + """Return True if an annotation already carries a Field description. + + This lets an explicit ``Annotated[T, Field(description=...)]`` take + precedence over a description parsed from the docstring. + """ + for meta in getattr(annotation, "__metadata__", ()): + if isinstance(meta, FieldInfo) and meta.description: + return True + return False + class StrictJsonSchema(GenerateJsonSchema): """A JSON schema generator that raises exceptions instead of emitting warnings. @@ -215,6 +266,7 @@ def func_metadata( # model_rebuild right before using it 🤷 raise InvalidSignature(f"Unable to evaluate type annotations for callable {func.__name__!r}") from e params = sig.parameters + param_descriptions = _extract_param_descriptions(func) dynamic_pydantic_model_params: dict[str, Any] = {} for param in params.values(): if param.name.startswith("_"): # pragma: no cover @@ -227,8 +279,17 @@ def func_metadata( field_kwargs: dict[str, Any] = {} field_metadata: list[Any] = [] + # Apply a description parsed from the docstring, unless the parameter already + # declares one via `Annotated[T, Field(description=...)]`, which takes precedence. + doc_description = param_descriptions.get(param.name) + if doc_description and not _param_has_description(param.annotation): + field_kwargs["description"] = doc_description + if param.annotation is inspect.Parameter.empty: - field_metadata.append(WithJsonSchema({"title": param.name, "type": "string"})) + json_schema: dict[str, Any] = {"title": param.name, "type": "string"} + if doc_description: + json_schema["description"] = doc_description + field_metadata.append(WithJsonSchema(json_schema)) # Check if the parameter name conflicts with BaseModel attributes # This is necessary because Pydantic warns about shadowing parent attributes if hasattr(BaseModel, field_name) and callable(getattr(BaseModel, field_name)): diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index c57d1ee9f0..6f278d75c3 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -1189,3 +1189,152 @@ def func_with_metadata() -> Annotated[int, Field(gt=1)]: ... # pragma: no branc assert meta.output_schema is not None assert meta.output_schema["properties"]["result"] == {"exclusiveMinimum": 1, "title": "Result", "type": "integer"} + + +def _props(meta: Any) -> dict[str, Any]: + """Return the JSON schema properties for a function's arguments.""" + return meta.arg_model.model_json_schema()["properties"] + + +def test_docstring_param_descriptions_google(): + """Parameter descriptions are extracted from a Google-style docstring.""" + + def add(a: int, b: int): # pragma: no cover + """Add two numbers. + + Args: + a: The first number to add. + b: The second number to add. + """ + return a + b + + props = _props(func_metadata(add)) + assert props["a"]["description"] == "The first number to add." + assert props["b"]["description"] == "The second number to add." + + +def test_docstring_param_descriptions_numpy(): + """Parameter descriptions are extracted from a NumPy-style docstring.""" + + def sub(a: int, b: int): # pragma: no cover + """Subtract two numbers. + + Parameters + ---------- + a : int + The minuend value. + b : int + The subtrahend value. + """ + return a - b + + props = _props(func_metadata(sub)) + assert props["a"]["description"] == "The minuend value." + assert props["b"]["description"] == "The subtrahend value." + + +def test_docstring_param_descriptions_sphinx(): + """Parameter descriptions are extracted from a Sphinx-style docstring.""" + + def mul(a: int, b: int): # pragma: no cover + """Multiply two numbers. + + :param a: The first factor. + :param b: The second factor. + """ + return a * b + + props = _props(func_metadata(mul)) + assert props["a"]["description"] == "The first factor." + assert props["b"]["description"] == "The second factor." + + +def test_docstring_param_description_does_not_override_explicit_field(): + """An explicit Field(description=...) takes precedence over the docstring.""" + + def func( # pragma: no cover + a: Annotated[int, Field(description="Explicit description for a.")], + b: int, + ): + """Do something. + + Args: + a: Docstring description for a (should be ignored). + b: Docstring description for b. + """ + return a + b + + props = _props(func_metadata(func)) + assert props["a"]["description"] == "Explicit description for a." + assert props["b"]["description"] == "Docstring description for b." + + +def test_docstring_param_descriptions_untyped_params(): + """Descriptions are applied to parameters without type annotations.""" + + def func(a, b): # pragma: no cover + """Do something. + + Args: + a: Description for untyped a. + b: Description for untyped b. + """ + ... + + props = _props(func_metadata(func)) + assert props["a"]["description"] == "Description for untyped a." + assert props["b"]["description"] == "Description for untyped b." + + +def test_no_docstring_yields_no_descriptions(): + """Functions without a docstring produce schemas without descriptions.""" + + def func(a: int, b: int): # pragma: no cover + return a + b + + props = _props(func_metadata(func)) + assert "description" not in props["a"] + assert "description" not in props["b"] + + +def test_docstring_without_params_section_is_safe(): + """A docstring lacking a parameters section doesn't add descriptions or error.""" + + def func(a: int): # pragma: no cover + """Just a summary line with no documented parameters.""" + return a + + props = _props(func_metadata(func)) + assert "description" not in props["a"] + + +def test_docstring_param_with_empty_description_is_skipped(): + """A documented parameter with no description text gets no description.""" + + def func(a: int, b: int): # pragma: no cover + """Do something. + + Args: + a: + b: Description for b. + """ + ... + + props = _props(func_metadata(func)) + assert "description" not in props["a"] + assert props["b"]["description"] == "Description for b." + + +def test_docstring_description_applied_with_non_field_metadata(): + """Docstring descriptions still apply when annotation metadata isn't a Field.""" + + def func(a: Annotated[int, "not a field"]): # pragma: no cover + """Do something. + + Args: + a: Description for a. + """ + ... + + props = _props(func_metadata(func)) + assert props["a"]["description"] == "Description for a." diff --git a/uv.lock b/uv.lock index b9755c382b..55a70c0db6 100644 --- a/uv.lock +++ b/uv.lock @@ -846,6 +846,7 @@ name = "mcp" source = { editable = "." } dependencies = [ { name = "anyio" }, + { name = "griffe" }, { name = "httpx" }, { name = "httpx-sse" }, { name = "jsonschema" }, @@ -903,6 +904,7 @@ docs = [ requires-dist = [ { name = "anyio", marker = "python_full_version < '3.14'", specifier = ">=4.9" }, { name = "anyio", marker = "python_full_version >= '3.14'", specifier = ">=4.10" }, + { name = "griffe", specifier = ">=1.0.0" }, { name = "httpx", specifier = ">=0.27.1,<1.0.0" }, { name = "httpx-sse", specifier = ">=0.4" }, { name = "jsonschema", specifier = ">=4.20.0" },