fix(mcp): make list-item truncation cap configurable in get_dashboard_info (#41698)

This commit is contained in:
Amin Ghadersohi
2026-07-13 14:46:10 -04:00
committed by GitHub
parent c5131bff47
commit 62ccdfacc2
12 changed files with 286 additions and 11 deletions

View File

@@ -529,6 +529,7 @@ MCP_RESPONSE_SIZE_CONFIG = {
"enabled": True,
"token_limit": 25000,
"warn_threshold_pct": 80,
"max_list_items": 100,
"excluded_tools": [
"health_check",
"get_chart_preview",
@@ -543,6 +544,7 @@ MCP_RESPONSE_SIZE_CONFIG = {
| `enabled` | `True` | Enable response size checking |
| `token_limit` | `25000` | Maximum estimated token count per response |
| `warn_threshold_pct` | `80` | Warn when response exceeds this percentage of the limit |
| `max_list_items` | `100` | Cap on list-field length (e.g. `charts`, `native_filters`) applied to the `get_*_info` tools before falling back to more aggressive truncation. Raised from a hardcoded 30 in earlier versions; the higher default only keeps more data before the same token-budget fallback kicks in, so it's not a breaking change, but tenants that tuned workflows around the old 30-item cap should lower this value explicitly. |
| `excluded_tools` | See above | Tools exempt from size checking (e.g., tools that return URLs, not data) |
### Caching

View File

@@ -105,7 +105,7 @@ async def list_charts(
Valid filter columns for ``filters[].col``:
``slice_name``, ``viz_type``, ``datasource_name``,
``created_by_fk``, ``changed_by_fk``
``created_by_fk``, ``changed_by_fk``, ``dashboards``
Sortable columns for ``order_column``:
``id``, ``slice_name``, ``viz_type``, ``description``,

View File

@@ -28,3 +28,5 @@ MAX_PAGE_SIZE = 100 # Maximum allowed page_size to prevent oversized responses
# Response size guard defaults
DEFAULT_TOKEN_LIMIT = 25_000 # ~25k tokens prevents overwhelming LLM context windows
DEFAULT_WARN_THRESHOLD_PCT = 80 # Log warnings above 80% of limit
# Phase 2 list-field truncation cap; matches MAX_PAGE_SIZE
DEFAULT_MAX_LIST_ITEMS = 100

View File

@@ -452,7 +452,20 @@ class DashboardInfo(BaseModel):
chart_count: int = 0
editors: List[SubjectInfo] = Field(default_factory=list)
tags: List[TagInfo] = Field(default_factory=list)
charts: List[DashboardChartSummary] = Field(default_factory=list)
charts: List[DashboardChartSummary] = Field(
default_factory=list,
description=(
"Charts on this dashboard. May be capped below chart_count "
"(cap: MCP_RESPONSE_SIZE_CONFIG['max_list_items']) when the full "
"response would exceed the token budget. "
"Compare len(charts) to chart_count to detect this. For "
"dashboards with more charts than the cap, call list_charts "
"with filters=[{'col': 'dashboards', 'opr': 'eq', "
"'value': <this dashboard's id>}] and page through with "
"page/page_size to retrieve the complete list regardless of "
"size."
),
)
# Structured filter information extracted from json_metadata
native_filters: List[NativeFilterSummary] = Field(
@@ -460,7 +473,11 @@ class DashboardInfo(BaseModel):
description=(
"Native filters configured on this dashboard. Extracted from "
"json_metadata for LLM consumption. Includes filter name/type, "
"and target columns only when data-model metadata is allowed."
"and target columns only when data-model metadata is allowed. "
"Subject to the same max_list_items cap as charts, though "
"dashboards rarely have enough native filters to hit it; "
"operators can raise MCP_RESPONSE_SIZE_CONFIG['max_list_items'] "
"for tenants that do."
),
)
cross_filters_enabled: bool | None = Field(

View File

@@ -122,6 +122,14 @@ async def get_dashboard_info(
Returns title, charts, and layout details.
For dashboards with many charts or native filters, the ``charts`` and
``native_filters`` lists may be capped below their true size (see
``chart_count`` for the real total, and ``_truncation_notes`` in the
response when truncation occurred). To retrieve the complete list of
charts on a large dashboard regardless of size, call ``list_charts``
with ``filters=[{"col": "dashboards", "opr": "eq", "value": <dashboard
id>}]`` and page through the results using ``page``/``page_size``.
When permalink_key is provided, also returns the filter state from that
permalink, allowing you to see what filters the user has applied to the
dashboard (not just the default filter state).

View File

@@ -27,6 +27,7 @@ from flask import Flask
from superset.constants import CHANGE_ME_GUEST_TOKEN_JWT_SECRET
from superset.mcp_service.composite_token_verifier import CompositeTokenVerifier
from superset.mcp_service.constants import (
DEFAULT_MAX_LIST_ITEMS,
DEFAULT_TOKEN_LIMIT,
DEFAULT_WARN_THRESHOLD_PCT,
)
@@ -322,6 +323,12 @@ MCP_CACHE_CONFIG: dict[str, Any] = {
# - token_limit: Maximum estimated tokens per response (default: 25,000)
# - excluded_tools: Tools to skip checking (e.g., streaming tools)
# - warn_threshold_pct: Log warnings above this % of limit (default: 80%)
# - max_list_items: Cap applied to list fields (e.g. ``charts``,
# ``native_filters``) during Phase 2 of dynamic truncation for the "info"
# tools (get_chart_info, get_dataset_info, get_dashboard_info,
# get_instance_info) when a response exceeds token_limit (default: 100).
# Operators with tenants that have unusually large dashboards (hundreds of
# charts/filters) can raise this value to return more complete responses.
#
# Token Estimation:
# -----------------
@@ -332,6 +339,7 @@ MCP_RESPONSE_SIZE_CONFIG: dict[str, Any] = {
"enabled": True, # Enabled by default to protect LLM clients
"token_limit": DEFAULT_TOKEN_LIMIT,
"warn_threshold_pct": DEFAULT_WARN_THRESHOLD_PCT,
"max_list_items": DEFAULT_MAX_LIST_ITEMS,
"excluded_tools": [ # Tools to skip size checking
"health_check", # Always small
"generate_explore_link", # Returns URLs

View File

@@ -47,6 +47,7 @@ from superset.mcp_service.auth import (
MCPPermissionDeniedError,
)
from superset.mcp_service.constants import (
DEFAULT_MAX_LIST_ITEMS,
DEFAULT_TOKEN_LIMIT,
DEFAULT_WARN_THRESHOLD_PCT,
)
@@ -1174,6 +1175,7 @@ class ResponseSizeGuardMiddleware(Middleware):
- enabled: Toggle the guard on/off (default: True)
- token_limit: Maximum estimated tokens per response (default: 25,000)
- warn_threshold_pct: Log warnings above this % of limit (default: 80%)
- max_list_items: Cap for list fields during dynamic truncation (default: 100)
- excluded_tools: Tools to skip checking
"""
@@ -1182,6 +1184,7 @@ class ResponseSizeGuardMiddleware(Middleware):
token_limit: int = DEFAULT_TOKEN_LIMIT,
warn_threshold_pct: int = DEFAULT_WARN_THRESHOLD_PCT,
excluded_tools: list[str] | str | None = None,
max_list_items: int = DEFAULT_MAX_LIST_ITEMS,
) -> None:
self.token_limit = token_limit
self.warn_threshold_pct = warn_threshold_pct
@@ -1189,6 +1192,7 @@ class ResponseSizeGuardMiddleware(Middleware):
if isinstance(excluded_tools, str):
excluded_tools = [excluded_tools]
self.excluded_tools = set(excluded_tools or [])
self.max_list_items = max(1, max_list_items)
@staticmethod
def _extract_payload_from_tool_result(
@@ -1272,7 +1276,9 @@ class ResponseSizeGuardMiddleware(Middleware):
try:
truncated, was_truncated, notes = truncate_oversized_response(
truncation_target, self.token_limit
truncation_target,
self.token_limit,
max_list_items=self.max_list_items,
)
except (MemoryError, RecursionError) as trunc_error:
logger.warning(
@@ -1420,6 +1426,27 @@ class ResponseSizeGuardMiddleware(Middleware):
return response
def _safe_int_config(config: Dict[str, Any], key: str, default: int) -> int:
"""Best-effort int coercion for MCP_RESPONSE_SIZE_CONFIG values.
Falls back to ``default`` (with a warning log) when the configured value
can't be converted to an int, so a malformed ``superset_config.py``
setting doesn't crash middleware initialization.
"""
value = config.get(key, default)
try:
return int(value)
except (TypeError, ValueError):
logger.warning(
"Invalid %s in MCP_RESPONSE_SIZE_CONFIG: %r is not a valid integer; "
"falling back to default %d",
key,
value,
default,
)
return default
def create_response_size_guard_middleware() -> ResponseSizeGuardMiddleware | None:
"""
Factory function to create ResponseSizeGuardMiddleware from config.
@@ -1445,12 +1472,17 @@ def create_response_size_guard_middleware() -> ResponseSizeGuardMiddleware | Non
logger.info("Response size guard is disabled")
return None
max_list_items: int = _safe_int_config(
config, "max_list_items", DEFAULT_MAX_LIST_ITEMS
)
middleware = ResponseSizeGuardMiddleware(
token_limit=int(config.get("token_limit", DEFAULT_TOKEN_LIMIT)),
warn_threshold_pct=int(
config.get("warn_threshold_pct", DEFAULT_WARN_THRESHOLD_PCT)
token_limit=_safe_int_config(config, "token_limit", DEFAULT_TOKEN_LIMIT),
warn_threshold_pct=_safe_int_config(
config, "warn_threshold_pct", DEFAULT_WARN_THRESHOLD_PCT
),
excluded_tools=config.get("excluded_tools"),
max_list_items=max_list_items,
)
logger.info(

View File

@@ -51,6 +51,8 @@ from typing import Any, Dict, List, Union
from pydantic import BaseModel
from typing_extensions import TypeAlias
from superset.mcp_service.constants import DEFAULT_MAX_LIST_ITEMS
logger = logging.getLogger(__name__)
# Type alias for MCP tool responses (Pydantic models, dicts, lists, strings, bytes)
@@ -469,8 +471,6 @@ INFO_TOOLS = frozenset(
# Maximum character length for string fields before truncation
_MAX_STRING_CHARS = 500
# Maximum items to keep in list fields before truncation
_MAX_LIST_ITEMS = 30
# Maximum keys to keep when summarizing large dict fields
_MAX_DICT_KEYS = 20
@@ -538,6 +538,7 @@ def _truncate_lists(data: Dict[str, Any], notes: List[str], max_items: int) -> b
metadata is communicated through the *notes* list and top-level response
fields ``_response_truncated`` / ``_truncation_notes``.
"""
max_items = max(1, max_items)
changed = False
for key, value in data.items():
if isinstance(value, list) and len(value) > max_items:
@@ -601,13 +602,15 @@ def _is_under_limit(data: Dict[str, Any], token_limit: int) -> bool:
def truncate_oversized_response(
response: ToolResponse,
token_limit: int,
# Configurable via MCP_RESPONSE_SIZE_CONFIG["max_list_items"]
max_list_items: int = DEFAULT_MAX_LIST_ITEMS,
) -> tuple[ToolResponse, bool, list[str]]:
"""
Dynamically truncate large fields in a response to fit within the token limit.
Applies five progressive phases of truncation:
1. Truncate long top-level string fields
2. Truncate large list fields to _MAX_LIST_ITEMS
2. Truncate large list fields to max_list_items (configurable)
3. Recursively truncate strings in nested structures (list items, nested dicts)
4. Aggressively reduce lists to 10 items and summarize large dicts
5. Replace all collections with empty values
@@ -615,6 +618,7 @@ def truncate_oversized_response(
Args:
response: The tool response (Pydantic model, dict, or other).
token_limit: Maximum estimated tokens allowed.
max_list_items: Maximum items to keep in list fields during Phase 2.
Returns:
A tuple of (possibly-truncated response, was_truncated, list of notes).
@@ -637,7 +641,7 @@ def truncate_oversized_response(
return data, was_truncated, notes
# Phase 2: Truncate large list fields
was_truncated |= _truncate_lists(data, notes, _MAX_LIST_ITEMS)
was_truncated |= _truncate_lists(data, notes, max_list_items)
if _is_under_limit(data, token_limit):
return data, was_truncated, notes

View File

@@ -32,6 +32,7 @@ from superset.mcp_service.dashboard.schemas import (
_extract_native_filters,
_safe_user_label,
dashboard_serializer,
DashboardInfo,
DuplicateDashboardRequest,
DuplicateDashboardResponse,
GenerateDashboardRequest,
@@ -883,6 +884,32 @@ class TestDuplicateDashboardRequestTitleSanitization:
assert req.sanitization_warnings == []
class TestDashboardInfoLargeListGuidance:
"""DashboardInfo documents how agents can retrieve charts/native_filters
beyond the response-size guard's list-item cap.
Regression test for the Medialab large-dashboard report: with the old
hardcoded 30-item cap and no documented escape hatch, agents had no way
to retrieve the rest of a dashboard's charts. These field descriptions
are the "documented, agent-usable way to access items beyond the cap"
called for by the story's acceptance criteria.
"""
def test_charts_field_documents_list_charts_escape_hatch(self) -> None:
"""The charts field description points to list_charts pagination."""
description: str | None = DashboardInfo.model_fields["charts"].description
assert description is not None
assert "list_charts" in description
def test_native_filters_field_documents_max_list_items_config(self) -> None:
"""The native_filters field description mentions the configurable cap."""
description: str | None = DashboardInfo.model_fields[
"native_filters"
].description
assert description is not None
assert "max_list_items" in description
class TestDuplicateDashboardResponse:
"""Serialization and error sanitization for DuplicateDashboardResponse."""

View File

@@ -306,6 +306,20 @@ async def test_list_dashboards_with_simple_filters(mock_list, mcp_server):
assert "count" in data
def test_get_dashboard_info_docstring_documents_list_charts_escape_hatch() -> None:
"""The tool docstring tells agents how to page past the charts cap.
Regression test for the Medialab large-dashboard report: agents calling
get_dashboard_info on a dashboard with more charts than the response-size
guard's cap need a documented way to retrieve the rest. This is surfaced
to the LLM via the tool's docstring (in addition to the charts field
description on DashboardInfo).
"""
doc = get_dashboard_info_module.get_dashboard_info.__doc__
assert doc is not None
assert "list_charts" in doc
@patch(
"superset.mcp_service.dashboard.schemas.user_can_view_data_model_metadata",
return_value=True,

View File

@@ -35,6 +35,7 @@ from superset.commands.exceptions import (
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetException, SupersetSecurityException
from superset.mcp_service.auth import MCPNoAuthSourceError, MCPPermissionDeniedError
from superset.mcp_service.constants import DEFAULT_MAX_LIST_ITEMS
from superset.mcp_service.mcp_config import MCP_RESPONSE_SIZE_CONFIG
from superset.mcp_service.middleware import (
_is_user_error,
@@ -55,6 +56,7 @@ class TestResponseSizeGuardMiddleware:
assert middleware.warn_threshold_pct == 80
assert middleware.warn_threshold == 20000
assert middleware.excluded_tools == set()
assert middleware.max_list_items == 100
def test_init_custom_values(self) -> None:
"""Should initialize with custom values."""
@@ -62,11 +64,13 @@ class TestResponseSizeGuardMiddleware:
token_limit=10000,
warn_threshold_pct=70,
excluded_tools=["health_check", "get_chart_preview"],
max_list_items=50,
)
assert middleware.token_limit == 10000
assert middleware.warn_threshold_pct == 70
assert middleware.warn_threshold == 7000
assert middleware.excluded_tools == {"health_check", "get_chart_preview"}
assert middleware.max_list_items == 50
def test_init_excluded_tools_as_string(self) -> None:
"""Should handle excluded_tools as a single string."""
@@ -75,6 +79,14 @@ class TestResponseSizeGuardMiddleware:
)
assert middleware.excluded_tools == {"health_check"}
@pytest.mark.parametrize("configured_value", [0, -1, -100])
def test_init_clamps_non_positive_max_list_items(
self, configured_value: int
) -> None:
"""A misconfigured max_list_items of 0 or negative should clamp to 1."""
middleware = ResponseSizeGuardMiddleware(max_list_items=configured_value)
assert middleware.max_list_items == 1
@pytest.mark.asyncio
async def test_allows_small_response(self) -> None:
"""Should allow responses under token limit."""
@@ -331,6 +343,43 @@ class TestResponseSizeGuardMiddleware:
call_args = mock_event_logger.log.call_args
assert call_args.kwargs["action"] == "mcp_response_truncated"
@pytest.mark.asyncio
async def test_truncates_dashboard_info_with_custom_max_list_items(self) -> None:
"""Should respect a custom max_list_items cap for get_dashboard_info.
Regression test for the Medialab large-dashboard report: with the
default hardcoded cap of 30, a dashboard's charts/native_filters
lists were always truncated to 30 regardless of configuration. This
verifies the cap is now threaded through from the middleware
constructor rather than hardcoded.
"""
middleware = ResponseSizeGuardMiddleware(token_limit=3000, max_list_items=50)
context = MagicMock()
context.message.name = "get_dashboard_info"
context.message.params = {}
large_response = {
"id": 1,
"dashboard_title": "x" * 2000,
"charts": [{"id": i, "slice_name": f"chart_{i}"} for i in range(463)],
"native_filters": [{"id": i, "name": f"filter_{i}"} for i in range(48)],
}
call_next = AsyncMock(return_value=large_response)
with (
patch("superset.mcp_service.middleware.get_user_id", return_value=1),
patch("superset.mcp_service.middleware.event_logger"),
):
result = await middleware.on_call_tool(context, call_next)
assert isinstance(result, dict)
assert result["_response_truncated"] is True
# Truncated to the custom cap (50), not the old hardcoded 30
assert len(result["charts"]) == 50
# native_filters (48 items) fits under the custom cap, untouched
assert len(result["native_filters"]) == 48
class TestCreateResponseSizeGuardMiddleware:
"""Test create_response_size_guard_middleware factory function."""
@@ -406,6 +455,46 @@ class TestCreateResponseSizeGuardMiddleware:
assert middleware.token_limit == 25_000 # Default
assert middleware.warn_threshold_pct == 80 # Default
def test_falls_back_to_default_when_max_list_items_is_none(self) -> None:
"""A config explicitly set to None (not just missing) shouldn't crash.
`dict.get(key, default)` only falls back when the key is absent, so
an operator setting MCP_RESPONSE_SIZE_CONFIG["max_list_items"] = None
would otherwise reach `int(None)` and raise TypeError.
"""
mock_config = {"enabled": True, "max_list_items": None}
mock_flask_app = MagicMock()
mock_flask_app.config.get.return_value = mock_config
with patch(
"superset.mcp_service.flask_singleton.get_flask_app",
return_value=mock_flask_app,
):
middleware = create_response_size_guard_middleware()
assert middleware is not None
assert middleware.max_list_items == DEFAULT_MAX_LIST_ITEMS
def test_falls_back_to_default_when_max_list_items_is_non_numeric(self) -> None:
"""A non-numeric config value (e.g. a typo in superset_config.py)
should fall back to the default instead of raising ValueError and
aborting middleware initialization.
"""
mock_config = {"enabled": True, "max_list_items": "many"}
mock_flask_app = MagicMock()
mock_flask_app.config.get.return_value = mock_config
with patch(
"superset.mcp_service.flask_singleton.get_flask_app",
return_value=mock_flask_app,
):
middleware = create_response_size_guard_middleware()
assert middleware is not None
assert middleware.max_list_items == DEFAULT_MAX_LIST_ITEMS
def test_handles_exception_gracefully(self) -> None:
"""Should return None on expected configuration exceptions."""
with patch(

View File

@@ -677,3 +677,75 @@ class TestTruncateOversizedResponse:
assert isinstance(result, dict)
assert result["id"] == 1 # Scalar fields preserved
assert len(notes) > 0
@staticmethod
def _build_large_dashboard_response() -> dict[str, Any]:
"""A dashboard with 463 charts and 48 native_filters, shared by the
default- and custom-max_list_items regression tests below."""
return {
"id": 1,
"dashboard_title": "x" * 2000, # forces Phase 2 to trigger
"charts": [{"id": i, "slice_name": f"chart_{i}"} for i in range(463)],
"native_filters": [{"id": i, "name": f"filter_{i}"} for i in range(48)],
}
def test_large_dashboard_respects_default_max_list_items(self) -> None:
"""Regression test for the Medialab large-dashboard report.
A dashboard with 463 charts and 48 native_filters should have
native_filters (48 items) left untouched under the new default cap
of 100, while charts (463 items) is truncated to 100 — a clear
improvement over the old flat 30-item cap, which truncated both.
"""
response: dict[str, Any] = self._build_large_dashboard_response()
result: Any
was_truncated: bool
notes: list[str]
result, was_truncated, notes = truncate_oversized_response(response, 3000)
assert was_truncated is True
assert isinstance(result, dict)
assert len(result["charts"]) == 100
assert len(result["native_filters"]) == 48
assert any("charts" in n and "463" in n for n in notes)
assert not any("native_filters" in n for n in notes)
def test_large_dashboard_respects_custom_max_list_items(self) -> None:
"""A custom max_list_items below both list sizes should truncate both fields."""
response: dict[str, Any] = self._build_large_dashboard_response()
result: Any
was_truncated: bool
notes: list[str]
result, was_truncated, notes = truncate_oversized_response(
response, 3000, max_list_items=30
)
assert was_truncated is True
assert isinstance(result, dict)
assert len(result["charts"]) == 30
assert len(result["native_filters"]) == 30
assert any("charts" in n and "30" in n for n in notes)
assert any("native_filters" in n and "30" in n for n in notes)
def test_custom_max_list_items_below_phase_four_survives_phase_four(self) -> None:
"""A max_list_items below Phase 4's hardcoded 10 should not be widened.
Phase 2 truncates ``charts`` to 5 first; the response is still over
budget because of the oversized ``form_data`` dict, so truncation
proceeds to Phase 4, whose ``_truncate_lists(..., max_items=10)``
call only shrinks lists larger than 10 — it must leave the
already-smaller 5-item list untouched rather than re-expanding it.
"""
response: dict[str, Any] = {
"id": 1,
"charts": [{"id": i, "slice_name": f"chart_{i}"} for i in range(300)],
"form_data": {f"key_{i}": f"val_{i}" for i in range(50)},
}
result: Any
was_truncated: bool
notes: list[str]
result, was_truncated, notes = truncate_oversized_response(
response, 200, max_list_items=5
)
assert was_truncated is True
assert isinstance(result, dict)
assert len(result["charts"]) == 5
assert any("form_data" in n for n in notes)