This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new e606cadff7c chore(mcp): fix malformed tool and prompt docstrings
(#44572)
e606cadff7c is described below
commit e606cadff7caf4c7a1134e62b3c13adbee26c459
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Sat Oct 3 02:49:19 2026 +1000
chore(mcp): fix malformed tool and prompt docstrings (#44572)
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../chart/prompts/create_chart_guided.py | 4 +-
superset/mcp_service/system/prompts/quickstart.py | 4 +-
superset/mcp_service/system/tool/get_schema.py | 7 +-
superset/mcp_service/system/tool/health_check.py | 5 +-
.../unit_tests/mcp_service/test_mcp_docstrings.py | 128 +++++++++++++++++++++
5 files changed, 138 insertions(+), 10 deletions(-)
diff --git a/superset/mcp_service/chart/prompts/create_chart_guided.py
b/superset/mcp_service/chart/prompts/create_chart_guided.py
index f1868fa7497..75cea4451fe 100644
--- a/superset/mcp_service/chart/prompts/create_chart_guided.py
+++ b/superset/mcp_service/chart/prompts/create_chart_guided.py
@@ -30,8 +30,8 @@ async def create_chart_guided_prompt(
Guided chart creation with step-by-step workflow.
Args:
- chart_type: Preferred chart type (auto, line, bar, table, scatter,
area)
- business_goal: Purpose (exploration, reporting, monitoring,
presentation)
+ chart_type (str): Preferred chart type (auto, line, bar, table,
scatter, area)
+ business_goal (str): Purpose (exploration, reporting, monitoring,
presentation)
"""
chart_intelligence = {
diff --git a/superset/mcp_service/system/prompts/quickstart.py
b/superset/mcp_service/system/prompts/quickstart.py
index 4427527029c..e691442e513 100644
--- a/superset/mcp_service/system/prompts/quickstart.py
+++ b/superset/mcp_service/system/prompts/quickstart.py
@@ -40,8 +40,8 @@ async def quickstart_prompt(
Guide new users through their first experience with the platform.
Args:
- user_type: Type of user (analyst, executive, developer)
- focus_area: Area of interest (sales, marketing, operations, general)
+ user_type (str): Type of user (analyst, executive, developer)
+ focus_area (str): Area of interest (sales, marketing, operations,
general)
"""
app_name = _get_app_name()
diff --git a/superset/mcp_service/system/tool/get_schema.py
b/superset/mcp_service/system/tool/get_schema.py
index d80b64478d8..3fa16810424 100644
--- a/superset/mcp_service/system/tool/get_schema.py
+++ b/superset/mcp_service/system/tool/get_schema.py
@@ -221,10 +221,13 @@ async def get_schema(
Column metadata is extracted dynamically from SQLAlchemy models.
Args:
- model_type: One of "chart", "dataset", "dashboard", "database", or
"report"
+ request (GetSchemaRequest): Request schema for unified get_schema
tool. Its
+ model_type is one of "chart", "dataset", "dashboard", "database" or
+ "report".
Returns:
- Comprehensive schema information for the requested model type
+ (GetSchemaResponse | PrivacyError): Comprehensive schema information
for
+ the requested model type.
"""
await ctx.info(f"Getting schema for model_type={request.model_type}")
diff --git a/superset/mcp_service/system/tool/health_check.py
b/superset/mcp_service/system/tool/health_check.py
index 35145038cd8..8868502909b 100644
--- a/superset/mcp_service/system/tool/health_check.py
+++ b/superset/mcp_service/system/tool/health_check.py
@@ -52,11 +52,8 @@ async def health_check() -> HealthCheckResponse:
Returns basic system information and confirms the service is running.
This is useful for testing connectivity and basic functionality.
- Parameters:
- None - This tool does not accept any parameters
-
Returns:
- HealthCheckResponse: Health status and system information including:
+ (HealthCheckResponse): Health status and system information including:
- status: "healthy" or "error"
- timestamp: ISO format timestamp
- service: Service name derived from APP_NAME config
diff --git a/tests/unit_tests/mcp_service/test_mcp_docstrings.py
b/tests/unit_tests/mcp_service/test_mcp_docstrings.py
new file mode 100644
index 00000000000..164bb95ea7d
--- /dev/null
+++ b/tests/unit_tests/mcp_service/test_mcp_docstrings.py
@@ -0,0 +1,128 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Guard against malformed MCP tool and prompt docstrings.
+
+FastMCP parses every registered tool and prompt docstring with griffe to
+derive the description and the per-argument descriptions it publishes to
+clients. A docstring griffe cannot parse makes griffe log a warning naming
+the offending line, and any entry naming something that is not a real
+parameter is dropped instead of reaching the published schema.
+
+FastMCP pins the griffe logger to ERROR when it is imported, so whether those
+warnings reach a given deployment's logs depends on that deployment's logging
+configuration. This module raises the level back to WARNING so the build fails
+on the malformed docstring itself rather than on whether anything happens to
+be listening.
+"""
+
+import asyncio
+import inspect
+import logging
+from collections.abc import Iterator
+from contextlib import contextmanager
+from typing import Any
+
+from fastmcp.utilities.docstring_parsing import parse_docstring
+
+from superset.mcp_service.app import mcp
+
+# Griffe emits its parsing diagnostics through a single logger of this name.
+GRIFFE_LOGGER = "griffe"
+
+
+@contextmanager
+def _captured_griffe_warnings() -> Iterator[list[str]]:
+ """Collect griffe warnings, bypassing any level FastMCP has imposed."""
+ messages: list[str] = []
+
+ class _Collector(logging.Handler):
+ def emit(self, record: logging.LogRecord) -> None:
+ messages.append(record.getMessage())
+
+ logger = logging.getLogger(GRIFFE_LOGGER)
+ handler = _Collector(level=logging.WARNING)
+ previous_level = logger.level
+ logger.setLevel(logging.WARNING)
+ logger.addHandler(handler)
+ try:
+ yield messages
+ finally:
+ logger.removeHandler(handler)
+ logger.setLevel(previous_level)
+
+
+def _registered_functions() -> list[tuple[str, Any]]:
+ """Pair every registered tool and prompt with its underlying function.
+
+ A list rather than a dict: ``list_tools()`` returns every version of a
+ tool without deduplicating, and a prompt may share a tool's name, so
+ keying by name alone would silently drop components from the guard.
+ """
+ tools = asyncio.run(mcp.list_tools())
+ prompts = asyncio.run(mcp.list_prompts())
+ functions = []
+ for kind, components in (("tool", tools), ("prompt", prompts)):
+ for component in components:
+ fn = getattr(component, "fn", None)
+ if fn is not None:
+ functions.append((f"{kind} {component.name}", fn))
+ return functions
+
+
+def test_registered_functions_are_discoverable() -> None:
+ """The guard below is only meaningful if it sees the real functions."""
+ labels = {label for label, _ in _registered_functions()}
+ assert "tool health_check" in labels
+ assert "tool get_schema" in labels
+ assert "prompt quickstart" in labels
+
+
+def test_mcp_docstrings_parse_without_griffe_warnings() -> None:
+ """No registered tool or prompt docstring may produce a griffe warning."""
+ offenders: dict[str, list[str]] = {}
+ for label, fn in _registered_functions():
+ with _captured_griffe_warnings() as messages:
+ parse_docstring(fn)
+ if messages:
+ offenders[label] = messages
+
+ assert not offenders, (
+ "Malformed MCP docstrings — griffe cannot parse these, so each one "
+ "logs a warning naming the offending line every time the module is "
+ "imported:\n"
+ + "\n".join(f" {label}: {msgs}" for label, msgs in
sorted(offenders.items()))
+ )
+
+
+def test_documented_arguments_reach_the_published_schema() -> None:
+ """Every documented argument must match a real parameter of the
function."""
+ mismatched: dict[str, list[str]] = {}
+ for label, fn in _registered_functions():
+ parsed = parse_docstring(fn)
+ if not parsed.parameters:
+ continue
+ signature_params = set(inspect.signature(fn).parameters)
+ unknown = sorted(set(parsed.parameters) - signature_params)
+ if unknown:
+ mismatched[label] = unknown
+
+ assert not mismatched, (
+ "Docstrings document arguments that do not exist on the function, so "
+ "their descriptions never reach the published schema:\n"
+ + "\n".join(f" {label}: {args}" for label, args in
sorted(mismatched.items()))
+ )
