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 549508b340b fix(mcp): preserve calling constraints in compact tool
discovery (#44656)
549508b340b is described below
commit 549508b340b2e411f8b8ba4c6172406f39706d7c
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Sat Oct 3 19:35:46 2026 +1000
fix(mcp): preserve calling constraints in compact tool discovery (#44656)
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
docs/admin_docs/configuration/mcp-server.mdx | 22 +-
superset/mcp_service/chart/schemas.py | 22 +-
superset/mcp_service/chart/tool/generate_chart.py | 18 +-
superset/mcp_service/chart/tool/get_chart_info.py | 16 +-
superset/mcp_service/chart/tool/list_charts.py | 14 +-
superset/mcp_service/chart/tool/update_chart.py | 16 +-
.../mcp_service/chart/tool/update_chart_preview.py | 18 +-
superset/mcp_service/dashboard/schemas.py | 63 +-
.../dashboard/tool/generate_dashboard.py | 18 +-
.../dashboard/tool/get_dashboard_info.py | 21 +-
.../mcp_service/dashboard/tool/list_dashboards.py | 18 +-
superset/mcp_service/dataset/schemas.py | 38 +-
.../dataset/tool/create_virtual_dataset.py | 16 +-
superset/mcp_service/dataset/tool/list_datasets.py | 18 +-
superset/mcp_service/mcp_config.py | 5 +-
superset/mcp_service/server.py | 112 +++-
.../test_tool_description_constraints.py | 655 +++++++++++++++++++++
.../unit_tests/mcp_service/test_tool_inventory.py | 2 +-
.../mcp_service/test_tool_search_transform.py | 82 ++-
19 files changed, 1081 insertions(+), 93 deletions(-)
diff --git a/docs/admin_docs/configuration/mcp-server.mdx
b/docs/admin_docs/configuration/mcp-server.mdx
index 1fd25f06a45..b0358d3d375 100644
--- a/docs/admin_docs/configuration/mcp-server.mdx
+++ b/docs/admin_docs/configuration/mcp-server.mdx
@@ -918,7 +918,27 @@ MCP_TOOL_SEARCH_CONFIG = {
| `always_visible` | See above | Tools that always appear in
`list_tools`, regardless of search
|
| `include_schemas` | `True` | Include the full `inputSchema` in
search results by default. When `False` ("summary mode"), search results omit
`inputSchema` entirely and include a lightweight `parameters_hint` listing
top-level parameter names. Full schemas are always used when a tool is actually
invoked via `call_tool`. |
| `compact_schemas` | `True` | Legacy setting selecting the default
description limit (300 when `True`, 0 when `False`) if `max_description_length`
is omitted. Input schemas preserve `$defs`, `$ref`, nullable unions, and
constraints in either mode; titles are omitted. Clients should resolve
references within each tool's `inputSchema`.
|
-| `max_description_length` | `300` | Truncate tool descriptions in search
results (0 = no truncation). Applies in both summary and full-schema modes.
|
+| `max_description_length` | `300` | Budget for discovery prose and
request-wrapper instructions (0 = no prose truncation). Complete paragraphs are
retained, followed by complete sentences of the next paragraph unless it is a
list, heading, or IMPORTANT block. Applies in both summary and full-schema
modes.
[...]
+
+The no-query catalog uses the same bounded serializer as targeted search.
+Chart creation/update and virtual-dataset tools keep request wrapping,
save/preview
+behavior, required URL display, and workflow guidance in request metadata
rather
+than relying on truncated introductory prose. Paragraph truncation never
leaves a
+partial IMPORTANT block or numbered workflow in the catalog.
+
+Calling constraints are authored as a `Field(description=...)` on the tool's
+`request` parameter and served as the `request` property's schema description,
so
+schema-first clients can identify the wrapper without remembering previous
calls.
+Summary mode preserves these instructions in `parameters_hint`. Instructions
are
+never truncated: their length is deducted from the prose budget first. A
request
+model's own docstring is not a calling instruction and is never deducted. If a
+configured limit is smaller than the instructions, prose is omitted rather than
+cutting the instructions. Built-in calling instructions are capped at 300
+characters by regression tests, and the complete tool entries retain per-tool
+size budgets. Field-level descriptions in the default full input schema also
+preserve tool-specific search semantics, sortable columns, chart pagination,
and
+layout guidance. These details are not included in schema-free summary mode.
+This setting does not change result-size limits or schema validation.
:::tip
Set `enabled: False` to revert to the traditional "show all tools at once"
behavior, which some clients or workflows may prefer.
diff --git a/superset/mcp_service/chart/schemas.py
b/superset/mcp_service/chart/schemas.py
index c411b481912..ef8b0322517 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -370,7 +370,7 @@ class GetChartInfoRequest(BaseModel):
form_data_key: str | None = Field(
default=None,
description=(
- "Cache key for retrieving unsaved chart state. When a user "
+ "Cache key from the Explore URL for unsaved chart state. When a
user "
"edits a chart in Explore but hasn't saved, the current state is
stored "
"with this key. If provided, the tool returns the current unsaved "
"configuration instead of the saved version. "
@@ -391,8 +391,9 @@ class GetChartInfoRequest(BaseModel):
default=None,
description=(
"When provided, resolves dashboard-level native filters that are
in "
- "scope for this chart on the given dashboard and returns them
under "
- "filters.dashboard_filters. Requires the chart to be on the
dashboard "
+ "scope for this chart on the given dashboard and returns their
column, "
+ "operator, and value under filters.dashboard_filters. Requires the
chart "
+ "to be on the dashboard "
"and the caller to have dashboard access."
),
)
@@ -413,7 +414,8 @@ class GetChartInfoRequest(BaseModel):
description=(
"Top-level fields to include in the response. Defaults to a
lean "
"set that excludes 'form_data' (the full chart config, can be
50KB+). "
- "Add 'form_data' explicitly when you need the raw chart
configuration."
+ "Add 'form_data' explicitly when you need the raw chart
configuration. "
+ "The url field links to the chart's Explore page in Superset."
),
validation_alias=AliasChoices("select_columns", "columns"),
),
@@ -3886,6 +3888,18 @@ class ListChartsRequest(
):
"""Request schema for list_charts with clear, unambiguous types."""
+ order_column: Annotated[
+ str | None,
+ Field(
+ default=None,
+ description=(
+ "Sortable columns: id, slice_name, viz_type, description, "
+ "changed_on, created_on; "
+ "changed_on_delta_humanized is an alias for changed_on."
+ ),
+ ),
+ ]
+
certified: Annotated[
StrictBool | None,
Field(
diff --git a/superset/mcp_service/chart/tool/generate_chart.py
b/superset/mcp_service/chart/tool/generate_chart.py
index 2e9948d5a46..78f803a1a8f 100644
--- a/superset/mcp_service/chart/tool/generate_chart.py
+++ b/superset/mcp_service/chart/tool/generate_chart.py
@@ -20,8 +20,10 @@ MCP tool: generate_chart (simplified schema)
import logging
import time
+from typing import Annotated
from fastmcp import Context
+from pydantic import Field
from sqlalchemy.exc import SQLAlchemyError
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -76,9 +78,21 @@ __all__ = ["CompileResult", "_compile_chart",
"validate_and_compile", "generate_
),
)
async def generate_chart( # noqa: C901
- request: GenerateChartRequest, ctx: Context
+ request: Annotated[
+ GenerateChartRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {...}}. '
+ "Preview only; save_chart=True saves. MUST display chart URL. "
+ "dataset_id: numeric ID/UUID, NOT schema.table_name. "
+ "config.chart_type required; line/bar/area/scatter are xy kind
values. "
+ "Check get_chart_type_schema for host-gated types."
+ )
+ ),
+ ],
+ ctx: Context,
) -> GenerateChartResponse:
- """Create a chart preview in Superset, optionally saving it permanently.
+ """Preview a chart; optionally save.
IMPORTANT BEHAVIOR:
- Charts are NOT saved by default (save_chart=False) - preview only
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py
b/superset/mcp_service/chart/tool/get_chart_info.py
index b2728a9b13f..d3503294378 100644
--- a/superset/mcp_service/chart/tool/get_chart_info.py
+++ b/superset/mcp_service/chart/tool/get_chart_info.py
@@ -20,10 +20,11 @@ MCP tool: get_chart_info
"""
import logging
-from typing import Any, cast
+from typing import Annotated, Any, cast
from fastmcp import Context
from marshmallow import ValidationError
+from pydantic import Field
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm import subqueryload
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -444,7 +445,18 @@ def _dump_chart_info(
),
)
async def get_chart_info( # noqa: C901
- request: GetChartInfoRequest, ctx: Context
+ request: Annotated[
+ GetChartInfoRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {"identifier": 123}}. '
+ "Use ID/UUID, NOT chart name; discover IDs with list_charts. "
+ "form_data_key reads unsaved state; permalink_key reads shared
"
+ "Explore state and makes identifier optional."
+ )
+ ),
+ ],
+ ctx: Context,
) -> ChartInfo | ChartError:
"""Get chart metadata by ID or UUID.
diff --git a/superset/mcp_service/chart/tool/list_charts.py
b/superset/mcp_service/chart/tool/list_charts.py
index 4a41df1c8d2..dabbb2637ef 100644
--- a/superset/mcp_service/chart/tool/list_charts.py
+++ b/superset/mcp_service/chart/tool/list_charts.py
@@ -20,9 +20,10 @@ MCP tool: list_charts (advanced filtering with metadata
cache control)
"""
import logging
-from typing import Any, cast, TYPE_CHECKING
+from typing import Annotated, Any, cast, TYPE_CHECKING
from fastmcp import Context
+from pydantic import Field
from sqlalchemy import case, select
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -179,7 +180,16 @@ class _ChartListCore(ModelListCore[ChartList]):
),
)
async def list_charts(
- request: ListChartsRequest | None = None,
+ request: Annotated[
+ ListChartsRequest | None,
+ Field(
+ description=(
+ 'Wrap parameters as {"request": {"search": "sales"}}; omit
request for defaults. '
+ "Do NOT pass search, page, page_size or filters as top-level
arguments. "
+ "For people, resolve IDs with find_users and use filters, not
search."
+ )
+ ),
+ ] = None,
ctx: Context = None,
) -> ChartList | ChartError:
"""List charts with filtering and search.
diff --git a/superset/mcp_service/chart/tool/update_chart.py
b/superset/mcp_service/chart/tool/update_chart.py
index 384f4445ae3..1161bead713 100644
--- a/superset/mcp_service/chart/tool/update_chart.py
+++ b/superset/mcp_service/chart/tool/update_chart.py
@@ -21,9 +21,10 @@ MCP tool: update_chart
import logging
import time
-from typing import Any
+from typing import Annotated, Any
from fastmcp import Context
+from pydantic import Field
from sqlalchemy.exc import SQLAlchemyError
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -705,7 +706,18 @@ def _create_preview_url(
),
)
async def update_chart( # noqa: C901
- request: UpdateChartRequest, ctx: Context
+ request: Annotated[
+ UpdateChartRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {...}}. '
+ "generate_preview=True previews; False persists immediately. "
+ "MUST display explore URL. identifier: ID/UUID, NOT chart
name. "
+ "Omit config to rename only; add_columns appends table
columns."
+ )
+ ),
+ ],
+ ctx: Context,
) -> GenerateChartResponse:
"""Update existing chart with new configuration.
diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py
b/superset/mcp_service/chart/tool/update_chart_preview.py
index 34eaf5b3026..57d316a9973 100644
--- a/superset/mcp_service/chart/tool/update_chart_preview.py
+++ b/superset/mcp_service/chart/tool/update_chart_preview.py
@@ -21,9 +21,10 @@ MCP tool: update_chart_preview
import logging
import time
-from typing import Any, Dict
+from typing import Annotated, Any, Dict
from fastmcp import Context
+from pydantic import Field
from sqlalchemy.exc import SQLAlchemyError
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -123,7 +124,20 @@ def _get_previous_form_data(form_data_key: str) ->
dict[str, Any] | None:
),
)
def update_chart_preview( # noqa: C901
- request: UpdateChartPreviewRequest, ctx: Context
+ request: Annotated[
+ UpdateChartPreviewRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {...}}. '
+ "Cached preview only, not saved. Supplied form_data_key "
+ "is invalidated; "
+ "use the returned key. MUST display explore_url. "
+ "For a fresh preview provide config + dataset_id "
+ "and omit form_data_key."
+ )
+ ),
+ ],
+ ctx: Context,
) -> UpdateChartPreviewResponse:
"""Update cached chart preview without saving.
diff --git a/superset/mcp_service/dashboard/schemas.py
b/superset/mcp_service/dashboard/schemas.py
index e6d25586db9..37e1c255e1e 100644
--- a/superset/mcp_service/dashboard/schemas.py
+++ b/superset/mcp_service/dashboard/schemas.py
@@ -204,6 +204,30 @@ class ListDashboardsRequest(
):
"""Request schema for list_dashboards with clear, unambiguous types."""
+ order_column: Annotated[
+ str | None,
+ Field(
+ default=None,
+ description=(
+ "Sortable columns: id, dashboard_title, slug, published, "
+ "changed_on, created_on; "
+ "changed_on_delta_humanized is an alias for changed_on."
+ ),
+ ),
+ ]
+
+ search: Annotated[
+ str | None,
+ Field(
+ default=None,
+ description=(
+ "Search matches titles and slugs only, not people. Resolve
names "
+ "with find_users and filter by created_by_fk or changed_by_fk "
+ "using the user ID. Mutually exclusive with 'filters'."
+ ),
+ ),
+ ]
+
deleted_state: Annotated[
Literal["include", "only"] | None,
Field(
@@ -260,10 +284,7 @@ DEFAULT_GET_DASHBOARD_INFO_COLUMNS: List[str] = [
class GetDashboardInfoRequest(MetadataCacheControl):
"""Request schema for dashboard identifiers and shared permalink URLs.
- When permalink_key is provided, the tool will retrieve the dashboard's
filter
- state from the permalink, allowing you to see what filters the user has
applied
- (not just the default filter state). This is useful when a user applies
filters
- in a dashboard but the URL contains a permalink_key.
+ permalink_key retrieves the user's applied filter state, not just defaults.
"""
model_config = ConfigDict(populate_by_name=True)
@@ -291,14 +312,16 @@ class GetDashboardInfoRequest(MetadataCacheControl):
filter_state: dict[str, Any] | None = Field(
default=None,
description=(
- "Active filters supplied directly rather than via a permalink, so
the "
- "tool can describe the dashboard as the user currently views it, "
- 'filtered. Accepts dashboard dataMask state, e.g. {"dataMask": '
+ "Direct active filters describing the user's dashboard view. "
+ 'Accepts dashboard dataMask state, e.g. {"dataMask": '
'{"<configured filter ID>": {"filterState": {"value":
["EMEA"]}}}}, '
'or {"applied_filters": [{"col": "region", "op": "IN", '
- '"val": ["EMEA"]}]}. Native mask values '
- "are projected without column metadata for restricted users.
Ignored "
- "when permalink_key is provided."
+ '"val": ["EMEA"]}]}. Ignored '
+ "when permalink_key is provided. Use returned filter_state as
context. "
+ "Restricted users receive native_filter_values (names, types,
values, "
+ "labels, exclusion flags), not raw dataMask/column targets. "
+ "native_filter_values_incomplete flags unsupported filters/chart
state "
+ "that cannot be summarized safely."
),
)
select_columns: Annotated[
@@ -306,11 +329,14 @@ class GetDashboardInfoRequest(MetadataCacheControl):
Field(
default_factory=lambda: list(DEFAULT_GET_DASHBOARD_INFO_COLUMNS),
description=(
- "Top-level fields to include in the response. Defaults to a
lean "
- "set that excludes 'css' (raw CSS, can be many KB) and
'filter_state' "
- "(only relevant when permalink_key is provided). Pass an
explicit list "
- "to override, e.g. ['id','dashboard_title','charts'] for
minimal "
- "output, or add 'css' to include raw dashboard CSS."
+ "Top-level response fields; defaults exclude 'css' (raw CSS, "
+ "potentially KBs) and 'filter_state' (shared/applied filter
context). "
+ "Override with "
+ "e.g. ['id','dashboard_title','charts'], or add 'css' for raw
CSS. "
+ "Charts/native_filters may be capped: check chart_count and "
+ "_truncation_notes. For all charts, call list_charts with "
+ 'request={"filters": [{"col": "dashboards", "opr": "eq", '
+ '"value": <dashboard id>}]} and paginate with page/page_size.'
),
validation_alias=AliasChoices("select_columns", "columns"),
),
@@ -704,8 +730,7 @@ class GenerateDashboardRequest(BaseModel):
dashboard_title: str | None = Field(
None,
description=(
- "Title for the new dashboard. When omitted a descriptive title "
- "is generated from the included chart names."
+ "Dashboard title; if omitted, generated descriptively from chart
names."
),
validation_alias=AliasChoices("dashboard_title", "title", "name"),
)
@@ -729,7 +754,9 @@ class GenerateDashboardRequest(BaseModel):
"dict). When set, replaces the auto-generated layout entirely. "
"Pass this when you need custom row composition, MARKDOWN "
"blocks, HEADER components, or specific chart widths/heights. "
- "Omit to let the tool auto-generate a packed grid from chart_ids."
+ "Omit for an auto-generated 2-column grid from chart_ids. "
+ "Each component's parents is recomputed from its children edges "
+ "before saving; omitted or incomplete parents arrays are fine."
),
)
json_metadata_overrides: Dict[str, Any] | None = Field(
diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py
b/superset/mcp_service/dashboard/tool/generate_dashboard.py
index 624a04d9268..21dfc20be37 100644
--- a/superset/mcp_service/dashboard/tool/generate_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py
@@ -22,11 +22,11 @@ This tool creates a new dashboard with specified charts and
layout configuration
"""
import logging
-from typing import Any, Dict, List
+from typing import Annotated, Any, Dict, List
from fastmcp import Context
from flask import g
-from pydantic import ValidationError
+from pydantic import Field, ValidationError
from sqlalchemy.exc import IntegrityError, SQLAlchemyError
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -198,7 +198,19 @@ def _generate_title_from_charts(chart_objects: List[Any])
-> str:
),
)
def generate_dashboard( # noqa: C901
- request: GenerateDashboardRequest, ctx: Context
+ request: Annotated[
+ GenerateDashboardRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {"chart_ids": [1], "dashboard_title":
"New"}}. '
+ "Charts must exist and be accessible. NEW dashboards only; "
+ "use add_chart_to_existing_dashboard for existing ones. "
+ "Never use as a fallback if that fails. "
+ "position_json overrides the default grid."
+ )
+ ),
+ ],
+ ctx: Context,
) -> GenerateDashboardResponse:
"""Create a NEW dashboard from chart IDs.
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_info.py
b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
index 1a03ebe8ff2..eeab62708d1 100644
--- a/superset/mcp_service/dashboard/tool/get_dashboard_info.py
+++ b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
@@ -24,9 +24,10 @@ about a specific dashboard.
import logging
from datetime import datetime, timezone
-from typing import cast
+from typing import Annotated, cast
from fastmcp import Context
+from pydantic import Field
from sqlalchemy.orm import subqueryload
from superset_core.mcp.decorators import tool, ToolAnnotations
@@ -104,10 +105,22 @@ def _lookup_dashboard(
),
)
async def get_dashboard_info(
- request: GetDashboardInfoRequest, ctx: Context
+ request: Annotated[
+ GetDashboardInfoRequest,
+ Field(
+ description=(
+ 'Wrap {"request": {...}}. Filtered? Use
permalink_key/filter_state: '
+ "snapshots, not query predicates; "
+ "heed scope/native_filter_values_incomplete. "
+ "Missing state != no filters. "
+ "Don't guess columns/query workspace-wide; clarify. "
+ "Charts: list_charts."
+ )
+ ),
+ ],
+ ctx: Context,
) -> DashboardInfo | DashboardError:
- """
- Get dashboard metadata by ID, UUID, slug, or dashboard permalink.
+ """Get dashboard info by ID, UUID, slug, or permalink.
Returns title, charts, and layout details.
diff --git a/superset/mcp_service/dashboard/tool/list_dashboards.py
b/superset/mcp_service/dashboard/tool/list_dashboards.py
index b9c2a2b31e9..50c4279e168 100644
--- a/superset/mcp_service/dashboard/tool/list_dashboards.py
+++ b/superset/mcp_service/dashboard/tool/list_dashboards.py
@@ -23,9 +23,10 @@ advanced filtering with clear, unambiguous request schema
and metadata cache con
"""
import logging
-from typing import TYPE_CHECKING
+from typing import Annotated, TYPE_CHECKING
from fastmcp import Context
+from pydantic import Field
from superset_core.mcp.decorators import tool, ToolAnnotations
if TYPE_CHECKING:
@@ -70,10 +71,21 @@ _DEFAULT_LIST_DASHBOARDS_REQUEST = ListDashboardsRequest()
),
)
async def list_dashboards(
- request: ListDashboardsRequest | None = None,
+ request: Annotated[
+ ListDashboardsRequest | None,
+ Field(
+ description=(
+ 'Wrap parameters as {"request": {"search": "sales"}}; omit
request for defaults. '
+ "Do NOT pass search, page, page_size or filters as top-level
arguments. "
+ "For people, resolve IDs with find_users and use filters, not
search."
+ )
+ ),
+ ] = None,
ctx: Context = None,
) -> DashboardList:
- """List dashboards with filtering and search. Returns dashboard metadata
+ """List dashboards with filtering and search.
+
+ Returns dashboard metadata
including title, slug, URL, and last modified time. Use select_columns to
request additional fields.
diff --git a/superset/mcp_service/dataset/schemas.py
b/superset/mcp_service/dataset/schemas.py
index 12ec0bdcdb0..b56f88f5d16 100644
--- a/superset/mcp_service/dataset/schemas.py
+++ b/superset/mcp_service/dataset/schemas.py
@@ -88,10 +88,9 @@ class DatasetFilter(ColumnOperator):
"changed_by_fk",
] = Field(
...,
- description="Column to filter on. Use get_schema(model_type='dataset')
for "
- "available filter columns. To filter by a person, first call
find_users "
- "to resolve a name to a user ID, then filter by created_by_fk or "
- "changed_by_fk with that integer ID.",
+ description="Filter column; see get_schema(model_type='dataset'). "
+ "For people, resolve names to IDs with find_users; filter by "
+ "created_by_fk or changed_by_fk with that integer ID.",
)
opr: ColumnOperatorEnum = Field(
...,
@@ -284,15 +283,38 @@ class ListDatasetsRequest(
test_list_datasets_with_string_filters.
"""
+ order_column: Annotated[
+ str | None,
+ Field(
+ default=None,
+ description=(
+ "Sortable columns: id, table_name, schema, changed_on,
created_on; "
+ "changed_on_delta_humanized is an alias for changed_on."
+ ),
+ ),
+ ]
+
+ search: Annotated[
+ str | None,
+ Field(
+ default=None,
+ description=(
+ "Case-insensitive substring search of schema, SQL, table name,
"
+ "and description. A complete UUID is an exact UUID lookup; "
+ "or use a uuid filter. Compare candidate descriptions "
+ "and metadata. Mutually exclusive with 'filters'."
+ ),
+ ),
+ ]
+
certified: Annotated[
StrictBool | None,
Field(
default=None,
description=(
- "Filter by governance certification status. Use true to return
"
- "only certified datasets (preferred when selecting governed "
- "semantic-layer assets), false to return only uncertified "
- "datasets, or omit to return both (default)."
+ "Use true to return only certified datasets (preferred for
governed "
+ "semantic-layer assets), false to return only uncertified
datasets; "
+ "omit to return both (default)."
),
),
]
diff --git a/superset/mcp_service/dataset/tool/create_virtual_dataset.py
b/superset/mcp_service/dataset/tool/create_virtual_dataset.py
index 31eb795a456..ea0d24b4723 100644
--- a/superset/mcp_service/dataset/tool/create_virtual_dataset.py
+++ b/superset/mcp_service/dataset/tool/create_virtual_dataset.py
@@ -16,9 +16,10 @@
# under the License.
import logging
-from typing import Any
+from typing import Annotated, Any
from fastmcp import Context
+from pydantic import Field
from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.exceptions import SupersetGenericDBErrorException
@@ -96,7 +97,18 @@ def _update_virtual_dataset(dataset_id: int, update_props:
dict[str, Any]) -> An
),
)
async def create_virtual_dataset( # noqa: C901
- request: CreateVirtualDatasetRequest, ctx: Context
+ request: Annotated[
+ CreateVirtualDatasetRequest,
+ Field(
+ description=(
+ 'Wrap as {"request": {...}}. '
+ "Provide SQL and a dataset name. Use returned id as dataset_id
in "
+ "generate_chart or generate_explore_link; "
+ "pick chart columns from returned columns."
+ )
+ ),
+ ],
+ ctx: Context,
) -> CreateVirtualDatasetResponse:
"""Save a SQL query as a virtual dataset so it can be charted.
diff --git a/superset/mcp_service/dataset/tool/list_datasets.py
b/superset/mcp_service/dataset/tool/list_datasets.py
index 9bf9a42dd56..d29e1a42c3a 100644
--- a/superset/mcp_service/dataset/tool/list_datasets.py
+++ b/superset/mcp_service/dataset/tool/list_datasets.py
@@ -23,10 +23,11 @@ advanced filtering with clear, unambiguous request schema
and metadata cache con
"""
import logging
-from typing import TYPE_CHECKING
+from typing import Annotated, TYPE_CHECKING
from uuid import UUID
from fastmcp import Context
+from pydantic import Field
from superset_core.mcp.decorators import tool, ToolAnnotations
if TYPE_CHECKING:
@@ -83,10 +84,21 @@ _DEFAULT_LIST_DATASETS_REQUEST = ListDatasetsRequest()
)
@requires_data_model_metadata_access
async def list_datasets(
- request: ListDatasetsRequest | None = None,
+ request: Annotated[
+ ListDatasetsRequest | None,
+ Field(
+ description=(
+ 'Wrap {"request": {...}}; Do NOT pass search/page/filters
top-level. '
+ "Candidates, not a ranking: several fit? explain alternatives,
"
+ "clarify before querying; empty result doesn't prove absence. "
+ "Never substitute out-of-scope data. "
+ "Users: find_users ID filter, not search."
+ )
+ ),
+ ] = None,
ctx: Context | None = None,
) -> DatasetList | DatasetError:
- """List datasets with filtering and search.
+ """List/search/filter datasets.
Returns dataset metadata including table name, schema, and last modified
time. Set ``request.certified`` to true to return only governed,
diff --git a/superset/mcp_service/mcp_config.py
b/superset/mcp_service/mcp_config.py
index f352939c2a3..e69b2fb64c0 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -500,6 +500,9 @@ MCP_RESPONSE_SIZE_CONFIG: dict[str, Any] = {
# each tool's inputSchema. Inlining references duplicates shared chart models.
# The legacy compact_schemas setting only selects the default description limit
# (300 when True, 0 when False) if max_description_length is omitted.
+# Field descriptions on a tool's request parameter carry untruncated calling
+# instructions. Their length is deducted from the prose budget; small limits
+# omit prose instead. Request-model docstrings are not deducted.
#
# Rollback:
# ---------
@@ -510,7 +513,7 @@ MCP_RESPONSE_SIZE_CONFIG: dict[str, Any] = {
# --------------------------------
# When include_schemas=False, search results omit inputSchema entirely and
# include a lightweight "parameters_hint" field listing top-level parameter
-# names (e.g. "page, page_size, search, filters"). This reduces per-search
+# names and any request-wrapper instructions. This reduces per-search
# token cost by ~80% vs compact mode while still conveying what parameters
# a tool accepts. Full schemas remain available when invoking the tool via
# call_tool.
diff --git a/superset/mcp_service/server.py b/superset/mcp_service/server.py
index 328f246a140..e37df646707 100644
--- a/superset/mcp_service/server.py
+++ b/superset/mcp_service/server.py
@@ -25,12 +25,14 @@ For multi-pod deployments, configure MCP_EVENT_STORE_CONFIG
with Redis URL.
import inspect
import logging
import os
+import re
from collections.abc import Sequence
from typing import Annotated, Any, Callable
import uvicorn
from fastmcp.exceptions import ToolError
from fastmcp.server.middleware import Middleware
+from pydantic.fields import FieldInfo
from starlette.requests import ClientDisconnect
from superset.mcp_service.app import create_mcp_app, init_fastmcp_server
@@ -301,30 +303,80 @@ def _strip_titles(obj: Any, in_properties_map: bool =
False) -> Any:
return obj
+_PARAGRAPH_BREAK = re.compile(r"\n\s*\n")
+# A list item, an IMPORTANT marker, or a first line ending in a colon
(heading).
+_STRUCTURED_PARAGRAPH = re.compile(
+ r"^\s*(?:[-*+]|\d+[.)])\s|^\W*IMPORTANT\b|\A[^\n]*:[ \t]*$", re.MULTILINE
+)
+
+
+def _complete_sentences(text: str, max_length: int) -> str:
+ """Return the longest prefix of *text* made of complete sentences."""
+ if max_length <= 0:
+ return ""
+ # Look one character past the budget so a boundary exactly at the limit
counts.
+ boundaries = re.finditer(r"[.!?](?=\s|$)", text[: max_length + 1])
+ ends = [match.end() for match in boundaries if match.end() <= max_length]
+ return text[: ends[-1]].strip() if ends else ""
+
+
+def _drop_trailing_lead_in(text: str) -> str:
+ """Drop a final sentence ending in a colon, which introduces a cut-off
list."""
+ if not text.endswith(":"):
+ return text
+ boundaries = list(re.finditer(r"[.!?](?=\s)", text))
+ return text[: boundaries[-1].end()].strip() if boundaries else text
+
+
def _truncate_description(text: str, max_length: int) -> str:
- """Truncate a tool description for search results.
-
- Cuts at the last sentence boundary before *max_length*, or at
- *max_length* with an ellipsis if no sentence boundary is found.
-
- Dedents first: Python 3.13 has the compiler strip a docstring's common
- leading whitespace at compile time (``__doc__`` comes out already
- cleaned), while 3.11/3.12 store it raw and leave that to the caller. A
- multi-line tool docstring's raw, un-dedented form is longer per line, so
- the same character budget lands at a different point in the text
- depending on which Python compiled it. Cleaning here first makes the cut
- point (and this function's callers' byte budgets) consistent regardless
- of interpreter version.
+ """Keep whole paragraphs, then whole sentences of the next one, within
budget.
+
+ Clean docstring indentation before applying the budget so the cut point
+ is consistent across Python versions that store docstrings differently.
"""
+ if max_length <= 0:
+ return ""
text = inspect.cleandoc(text) if text else text
if not text or len(text) <= max_length:
return text
- # Try to cut at the last sentence boundary
- truncated = text[:max_length]
- last_period = truncated.rfind(". ")
- if last_period > max_length // 2:
- return truncated[: last_period + 1]
- return truncated.rstrip() + "..."
+ kept, rest = "", text
+ for match in _PARAGRAPH_BREAK.finditer(text):
+ if match.start() > max_length:
+ break
+ kept, rest = text[: match.start()].strip(), text[match.end() :]
+ kept = _drop_trailing_lead_in(kept)
+ following = _PARAGRAPH_BREAK.split(rest, maxsplit=1)[0]
+ # Do not leave a heading, IMPORTANT block or list workflow partly
advertised.
+ if kept and _STRUCTURED_PARAGRAPH.search(following):
+ return kept
+ separator = "\n\n" if kept else ""
+ extra = _complete_sentences(following, max_length - len(kept) -
len(separator))
+ return f"{kept}{separator}{extra}" if extra else kept
+
+
+def _request_instructions(tool: Any) -> str:
+ """Return calling instructions authored on the tool's ``request``
parameter.
+
+ Only ``Field(description=...)`` on the parameter itself counts. Schema
+ dereferencing also copies the request model's docstring onto the served
+ ``request`` property; that is model documentation, not calling
instructions,
+ and must not be advertised or deducted from the prose budget.
+ """
+ try:
+ signature = inspect.signature(tool.fn)
+ except (AttributeError, TypeError, ValueError):
+ return ""
+ if (parameter := signature.parameters.get("request")) is None:
+ return ""
+ fields = (*getattr(parameter.annotation, "__metadata__", ()),
parameter.default)
+ return next(
+ (
+ field.description
+ for field in fields
+ if isinstance(field, FieldInfo) and field.description
+ ),
+ "",
+ )
def _extract_parameter_names(input_schema: dict[str, Any]) -> str:
@@ -367,7 +419,8 @@ def _build_summary_serializer(max_desc: int) -> Any:
Returns a callable that serializes each tool to ``name``,
``description`` (optionally truncated), and a ``parameters_hint``
- string listing top-level parameter names. ``inputSchema`` and
+ string listing top-level parameter names and unabridged request
instructions.
+ Instruction length is reserved from the prose budget. ``inputSchema`` and
``outputSchema`` are stripped entirely.
"""
@@ -378,12 +431,17 @@ def _build_summary_serializer(max_desc: int) -> Any:
mode="json", exclude_none=True, exclude={"outputSchema"}
)
data.pop("outputSchema", None)
+ instructions = _request_instructions(tool)
if input_schema := data.pop("inputSchema", None):
hint = _extract_parameter_names(input_schema)
if hint:
- data["parameters_hint"] = hint
+ data["parameters_hint"] = (
+ f"{hint}: {instructions}" if instructions else hint
+ )
if max_desc and (desc := data.get("description")):
- data["description"] = _truncate_description(desc, max_desc)
+ data["description"] = _truncate_description(
+ desc, max(0, max_desc - len(instructions))
+ )
results.append(data)
return results
@@ -470,7 +528,8 @@ def _create_search_result_serializer(
Titles and output schemas are stripped by the base serializer. The legacy
``compact_schemas`` setting only selects the default description limit;
- ``max_description_length`` explicitly controls description truncation.
+ ``max_description_length`` budgets prose plus request-wrapper instructions.
+ Instructions stay in the schema even when they exceed a small prose limit.
"""
include_schemas = config.get("include_schemas", False)
@@ -488,9 +547,12 @@ def _create_search_result_serializer(
def _serializer(tools: Sequence[Any]) -> list[dict[str, Any]]:
results = _serialize_tools_without_output_schema(tools)
- for data in results:
+ for tool, data in zip(tools, results, strict=True):
if desc := data.get("description"):
- data["description"] = _truncate_description(desc, max_desc)
+ instructions = _request_instructions(tool)
+ data["description"] = _truncate_description(
+ desc, max(0, max_desc - len(instructions))
+ )
return results
return _serializer
diff --git a/tests/unit_tests/mcp_service/test_tool_description_constraints.py
b/tests/unit_tests/mcp_service/test_tool_description_constraints.py
new file mode 100644
index 00000000000..b5889f40cd1
--- /dev/null
+++ b/tests/unit_tests/mcp_service/test_tool_description_constraints.py
@@ -0,0 +1,655 @@
+# 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.
+
+"""Calling guidance must survive discovery without relying on truncated
prose."""
+
+import inspect
+import re
+
+import pytest
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.mcp_config import MCP_TOOL_SEARCH_CONFIG
+from superset.mcp_service.server import (
+ _create_search_result_serializer,
+ _request_instructions,
+ _truncate_description,
+)
+
+CONSTRAINTS = {
+ "list_dashboards": ("Do NOT pass", "top-level", '{"request": {'),
+ "list_charts": ("Do NOT pass", "top-level", '{"request": {'),
+ "list_datasets": (
+ "Do NOT pass",
+ "search/page/filters",
+ "top-level",
+ "out-of-scope",
+ '{"request": {',
+ ),
+ "get_chart_info": (
+ "NOT chart name",
+ "form_data_key",
+ "permalink_key",
+ '{"request": {',
+ ),
+ "get_dashboard_info": (
+ "filter_state",
+ "permalink_key",
+ "snapshots, not query predicates",
+ '{"request": {',
+ ),
+ "generate_dashboard": ("Never use as a fallback", "accessible",
'{"request": {'),
+}
+
+
[email protected]
[email protected]("name", CONSTRAINTS)
[email protected]("include_schemas", [True, False])
[email protected]("max_desc", [1, 20, 300, 0])
+async def test_registered_tool_calling_constraints(
+ name: str, include_schemas: bool, max_desc: int
+) -> None:
+ """Exercise real registrations, including small limits and summary
discovery."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ entry = _create_search_result_serializer(
+ {"include_schemas": include_schemas, "max_description_length":
max_desc}
+ )([tool])[0]
+ if include_schemas:
+ guidance =
entry["inputSchema"]["properties"]["request"].get("description", "")
+ schema_text = _schema_text(entry["inputSchema"])
+ for _, phrases in SCHEMA_DOCSTRING_CONSTRAINTS[name]:
+ assert all(phrase in schema_text for phrase in phrases), (name,
phrases)
+ else:
+ guidance = str(entry.get("parameters_hint", ""))
+ for constraint in CONSTRAINTS[name]:
+ assert constraint in guidance, (name, constraint, entry["description"])
+ if max_desc:
+ assert len(entry["description"]) <= max_desc
+
+
[email protected]
[email protected]("name", CONSTRAINTS)
+async def test_registered_constraints_have_bounded_size(name: str) -> None:
+ """Critical metadata has its own fixed ceiling, not an unbounded IMPORTANT
block."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ instructions = tool.parameters["properties"]["request"]["description"]
+ assert len(instructions) <= 300
+ entry = _create_search_result_serializer({"include_schemas":
True})([tool])[0]
+ assert len(instructions) + len(entry["description"]) <= 300
+
+
[email protected]
[email protected]("name", CONSTRAINTS)
[email protected]("include_schemas", [True, False])
+async def test_oversized_registered_description_keeps_calling_constraints(
+ name: str, include_schemas: bool
+) -> None:
+ """Long introductory prose cannot displace metadata or exhaust its
budget."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ oversized = tool.model_copy(
+ update={"description": "Long introduction. " * 10_000 +
tool.description}
+ )
+ entry = _create_search_result_serializer({"include_schemas":
include_schemas})(
+ [oversized]
+ )[0]
+ instructions = tool.parameters["properties"]["request"]["description"]
+ if include_schemas:
+ assert entry["inputSchema"]["properties"]["request"]["description"] ==
(
+ instructions
+ )
+ schema_text = _schema_text(entry["inputSchema"])
+ for _, phrases in SCHEMA_DOCSTRING_CONSTRAINTS[name]:
+ assert all(phrase in schema_text for phrase in phrases), (name,
phrases)
+ else:
+ assert instructions in entry["parameters_hint"]
+ assert len(entry["description"]) + len(instructions) <= 300
+
+
[email protected]
+async def test_direct_inventory_keeps_bounded_calling_metadata() -> None:
+ """Served tools/list carries bounded instructions, measured after
middleware.
+
+ The list_tools middleware inlines each request model, copying its docstring
+ onto ``properties.request``. Only authored request-parameter instructions
+ count toward the 300-character cap and the prose deduction.
+ """
+ for tool in await mcp.list_tools(run_middleware=True):
+ schema = tool.to_mcp_tool().inputSchema
+ served = schema.get("properties", {}).get("request", {})
+ instructions = _request_instructions(tool)
+ assert len(instructions) <= 300, tool.name
+ if instructions:
+ assert served["description"] == instructions, tool.name
+ if tool.name in CONSTRAINTS:
+ assert all(part in instructions for part in CONSTRAINTS[tool.name])
+
+
+# Each docstring constraint sentence, paired with the phrases that must carry
it
+# through default discovery. Wrapper boilerplate alone does not satisfy the
audit.
+SUMMARY_DOCSTRING_CONSTRAINTS: dict[str, list[tuple[str, tuple[str, ...]]]] = {
+ "list_datasets": [
+ ("must be wrapped in a ``request`` object", ('{"request": {',)),
+ ("Do NOT pass ``search``, ``page``", ("Do NOT pass", "page",
"top-level")),
+ ("Results are candidates, not a relevance ranking", ("not a
ranking",)),
+ (
+ "when multiple candidates fit, explain the alternatives and
clarify "
+ "before querying",
+ ("explain alternatives", "clarify before querying"),
+ ),
+ (
+ "An empty search result does not establish that the requested data
"
+ "does not exist",
+ ("empty result", "prove absence"),
+ ),
+ (
+ "Never substitute a different dataset for one outside the MCP
scope",
+ ("Never substitute", "out-of-scope"),
+ ),
+ ("call find_users to resolve the name to a user ID", ("find_users",)),
+ ("Do not pass the name as search", ("not search",)),
+ ],
+ "list_charts": [
+ ("must be wrapped in a ``request`` object", ('{"request": {',)),
+ ("Do NOT pass ``search``", ("Do NOT pass", "top-level")),
+ ("call find_users to resolve the name to a user ID", ("find_users",)),
+ ("Do not pass the name as search", ("not search",)),
+ ],
+ "list_dashboards": [
+ ("must be wrapped in a ``request`` object", ('{"request": {',)),
+ ("Do NOT pass ``search``", ("Do NOT pass", "top-level")),
+ ("call find_users to resolve the name to a user ID", ("find_users",)),
+ ("do NOT pass the name as the search parameter", ("not search",)),
+ ],
+ "get_chart_info": [
+ ("Use numeric ID or UUID string (NOT chart name)", ("NOT chart
name",)),
+ ("To find a chart ID, use the list_charts tool first",
("list_charts",)),
+ ("When form_data_key is provided", ("form_data_key", "unsaved")),
+ ("so identifier is optional", ("permalink_key", "identifier
optional")),
+ ],
+ "get_dashboard_info": [
+ ("supply its permalink_key or filter_state", ("permalink_key",
"filter_state")),
+ ("Check native_filter_values_incomplete",
("native_filter_values_incomplete",)),
+ (
+ "Missing state is not evidence of no filters",
+ ("Missing state", "no filters"),
+ ),
+ (
+ "snapshot values, not automatically enforced query predicates",
+ ("snapshot", "not query predicates"),
+ ),
+ ("Respect filter scope", ("scope",)),
+ ("do not guess columns", ("guess columns",)),
+ ("query workspace-wide data", ("workspace-wide",)),
+ ("Ask for clarification instead", ("clarify",)),
+ ("To retrieve the complete list of charts", ("list_charts",)),
+ ],
+ "generate_dashboard": [
+ ("Use this tool ONLY when creating a brand-new", ("NEW dashboards
only",)),
+ ("use add_chart_to_existing_dashboard",
("add_chart_to_existing_dashboard",)),
+ ("Never use this tool as a fallback", ("Never use as a fallback",)),
+ ("must exist and be accessible", ("exist", "accessible")),
+ ("When ``position_json`` is supplied", ("position_json",)),
+ ],
+}
+
+
+# Field-level details are available in compact discovery's full inputSchema,
+# not in schema-free summary mode. Audit both without expanding the prose
budget.
+SCHEMA_DOCSTRING_CONSTRAINTS: dict[str, list[tuple[str, tuple[str, ...]]]] = {
+ "list_datasets": [
+ (
+ "Search matches schema, SQL, table name, and description as "
+ "case-insensitive substrings",
+ ("Case-insensitive substring search of schema, SQL, table name",),
+ ),
+ (
+ "A complete UUID passed as ``search`` is treated as an exact UUID",
+ ("A complete UUID is an exact UUID lookup", "uuid filter"),
+ ),
+ (
+ "Compare descriptions and metadata",
+ ("Compare candidate descriptions and metadata",),
+ ),
+ (
+ "Sortable columns for ``order_column``",
+ ("Sortable columns: id, table_name, schema, changed_on,
created_on",),
+ ),
+ (
+ "``changed_on_delta_humanized`` (alias for ``changed_on``)",
+ ("changed_on_delta_humanized is an alias for changed_on",),
+ ),
+ (
+ "Set ``request.certified`` to true",
+ (
+ "true to return only certified datasets",
+ "false to return only uncertified",
+ "omit to return both",
+ ),
+ ),
+ (
+ "Valid filter columns for ``filters[].col``",
+ (
+ "uuid",
+ "table_name",
+ "schema",
+ "database_name",
+ "created_by_fk",
+ "changed_by_fk",
+ ),
+ ),
+ (
+ 'filters=[{"col": "created_by_fk", "opr": "eq", "value": <id>}]',
+ ("created_by_fk or changed_by_fk with that integer ID",),
+ ),
+ ],
+ "list_dashboards": [
+ (
+ "Sortable columns for ``order_column``",
+ (
+ "Sortable columns: id, dashboard_title, slug, published, "
+ "changed_on, created_on",
+ ),
+ ),
+ (
+ "``changed_on_delta_humanized`` (alias for ``changed_on``)",
+ ("changed_on_delta_humanized is an alias for changed_on",),
+ ),
+ (
+ "search matches titles and slugs only",
+ ("Search matches titles and slugs only",),
+ ),
+ (
+ "Use select_columns to request additional fields",
+ (
+ "select_columns",
+ "List of columns to select",
+ ),
+ ),
+ (
+ "Valid filter columns for ``filters[].col``",
+ (
+ "dashboard_title",
+ "published",
+ "editor",
+ "favorite",
+ "created_by_fk",
+ "changed_by_fk",
+ ),
+ ),
+ (
+ 'filters=[{"col": "created_by_fk", "opr": "eq", "value": <id>}]',
+ ("created_by_fk or changed_by_fk with that integer ID",),
+ ),
+ ],
+ "list_charts": [
+ (
+ "Sortable columns for ``order_column``",
+ (
+ "Sortable columns: id, slice_name, viz_type, description, "
+ "changed_on, created_on",
+ ),
+ ),
+ (
+ "``changed_on_delta_humanized`` (alias for ``changed_on``)",
+ ("changed_on_delta_humanized is an alias for changed_on",),
+ ),
+ (
+ "Set ``request.certified`` to true",
+ (
+ "true to return only certified charts",
+ "false to return only uncertified",
+ "omit to return both",
+ ),
+ ),
+ (
+ "Valid filter columns for ``filters[].col``",
+ (
+ "slice_name",
+ "viz_type",
+ "datasource_name",
+ "editor",
+ "created_by_fk",
+ "changed_by_fk",
+ "dashboards",
+ ),
+ ),
+ (
+ 'filters=[{"col": "created_by_fk", "opr": "eq", "value": <id>}]',
+ ("created_by_fk or changed_by_fk with that integer ID",),
+ ),
+ ],
+ "get_dashboard_info": [
+ (
+ "use the returned filter_state as context",
+ ("Use returned filter_state as context",),
+ ),
+ (
+ "Restricted users receive native_filter_values",
+ (
+ "native_filter_values (names, types, values, labels, exclusion
flags)",
+ "not raw dataMask/column targets",
+ ),
+ ),
+ (
+ "unsupported filters and chart state cannot be summarized safely",
+ (
+ "native_filter_values_incomplete flags unsupported
filters/chart state",
+ "cannot be summarized safely",
+ ),
+ ),
+ (
+ "lists may be capped below their true size",
+ ("Charts/native_filters may be capped", "chart_count",
"_truncation_notes"),
+ ),
+ (
+ "To retrieve the complete list of charts",
+ (
+ 'list_charts with request={"filters": [{"col": "dashboards", '
+ '"opr": "eq", "value": <dashboard id>}]}',
+ "paginate with page/page_size",
+ ),
+ ),
+ (
+ "pass the URL or bare key as ``identifier``",
+ (
+ "bare permalink key",
+ "shared URL",
+ "/superset/dashboard/p/<key>/",
+ "identifier",
+ ),
+ ),
+ (
+ "or use ``permalink_key`` alone",
+ ("no identifier is required",),
+ ),
+ ],
+ "get_chart_info": [
+ (
+ "URL field links to the chart's explore page",
+ ("url field links to the chart's Explore page",),
+ ),
+ ("form_data_key from Explore URL", ("Cache key from the Explore
URL",)),
+ (
+ "With an Explore permalink (key or full URL)",
+ ("full permalink URL", "/explore/p/<key>/"),
+ ),
+ (
+ "When dashboard_id is provided",
+ (
+ "dashboard_id",
+ "column, operator, and value under filters.dashboard_filters",
+ "scope for this chart",
+ ),
+ ),
+ ],
+ "generate_dashboard": [
+ ("auto-generated 2-column grid", ("auto-generated 2-column grid",)),
+ ("MARKDOWN/HEADER components", ("MARKDOWN", "HEADER components")),
+ (
+ "``parents`` is recomputed from its ``children`` edges",
+ (
+ "parents is recomputed from its children edges",
+ "omitted or incomplete parents arrays are fine",
+ ),
+ ),
+ ],
+}
+
+DOCSTRING_CONSTRAINTS = {
+ name: constraints + SCHEMA_DOCSTRING_CONSTRAINTS[name]
+ for name, constraints in SUMMARY_DOCSTRING_CONSTRAINTS.items()
+}
+
+
+def _schema_text(value: object) -> str:
+ """Collect schema text, including referenced definitions, without JSON
escaping."""
+ if isinstance(value, dict):
+ return " ".join(f"{key} {_schema_text(item)}" for key, item in
value.items())
+ if isinstance(value, list):
+ return " ".join(_schema_text(item) for item in value)
+ return str(value)
+
+
+def _discovery_text(entry: dict[str, object], include_schemas: bool) -> str:
+ """Everything a client sees for one tool in a search result."""
+ if include_schemas:
+ schema = entry["inputSchema"]
+ assert isinstance(schema, dict)
+ guidance = _schema_text(schema)
+ else:
+ guidance = str(entry.get("parameters_hint", ""))
+ return f"{entry.get('description', '')} {guidance}"
+
+
+def test_docstring_constraint_audit_covers_touched_tools() -> None:
+ """Every tool carrying request instructions is audited sentence by
sentence."""
+ assert set(DOCSTRING_CONSTRAINTS) == set(CONSTRAINTS)
+
+
[email protected]
[email protected]("name", DOCSTRING_CONSTRAINTS)
[email protected]("include_schemas", [True, False])
+async def test_docstring_constraints_survive_default_discovery(
+ name: str, include_schemas: bool
+) -> None:
+ """Default compact and summary results keep each constraint the docstring
states."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ docstring = re.sub(r"\s+", " ", tool.description or "")
+ config = {**MCP_TOOL_SEARCH_CONFIG, "include_schemas": include_schemas}
+ entry = _create_search_result_serializer(config)([tool])[0]
+ text = _discovery_text(entry, include_schemas)
+ missing: list[tuple[str, str]] = []
+ constraints = (
+ DOCSTRING_CONSTRAINTS[name]
+ if include_schemas
+ else SUMMARY_DOCSTRING_CONSTRAINTS[name]
+ )
+ for sentence, phrases in constraints:
+ assert sentence in docstring, (name, sentence)
+ missing.extend((sentence, phrase) for phrase in phrases if phrase not
in text)
+ assert not missing, (name, missing, text)
+
+
[email protected]
[email protected]("name", CONSTRAINTS)
[email protected]("include_schemas", [True, False])
+async def test_default_discovery_keeps_purpose_line(
+ name: str, include_schemas: bool
+) -> None:
+ """Request guidance must leave room for the docstring's first purpose
sentence."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ docstring = re.sub(r"\s+", " ", tool.description or "").strip()
+ purpose = re.match(r".+?[.!?](?=\s|$)", docstring)
+ assert purpose is not None, name
+ config = {**MCP_TOOL_SEARCH_CONFIG, "include_schemas": include_schemas}
+ entry = _create_search_result_serializer(config)([tool])[0]
+ assert entry["description"].startswith(purpose.group(0)), (name, entry)
+
+
+DIRECT_CATALOG_CONSTRAINTS = {
+ "generate_chart": (
+ "save_chart=True saves",
+ "MUST display chart URL",
+ "numeric ID/UUID",
+ "NOT schema.table_name",
+ "config.chart_type required",
+ "line/bar/area/scatter are xy kind values",
+ "get_chart_type_schema",
+ ),
+ "create_virtual_dataset": (
+ "SQL and a dataset name",
+ "returned id as dataset_id",
+ "generate_chart or generate_explore_link",
+ "columns from returned columns",
+ ),
+ "update_chart": (
+ "generate_preview=True previews; False persists immediately",
+ "MUST display explore URL",
+ "ID/UUID, NOT chart name",
+ "Omit config to rename only",
+ "add_columns appends table columns",
+ ),
+ "update_chart_preview": (
+ "Cached preview only, not saved",
+ "form_data_key is invalidated",
+ "use the returned key",
+ "MUST display explore_url",
+ "config + dataset_id and omit form_data_key",
+ ),
+}
+
+
[email protected]
[email protected]("name", DIRECT_CATALOG_CONSTRAINTS)
[email protected]("strategy", ["bm25", "regex"])
[email protected]("include_schemas", [True, False])
+async def test_direct_no_query_catalog_preserves_priority_guidance(
+ name: str, strategy: str, include_schemas: bool
+) -> None:
+ """Reporter cases use real registrations and no-query search, without
writes."""
+ from unittest.mock import AsyncMock, MagicMock, patch
+
+ from superset.mcp_service.server import _apply_tool_search_transform
+ from superset.utils import json
+
+ server = MagicMock()
+ _apply_tool_search_transform(
+ server,
+ {
+ **MCP_TOOL_SEARCH_CONFIG,
+ "strategy": strategy,
+ "include_schemas": include_schemas,
+ },
+ )
+ transform = server.add_transform.call_args.args[0]
+ tools = await mcp.list_tools(run_middleware=False)
+ # Only discovery of visible tools is substituted; rendering and
serialization
+ # run as in a direct MCP call, with no query and the default 300-char
budget.
+ with patch.object(transform, "_get_visible_tools",
AsyncMock(return_value=tools)):
+ result = await transform._make_search_tool().fn()
+ catalog = json.loads(result) if isinstance(result, str) else result
+ assert len(catalog) == len(tools)
+ entry = next(item for item in catalog if item["name"] == name)
+ tool = next(item for item in tools if item.name == name)
+ instructions = tool.parameters["properties"]["request"]["description"]
+ text = _discovery_text(entry, include_schemas)
+ assert 'Wrap as {"request": {...}}.' in text
+ assert all(phrase in text for phrase in DIRECT_CATALOG_CONSTRAINTS[name]),
text
+ assert len(entry["description"]) + len(instructions) <= 300
+ # A purpose paragraph survives; any subsequent paragraphs are whole, never
+ # an incomplete bullet or the misleading numbered-list fragment "Workflow:
1."
+ description = inspect.cleandoc(tool.description or "")
+ paragraphs = re.split(r"\n\s*\n", description)
+ assert entry["description"].startswith(paragraphs[0])
+ assert entry["description"] in [
+ description[: match.start()].strip()
+ for match in re.finditer(r"\n\s*\n", description)
+ ]
+
+
[email protected]
[email protected]("name", DIRECT_CATALOG_CONSTRAINTS)
+async def test_direct_catalog_oversized_prose_keeps_bounded_guidance(name:
str) -> None:
+ """Priority rules are not displaced by an arbitrarily long introduction."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ oversized = tool.model_copy(update={"description": "Long prose. " *
10_000})
+ entry = _create_search_result_serializer({"include_schemas":
True})([oversized])[0]
+ instructions = entry["inputSchema"]["properties"]["request"]["description"]
+ assert all(phrase in instructions for phrase in
DIRECT_CATALOG_CONSTRAINTS[name])
+ assert len(entry["description"]) + len(instructions) <= 300
+
+
[email protected]
[email protected]("name", DIRECT_CATALOG_CONSTRAINTS)
+async def test_reported_descriptions_truncate_at_whole_paragraphs(name: str)
-> None:
+ """The reported 300-char cut cannot leave a partial IMPORTANT block or
step 1."""
+ tool = await mcp.get_tool(name)
+ assert tool is not None
+ description = inspect.cleandoc(tool.description or "")
+ result = _truncate_description(tool.description or "", 300)
+ assert result
+ assert len(result) <= 300
+ assert result in [
+ description[: match.start()].strip()
+ for match in re.finditer(r"\n\s*\n", description)
+ ]
+
+
+TOOLS_WITH_REQUEST_INSTRUCTIONS = {*CONSTRAINTS, *DIRECT_CATALOG_CONSTRAINTS}
+
+
[email protected]
[email protected]("include_schemas", [True, False])
+async def test_served_discovery_keeps_untouched_tool_descriptions(
+ include_schemas: bool,
+) -> None:
+ """Inlined request-model docstrings never shrink other tools' served
prose."""
+ tools = await mcp.list_tools(run_middleware=True)
+ config = {**MCP_TOOL_SEARCH_CONFIG, "include_schemas": include_schemas}
+ max_desc = config.get("max_description_length", 300)
+ entries = _create_search_result_serializer(config)(tools)
+ assert {tool.name for tool in tools} >= TOOLS_WITH_REQUEST_INSTRUCTIONS
+ for tool, entry in zip(tools, entries, strict=True):
+ assert entry["description"], tool.name
+ assert len(entry["description"]) <= max_desc, tool.name
+ if tool.name in TOOLS_WITH_REQUEST_INSTRUCTIONS:
+ continue
+ assert _request_instructions(tool) == "", tool.name
+ assert entry["description"] == _truncate_description(
+ tool.description or "", max_desc
+ ), tool.name
+ if not include_schemas and (hint := entry.get("parameters_hint")):
+ properties = tool.to_mcp_tool().inputSchema.get("properties", {})
+ assert hint == ", ".join(properties), tool.name
+
+
+# Tools whose served description was empty or lost a calling rule when the
+# request-model docstring was deducted, or when the next paragraph was dropped.
+UNTOUCHED_DESCRIPTION_PHRASES = {
+ "manage_dashboard_owners": "Owners can edit the dashboard",
+ "manage_dashboard_roles": "Dashboard access roles restrict who can view",
+ "get_chart_preview": "Returns preview URL or formatted content",
+ "get_chart_data": "Returns the actual data behind a chart",
+ "generate_bug_report": "Generate a copy-pasteable bug report",
+ "manage_dashboard_certification": "Set or clear a dashboard's
certification",
+ "update_dashboard": "Patch an existing dashboard's layout",
+ "get_chart_sql": "Returns the SQL that a chart would execute",
+ "find_users": "Resolve a person's name to user IDs",
+ "delete_chart": "Identify the chart by numeric ID or UUID string (NOT
chart name)",
+}
+
+
[email protected]
[email protected]("name", UNTOUCHED_DESCRIPTION_PHRASES)
[email protected]("include_schemas", [True, False])
+async def test_served_discovery_regressions_keep_prose(
+ name: str, include_schemas: bool
+) -> None:
+ """Reported tools keep their default-limit prose in served search
results."""
+ tools = await mcp.list_tools(run_middleware=True)
+ tool = next(item for item in tools if item.name == name)
+ config = {**MCP_TOOL_SEARCH_CONFIG, "include_schemas": include_schemas}
+ entry = _create_search_result_serializer(config)([tool])[0]
+ assert UNTOUCHED_DESCRIPTION_PHRASES[name] in re.sub(
+ r"\s+", " ", entry["description"]
+ ), (name, entry["description"])
diff --git a/tests/unit_tests/mcp_service/test_tool_inventory.py
b/tests/unit_tests/mcp_service/test_tool_inventory.py
index b1024f64c30..a9036d047fe 100644
--- a/tests/unit_tests/mcp_service/test_tool_inventory.py
+++ b/tests/unit_tests/mcp_service/test_tool_inventory.py
@@ -56,7 +56,7 @@ TOOL_BUDGETS = {
"apply_dashboard_filters": 2_900,
"create_dataset": 1_800,
"create_dataset_metric": 2_800,
- "create_theme": 1_100,
+ "create_theme": 1_200,
"create_virtual_dataset": 3_700,
"delete_chart": 1_100,
"delete_dashboard": 1_100,
diff --git a/tests/unit_tests/mcp_service/test_tool_search_transform.py
b/tests/unit_tests/mcp_service/test_tool_search_transform.py
index 652bf587ea7..91ae4a862dd 100644
--- a/tests/unit_tests/mcp_service/test_tool_search_transform.py
+++ b/tests/unit_tests/mcp_service/test_tool_search_transform.py
@@ -392,12 +392,11 @@ def test_truncate_description_cuts_at_sentence():
assert result == "First sentence. Second sentence."
-def test_truncate_description_ellipsis_fallback():
- """When no sentence boundary, truncates with ellipsis."""
+def test_truncate_description_without_sentence_boundary() -> None:
+ """Omit prose rather than advertising a partial instruction."""
text = "A very long single sentence without periods that goes on and on"
result = _truncate_description(text, 30)
- assert result.endswith("...")
- assert len(result) <= 33 # 30 + "..."
+ assert result == ""
def test_truncate_description_empty():
@@ -405,14 +404,77 @@ def test_truncate_description_empty():
assert _truncate_description("", 300) == ""
-def test_truncate_description_zero_max():
- """Zero max_length produces ellipsis; the serializer skips calling this."""
+def test_truncate_description_zero_max() -> None:
+ """No prose remains when schema instructions consume the entire budget."""
text = "Some text"
- # _truncate_description(text, 0) truncates to 0 chars and appends "...".
- # The caller (_create_search_result_serializer) skips calling it when
- # max_desc=0 so this edge case only matters for direct callers.
result = _truncate_description(text, 0)
- assert result == "..."
+ assert result == ""
+
+
[email protected]("limit", [1, 2, 20, 300])
+def test_truncate_description_oversized(limit: int) -> None:
+ """Oversized prose never exceeds even a very small configured budget."""
+ assert _truncate_description("x" * 100_000, limit) == ""
+
+
+def test_truncate_description_multiline_sentence() -> None:
+ """A newline after punctuation is a sentence boundary too."""
+ assert _truncate_description(
+ "First sentence.\nA long instruction follows.", 20
+ ) == ("First sentence.")
+
+
[email protected]("marker", ["IMPORTANT:", "**IMPORTANT**:"])
+def test_truncate_description_important_block_sentence(marker: str) -> None:
+ """Do not advertise a half instruction when the cut falls in an IMPORTANT
block."""
+ prefix = f"Summary.\n\n{marker} First rule."
+ text = prefix + "\n" + "An instruction too long for the remaining budget "
* 100
+ assert _truncate_description(text, 100) == "Summary."
+
+
+def test_truncate_description_spends_budget_on_next_paragraph_sentences() ->
None:
+ """A following prose paragraph contributes the complete sentences that
fit."""
+ text = (
+ "Delete a saved chart.\n\nIdentify the chart by ID (NOT name). "
+ + "A long trailing sentence that cannot fit in the budget. " * 5
+ )
+ assert _truncate_description(text, 70) == (
+ "Delete a saved chart.\n\nIdentify the chart by ID (NOT name)."
+ )
+
+
[email protected](
+ "following",
+ [
+ "Workflow:\n1. First step.\n2. Second step that is far too long " +
"x" * 80,
+ "Steps follow.\n- First item.\n- Second item " + "x" * 80,
+ "Parameters:\n None. Long text " + "x" * 80,
+ ],
+)
+def test_truncate_description_never_starts_structured_paragraph(
+ following: str,
+) -> None:
+ """Lists and headings after the kept paragraphs are never partly
advertised."""
+ assert _truncate_description(f"Summary.\n\n{following}", 60) == "Summary."
+
+
+def test_truncate_description_drops_trailing_lead_in_sentence() -> None:
+ """A kept paragraph ending in a colon must not advertise a cut-off list."""
+ text = "Edit things. An LLM can:\n\n- first\n- second\n\n" + "x" * 300
+ assert _truncate_description(text, 30) == "Edit things."
+
+
+def test_truncate_description_keeps_lone_lead_in_sentence() -> None:
+ """With no earlier sentence, the colon-terminated text is left
untouched."""
+ assert _truncate_description("An LLM can:\n\n- first\n\n" + "x" * 300, 15)
== (
+ "An LLM can:"
+ )
+
+
+def test_truncate_description_long_first_paragraph_keeps_sentences() -> None:
+ """An overlong first paragraph still yields its complete leading
sentences."""
+ text = "Purpose line. " + "More detail here. " * 20 + "\n\nSecond
paragraph."
+ assert _truncate_description(text, 40) == "Purpose line. More detail here."
# -- _create_search_result_serializer tests --
