mirror of
https://github.com/apache/superset.git
synced 2026-07-27 17:12:36 +00:00
fix(mcp): strip json_metadata and position_json from get_dashboard_info response (#39101)
(cherry picked from commit 680cef0ee0)
This commit is contained in:
@@ -169,9 +169,9 @@ _COLUMN_DESCRIPTIONS: dict[str, str] = {
|
||||
"dashboard_title": "Dashboard display title",
|
||||
"slug": "URL-friendly identifier for the dashboard",
|
||||
"published": "Whether the dashboard is published and visible",
|
||||
"position_json": "JSON layout of dashboard components",
|
||||
"json_metadata": "JSON metadata including filters and settings",
|
||||
"css": "Custom CSS for the dashboard",
|
||||
"native_filters": "Native filter configuration (name, type, targets)",
|
||||
"cross_filters_enabled": "Whether cross-filtering between charts is enabled",
|
||||
"theme_id": "Theme ID for dashboard styling",
|
||||
}
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ Example usage:
|
||||
dashboard_title="Sales Dashboard",
|
||||
published=True,
|
||||
owners=[UserInfo(id=1, username="admin")],
|
||||
charts=[ChartInfo(id=1, slice_name="Sales Chart")]
|
||||
charts=[DashboardChartSummary(id=1, slice_name="Sales Chart")]
|
||||
)
|
||||
|
||||
# For dashboard list responses
|
||||
@@ -65,6 +65,7 @@ Example usage:
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from datetime import datetime
|
||||
from typing import Annotated, Any, Dict, List, Literal, TYPE_CHECKING
|
||||
|
||||
@@ -82,7 +83,6 @@ if TYPE_CHECKING:
|
||||
from superset.models.dashboard import Dashboard
|
||||
|
||||
from superset.daos.base import ColumnOperator, ColumnOperatorEnum
|
||||
from superset.mcp_service.chart.schemas import ChartInfo, serialize_chart_object
|
||||
from superset.mcp_service.common.cache_schemas import MetadataCacheControl
|
||||
from superset.mcp_service.constants import DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE
|
||||
from superset.mcp_service.system.schemas import (
|
||||
@@ -92,6 +92,7 @@ from superset.mcp_service.system.schemas import (
|
||||
TagInfo,
|
||||
UserInfo,
|
||||
)
|
||||
from superset.utils.json import loads as json_loads
|
||||
|
||||
|
||||
class DashboardError(BaseModel):
|
||||
@@ -293,6 +294,43 @@ class GetDashboardInfoRequest(MetadataCacheControl):
|
||||
)
|
||||
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class NativeFilterSummary(BaseModel):
|
||||
"""Lightweight summary of a native filter for LLM consumption.
|
||||
|
||||
Extracts only the fields needed to understand what filters are
|
||||
available on a dashboard: name, type, and which columns they target.
|
||||
"""
|
||||
|
||||
id: str | None = Field(None, description="Filter ID")
|
||||
name: str | None = Field(None, description="Filter display name")
|
||||
filter_type: str | None = Field(
|
||||
None, description="Filter type (e.g. filter_select, filter_range)"
|
||||
)
|
||||
targets: List[Dict[str, Any]] = Field(
|
||||
default_factory=list,
|
||||
description="Filter targets (column name and dataset ID)",
|
||||
)
|
||||
|
||||
|
||||
class DashboardChartSummary(BaseModel):
|
||||
"""Lightweight chart representation for dashboard context.
|
||||
|
||||
Contains only the fields needed for LLMs to understand which charts
|
||||
are on a dashboard, omitting verbose fields like form_data, tags,
|
||||
owners, and timestamps that bloat the response.
|
||||
"""
|
||||
|
||||
id: int | None = Field(None, description="Chart ID")
|
||||
slice_name: str | None = Field(None, description="Chart name")
|
||||
viz_type: str | None = Field(None, description="Visualization type")
|
||||
datasource_name: str | None = Field(None, description="Datasource name")
|
||||
url: str | None = Field(None, description="Chart explore page URL")
|
||||
description: str | None = Field(None, description="Chart description")
|
||||
|
||||
|
||||
class DashboardInfo(BaseModel):
|
||||
id: int | None = None
|
||||
dashboard_title: str | None = None
|
||||
@@ -301,8 +339,6 @@ class DashboardInfo(BaseModel):
|
||||
css: str | None = None
|
||||
certified_by: str | None = None
|
||||
certification_details: str | None = None
|
||||
json_metadata: str | None = None
|
||||
position_json: str | None = None
|
||||
published: bool | None = None
|
||||
is_managed_externally: bool | None = None
|
||||
external_url: str | None = None
|
||||
@@ -318,7 +354,32 @@ class DashboardInfo(BaseModel):
|
||||
owners: List[UserInfo] = Field(default_factory=list)
|
||||
tags: List[TagInfo] = Field(default_factory=list)
|
||||
roles: List[RoleInfo] = Field(default_factory=list)
|
||||
charts: List[ChartInfo] = Field(default_factory=list)
|
||||
charts: List[DashboardChartSummary] = Field(default_factory=list)
|
||||
|
||||
# Structured filter information extracted from json_metadata
|
||||
native_filters: List[NativeFilterSummary] = Field(
|
||||
default_factory=list,
|
||||
description=(
|
||||
"Native filters configured on this dashboard. Extracted from "
|
||||
"json_metadata for LLM consumption — includes filter name, type, "
|
||||
"and target columns."
|
||||
),
|
||||
)
|
||||
cross_filters_enabled: bool | None = Field(
|
||||
None,
|
||||
description="Whether cross-filtering between charts is enabled.",
|
||||
)
|
||||
|
||||
# Omission metadata — tells the agent what was stripped and why
|
||||
omitted_fields: Dict[str, str] = Field(
|
||||
default_factory=dict,
|
||||
description=(
|
||||
"Fields omitted from this response to reduce size. Keys are field "
|
||||
"names, values describe what was omitted and how to access the full "
|
||||
"data. Useful filter information has been extracted into "
|
||||
"native_filters and cross_filters_enabled above."
|
||||
),
|
||||
)
|
||||
|
||||
# Fields for permalink/filter state support
|
||||
permalink_key: str | None = Field(
|
||||
@@ -458,6 +519,121 @@ class GenerateDashboardResponse(BaseModel):
|
||||
error: str | None = Field(None, description="Error message, if creation failed")
|
||||
|
||||
|
||||
def _parse_json_metadata(json_metadata_str: str | None) -> Dict[str, Any] | None:
|
||||
"""Parse json_metadata string into a dict, returning None on any failure.
|
||||
|
||||
Handles None/empty input, invalid JSON, and non-dict JSON values
|
||||
(e.g. ``"[]"``, ``"123"``) by returning None instead of raising.
|
||||
"""
|
||||
if not json_metadata_str:
|
||||
return None
|
||||
try:
|
||||
metadata = json_loads(json_metadata_str)
|
||||
except (ValueError, TypeError):
|
||||
return None
|
||||
if not isinstance(metadata, dict):
|
||||
return None
|
||||
return metadata
|
||||
|
||||
|
||||
def _extract_native_filters(json_metadata_str: str | None) -> List[NativeFilterSummary]:
|
||||
"""Extract native filter summaries from raw json_metadata string.
|
||||
|
||||
Parses the json_metadata JSON blob and pulls out only the filter
|
||||
name, type, and targets — dropping verbose fields like controlValues,
|
||||
defaultDataMask, scope, and cascadeParentIds.
|
||||
"""
|
||||
metadata = _parse_json_metadata(json_metadata_str)
|
||||
if metadata is None:
|
||||
return []
|
||||
|
||||
native_filters = metadata.get("native_filter_configuration", [])
|
||||
if not isinstance(native_filters, list):
|
||||
return []
|
||||
|
||||
summaries: List[NativeFilterSummary] = []
|
||||
for f in native_filters:
|
||||
if not isinstance(f, dict):
|
||||
continue
|
||||
raw_targets = f.get("targets", [])
|
||||
if not isinstance(raw_targets, list):
|
||||
raw_targets = []
|
||||
targets = [t for t in raw_targets if isinstance(t, dict)]
|
||||
summaries.append(
|
||||
NativeFilterSummary(
|
||||
id=f.get("id"),
|
||||
name=f.get("name"),
|
||||
filter_type=f.get("filterType"),
|
||||
targets=targets,
|
||||
)
|
||||
)
|
||||
return summaries
|
||||
|
||||
|
||||
def _extract_cross_filters_enabled(json_metadata_str: str | None) -> bool | None:
|
||||
"""Extract the cross_filters_enabled flag from json_metadata."""
|
||||
metadata = _parse_json_metadata(json_metadata_str)
|
||||
if metadata is None:
|
||||
return None
|
||||
cross_filters = metadata.get("cross_filters_enabled")
|
||||
if isinstance(cross_filters, bool):
|
||||
return cross_filters
|
||||
return None
|
||||
|
||||
|
||||
def _build_omitted_fields(
|
||||
json_metadata_str: str | None, position_json_str: str | None
|
||||
) -> Dict[str, str]:
|
||||
"""Build omission metadata describing which fields were stripped and why.
|
||||
|
||||
Uses the shared OmittedFieldsBuilder utility so the pattern is consistent
|
||||
across all MCP tool serializers.
|
||||
"""
|
||||
from superset.mcp_service.utils.response_utils import OmittedFieldsBuilder
|
||||
|
||||
return (
|
||||
OmittedFieldsBuilder()
|
||||
.add_raw_field(
|
||||
"position_json",
|
||||
raw_value=position_json_str,
|
||||
reason=(
|
||||
"Internal layout tree with component positions/hierarchy. "
|
||||
"Not useful for analysis or LLM context."
|
||||
),
|
||||
)
|
||||
.add_extracted_field(
|
||||
"json_metadata",
|
||||
raw_value=json_metadata_str,
|
||||
reason=(
|
||||
"native_filters and cross_filters_enabled extracted into "
|
||||
"dedicated fields above."
|
||||
),
|
||||
)
|
||||
.build()
|
||||
)
|
||||
|
||||
|
||||
def serialize_chart_summary(chart: Any) -> DashboardChartSummary | None:
|
||||
"""Serialize a chart to a lightweight summary for dashboard context."""
|
||||
if not chart:
|
||||
return None
|
||||
from superset.mcp_service.utils.url_utils import get_superset_base_url
|
||||
|
||||
chart_id = getattr(chart, "id", None)
|
||||
chart_url = None
|
||||
if chart_id is not None:
|
||||
chart_url = f"{get_superset_base_url()}/explore/?slice_id={chart_id}"
|
||||
|
||||
return DashboardChartSummary(
|
||||
id=chart_id,
|
||||
slice_name=getattr(chart, "slice_name", None),
|
||||
viz_type=getattr(chart, "viz_type", None),
|
||||
datasource_name=getattr(chart, "datasource_name", None),
|
||||
url=chart_url,
|
||||
description=getattr(chart, "description", None),
|
||||
)
|
||||
|
||||
|
||||
def dashboard_serializer(dashboard: "Dashboard") -> DashboardInfo:
|
||||
from superset.mcp_service.utils.url_utils import get_superset_base_url
|
||||
|
||||
@@ -473,8 +649,6 @@ def dashboard_serializer(dashboard: "Dashboard") -> DashboardInfo:
|
||||
css=dashboard.css,
|
||||
certified_by=dashboard.certified_by,
|
||||
certification_details=dashboard.certification_details,
|
||||
json_metadata=dashboard.json_metadata,
|
||||
position_json=dashboard.position_json,
|
||||
published=dashboard.published,
|
||||
is_managed_externally=dashboard.is_managed_externally,
|
||||
external_url=dashboard.external_url,
|
||||
@@ -491,6 +665,16 @@ def dashboard_serializer(dashboard: "Dashboard") -> DashboardInfo:
|
||||
created_on_humanized=dashboard.created_on_humanized,
|
||||
changed_on_humanized=dashboard.changed_on_humanized,
|
||||
chart_count=len(dashboard.slices) if dashboard.slices else 0,
|
||||
native_filters=_extract_native_filters(
|
||||
getattr(dashboard, "json_metadata", None)
|
||||
),
|
||||
cross_filters_enabled=_extract_cross_filters_enabled(
|
||||
getattr(dashboard, "json_metadata", None)
|
||||
),
|
||||
omitted_fields=_build_omitted_fields(
|
||||
getattr(dashboard, "json_metadata", None),
|
||||
getattr(dashboard, "position_json", None),
|
||||
),
|
||||
owners=[
|
||||
info
|
||||
for owner in dashboard.owners
|
||||
@@ -509,7 +693,11 @@ def dashboard_serializer(dashboard: "Dashboard") -> DashboardInfo:
|
||||
]
|
||||
if dashboard.roles
|
||||
else [],
|
||||
charts=[serialize_chart_object(chart) for chart in dashboard.slices]
|
||||
charts=[
|
||||
summary
|
||||
for chart in dashboard.slices
|
||||
if (summary := serialize_chart_summary(chart)) is not None
|
||||
]
|
||||
if dashboard.slices
|
||||
else [],
|
||||
)
|
||||
@@ -529,6 +717,9 @@ def serialize_dashboard_object(dashboard: Any) -> DashboardInfo:
|
||||
f"{get_superset_base_url()}/superset/dashboard/{slug or dashboard_id}/"
|
||||
)
|
||||
|
||||
json_metadata_str = getattr(dashboard, "json_metadata", None)
|
||||
position_json_str = getattr(dashboard, "position_json", None)
|
||||
|
||||
return DashboardInfo(
|
||||
id=dashboard_id,
|
||||
dashboard_title=getattr(dashboard, "dashboard_title", None),
|
||||
@@ -545,8 +736,9 @@ def serialize_dashboard_object(dashboard: Any) -> DashboardInfo:
|
||||
css=getattr(dashboard, "css", None),
|
||||
certified_by=getattr(dashboard, "certified_by", None),
|
||||
certification_details=getattr(dashboard, "certification_details", None),
|
||||
json_metadata=getattr(dashboard, "json_metadata", None),
|
||||
position_json=getattr(dashboard, "position_json", None),
|
||||
native_filters=_extract_native_filters(json_metadata_str),
|
||||
cross_filters_enabled=_extract_cross_filters_enabled(json_metadata_str),
|
||||
omitted_fields=_build_omitted_fields(json_metadata_str, position_json_str),
|
||||
is_managed_externally=getattr(dashboard, "is_managed_externally", None),
|
||||
external_url=getattr(dashboard, "external_url", None),
|
||||
uuid=str(getattr(dashboard, "uuid", ""))
|
||||
@@ -572,7 +764,9 @@ def serialize_dashboard_object(dashboard: Any) -> DashboardInfo:
|
||||
if getattr(dashboard, "roles", None)
|
||||
else [],
|
||||
charts=[
|
||||
serialize_chart_object(chart) for chart in getattr(dashboard, "slices", [])
|
||||
summary
|
||||
for chart in getattr(dashboard, "slices", [])
|
||||
if (summary := serialize_chart_summary(chart)) is not None
|
||||
]
|
||||
if getattr(dashboard, "slices", None)
|
||||
else [],
|
||||
|
||||
@@ -33,7 +33,6 @@ from superset.commands.exceptions import CommandException
|
||||
from superset.extensions import event_logger
|
||||
from superset.mcp_service.app import mcp
|
||||
from superset.mcp_service.auth import mcp_auth_hook
|
||||
from superset.mcp_service.chart.schemas import serialize_chart_object
|
||||
from superset.mcp_service.dashboard.constants import (
|
||||
generate_id,
|
||||
GRID_COLUMN_COUNT,
|
||||
@@ -43,6 +42,7 @@ from superset.mcp_service.dashboard.schemas import (
|
||||
AddChartToDashboardRequest,
|
||||
AddChartToDashboardResponse,
|
||||
DashboardInfo,
|
||||
serialize_chart_summary,
|
||||
)
|
||||
from superset.mcp_service.utils.url_utils import get_superset_base_url
|
||||
from superset.utils import json
|
||||
@@ -528,7 +528,7 @@ def add_chart_to_existing_dashboard(
|
||||
charts=[
|
||||
obj
|
||||
for chart in getattr(updated_dashboard, "slices", [])
|
||||
if (obj := serialize_chart_object(chart)) is not None
|
||||
if (obj := serialize_chart_summary(chart)) is not None
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
@@ -31,7 +31,6 @@ from mcp.types import ToolAnnotations
|
||||
from superset.extensions import event_logger
|
||||
from superset.mcp_service.app import mcp
|
||||
from superset.mcp_service.auth import mcp_auth_hook
|
||||
from superset.mcp_service.chart.schemas import serialize_chart_object
|
||||
from superset.mcp_service.dashboard.constants import (
|
||||
generate_id,
|
||||
GRID_COLUMN_COUNT,
|
||||
@@ -397,6 +396,7 @@ def generate_dashboard( # noqa: C901
|
||||
|
||||
# Convert to our response format
|
||||
from superset.mcp_service.dashboard.schemas import (
|
||||
serialize_chart_summary,
|
||||
serialize_tag_object,
|
||||
serialize_user_object,
|
||||
)
|
||||
@@ -428,7 +428,7 @@ def generate_dashboard( # noqa: C901
|
||||
charts=[
|
||||
obj
|
||||
for chart in getattr(dashboard, "slices", [])
|
||||
if (obj := serialize_chart_object(chart)) is not None
|
||||
if (obj := serialize_chart_summary(chart)) is not None
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
@@ -44,8 +44,6 @@ SENSITIVE_FIELDS = {
|
||||
"created_by_fk", # Internal user references
|
||||
},
|
||||
"dashboard": {
|
||||
"json_metadata", # May contain sensitive configuration
|
||||
"position_json", # Internal layout data
|
||||
"css", # May contain sensitive styling info
|
||||
"changed_by_fk", # Internal user references
|
||||
"created_by_fk", # Internal user references
|
||||
@@ -64,8 +62,6 @@ SENSITIVE_FIELD_PERMISSIONS = {
|
||||
"database_id": "can_this_form_get", # Database access permissions
|
||||
"query_context": "can_explore_json", # Explore permissions
|
||||
"cache_key": "can_warm_up_cache", # Cache management permissions
|
||||
"json_metadata": "can_this_form_get", # Advanced dashboard permissions
|
||||
"position_json": "can_this_form_get", # Dashboard edit permissions
|
||||
"css": "can_this_form_get", # Dashboard styling permissions
|
||||
}
|
||||
|
||||
|
||||
152
superset/mcp_service/utils/response_utils.py
Normal file
152
superset/mcp_service/utils/response_utils.py
Normal file
@@ -0,0 +1,152 @@
|
||||
# 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.
|
||||
|
||||
"""
|
||||
Utilities for building MCP tool responses with explicit omission metadata.
|
||||
|
||||
When MCP tool responses strip large fields to reduce context window usage,
|
||||
the LLM agent should be told *what* was omitted and *why* — otherwise it
|
||||
cannot distinguish "field is empty" from "field was stripped for size".
|
||||
|
||||
This module provides a reusable builder for omission metadata that any
|
||||
MCP tool serializer can use.
|
||||
|
||||
Industry context (as of 2026):
|
||||
- The MCP spec has no standard for field omission signaling.
|
||||
- Silent omission is considered an anti-pattern (Grafana MCP #557).
|
||||
- Production servers (mcp-git-polite, Blockscout, Axiom) converge on
|
||||
explicit omission indicators with size hints and retrieval guidance.
|
||||
- Anthropic's "Writing Tools for Agents" blog recommends surfacing
|
||||
what was stripped so agents can decide whether to fetch full data.
|
||||
|
||||
Usage example::
|
||||
|
||||
from superset.mcp_service.utils.response_utils import OmittedFieldsBuilder
|
||||
|
||||
omitted = (
|
||||
OmittedFieldsBuilder()
|
||||
.add_raw_field(
|
||||
"position_json",
|
||||
raw_value=dashboard.position_json,
|
||||
reason="Internal layout tree — not useful for analysis.",
|
||||
)
|
||||
.add_extracted_field(
|
||||
"json_metadata",
|
||||
raw_value=dashboard.json_metadata,
|
||||
reason="native_filters and cross_filters_enabled extracted above.",
|
||||
)
|
||||
.build()
|
||||
)
|
||||
# Returns: {"position_json": "Omitted (~42 KB) — ...", ...}
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Dict
|
||||
|
||||
|
||||
def _byte_size_label(value: str | None) -> str:
|
||||
"""Return a human-readable size label for a string value."""
|
||||
if not value or not isinstance(value, str):
|
||||
return "empty"
|
||||
size_bytes = len(value.encode("utf-8", errors="replace"))
|
||||
if size_bytes < 1024:
|
||||
return f"{size_bytes} B"
|
||||
return f"{size_bytes / 1024:.0f} KB"
|
||||
|
||||
|
||||
class OmittedFieldsBuilder:
|
||||
"""Builder for constructing omission metadata dicts.
|
||||
|
||||
Produces a ``Dict[str, str]`` mapping field names to human-readable
|
||||
descriptions of what was omitted, including approximate sizes.
|
||||
|
||||
Two field types are supported:
|
||||
|
||||
- **Raw fields** (``add_raw_field``): The field was stripped entirely
|
||||
with no replacement. The agent has no way to access this data
|
||||
unless a companion tool exists.
|
||||
|
||||
- **Extracted fields** (``add_extracted_field``): The raw blob was
|
||||
stripped, but useful subsets were extracted into structured fields
|
||||
on the same response object (e.g. ``native_filters`` extracted
|
||||
from ``json_metadata``).
|
||||
|
||||
All methods return ``self`` for fluent chaining.
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._fields: Dict[str, str] = {}
|
||||
|
||||
def add_raw_field(
|
||||
self,
|
||||
field_name: str,
|
||||
raw_value: str | None,
|
||||
reason: str,
|
||||
) -> "OmittedFieldsBuilder":
|
||||
"""Record a field that was stripped with no replacement.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
field_name:
|
||||
The original field name (e.g. ``"position_json"``).
|
||||
raw_value:
|
||||
The raw value that was omitted (used only to compute size).
|
||||
Pass ``None`` if the field was empty/unset.
|
||||
reason:
|
||||
Why the field was omitted, written for an LLM audience.
|
||||
"""
|
||||
size = _byte_size_label(raw_value)
|
||||
has_data = isinstance(raw_value, str) and len(raw_value) > 0
|
||||
if has_data:
|
||||
self._fields[field_name] = f"Omitted (~{size}) — {reason}"
|
||||
else:
|
||||
self._fields[field_name] = f"Omitted ({size}) — {reason}"
|
||||
return self
|
||||
|
||||
def add_extracted_field(
|
||||
self,
|
||||
field_name: str,
|
||||
raw_value: str | None,
|
||||
reason: str,
|
||||
) -> "OmittedFieldsBuilder":
|
||||
"""Record a field whose useful parts were extracted into other fields.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
field_name:
|
||||
The original raw field name (e.g. ``"json_metadata"``).
|
||||
raw_value:
|
||||
The raw value that was omitted (used only to compute size).
|
||||
reason:
|
||||
Explanation of what was extracted and where, for LLM context.
|
||||
"""
|
||||
size = _byte_size_label(raw_value)
|
||||
has_data = isinstance(raw_value, str) and len(raw_value) > 0
|
||||
if has_data:
|
||||
self._fields[field_name] = (
|
||||
f"Omitted (~{size}), useful parts extracted — {reason}"
|
||||
)
|
||||
else:
|
||||
self._fields[field_name] = (
|
||||
f"Omitted ({size}), useful parts extracted — {reason}"
|
||||
)
|
||||
return self
|
||||
|
||||
def build(self) -> Dict[str, str]:
|
||||
"""Return the omission metadata dict."""
|
||||
return dict(self._fields)
|
||||
@@ -24,7 +24,12 @@ Tests that serialize_dashboard_object correctly handles slug and other fields.
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from superset.mcp_service.dashboard.schemas import serialize_dashboard_object
|
||||
from superset.mcp_service.dashboard.schemas import (
|
||||
_extract_cross_filters_enabled,
|
||||
_extract_native_filters,
|
||||
serialize_dashboard_object,
|
||||
)
|
||||
from superset.utils.json import dumps as json_dumps
|
||||
|
||||
|
||||
def _mock_dashboard(
|
||||
@@ -117,3 +122,220 @@ class TestSerializeDashboardObject:
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert result.url == "http://localhost:8088/superset/dashboard/my-dashboard/"
|
||||
|
||||
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
|
||||
def test_no_json_metadata_or_position_json_in_response(self, mock_base_url):
|
||||
"""DashboardInfo should not contain json_metadata or position_json."""
|
||||
mock_base_url.return_value = "http://localhost:8088"
|
||||
|
||||
dashboard = _mock_dashboard(id=1)
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert not hasattr(result, "json_metadata")
|
||||
assert not hasattr(result, "position_json")
|
||||
|
||||
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
|
||||
def test_native_filters_extracted_from_json_metadata(self, mock_base_url):
|
||||
"""Native filters should be extracted from json_metadata."""
|
||||
mock_base_url.return_value = "http://localhost:8088"
|
||||
|
||||
metadata = {
|
||||
"native_filter_configuration": [
|
||||
{
|
||||
"id": "NATIVE_FILTER-abc123",
|
||||
"name": "Region Filter",
|
||||
"filterType": "filter_select",
|
||||
"targets": [{"column": {"name": "region"}, "datasetId": 10}],
|
||||
"controlValues": {"multiSelect": True},
|
||||
"defaultDataMask": {"filterState": {"value": ["US"]}},
|
||||
"scope": {"rootPath": ["ROOT_ID"]},
|
||||
},
|
||||
{
|
||||
"id": "NATIVE_FILTER-def456",
|
||||
"name": "Date Range",
|
||||
"filterType": "filter_range",
|
||||
"targets": [{"column": {"name": "order_date"}, "datasetId": 10}],
|
||||
},
|
||||
],
|
||||
"cross_filters_enabled": True,
|
||||
"color_scheme": "supersetColors",
|
||||
"shared_label_colors": {"Sales": "#1FA8C9"},
|
||||
}
|
||||
dashboard = _mock_dashboard(id=1)
|
||||
dashboard.json_metadata = json_dumps(metadata)
|
||||
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert len(result.native_filters) == 2
|
||||
assert result.native_filters[0].id == "NATIVE_FILTER-abc123"
|
||||
assert result.native_filters[0].name == "Region Filter"
|
||||
assert result.native_filters[0].filter_type == "filter_select"
|
||||
assert len(result.native_filters[0].targets) == 1
|
||||
assert result.native_filters[1].name == "Date Range"
|
||||
assert result.cross_filters_enabled is True
|
||||
|
||||
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
|
||||
def test_chart_summaries_are_lightweight(self, mock_base_url):
|
||||
"""Charts in dashboard response should only have core fields."""
|
||||
mock_base_url.return_value = "http://localhost:8088"
|
||||
|
||||
chart = MagicMock()
|
||||
chart.id = 5
|
||||
chart.slice_name = "Revenue Chart"
|
||||
chart.viz_type = "echarts_timeseries_bar"
|
||||
chart.datasource_name = "sales"
|
||||
chart.description = "Monthly revenue"
|
||||
|
||||
dashboard = _mock_dashboard(id=1, slices=[chart])
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert len(result.charts) == 1
|
||||
assert result.charts[0].id == 5
|
||||
assert result.charts[0].slice_name == "Revenue Chart"
|
||||
assert result.charts[0].viz_type == "echarts_timeseries_bar"
|
||||
assert result.charts[0].datasource_name == "sales"
|
||||
assert result.charts[0].url == "http://localhost:8088/explore/?slice_id=5"
|
||||
# Verify no heavy fields
|
||||
assert not hasattr(result.charts[0], "form_data")
|
||||
assert not hasattr(result.charts[0], "tags")
|
||||
assert not hasattr(result.charts[0], "owners")
|
||||
|
||||
|
||||
class TestExtractNativeFilters:
|
||||
"""Tests for _extract_native_filters helper."""
|
||||
|
||||
def test_none_input(self):
|
||||
assert _extract_native_filters(None) == []
|
||||
|
||||
def test_empty_string(self):
|
||||
assert _extract_native_filters("") == []
|
||||
|
||||
def test_invalid_json(self):
|
||||
assert _extract_native_filters("not json") == []
|
||||
|
||||
def test_no_filter_config(self):
|
||||
assert _extract_native_filters("{}") == []
|
||||
|
||||
def test_non_list_filter_config(self):
|
||||
assert _extract_native_filters('{"native_filter_configuration": "bad"}') == []
|
||||
|
||||
def test_valid_filters(self):
|
||||
metadata = json_dumps(
|
||||
{
|
||||
"native_filter_configuration": [
|
||||
{
|
||||
"id": "f1",
|
||||
"name": "Filter 1",
|
||||
"filterType": "filter_select",
|
||||
"targets": [{"column": {"name": "col1"}}],
|
||||
}
|
||||
]
|
||||
}
|
||||
)
|
||||
result = _extract_native_filters(metadata)
|
||||
assert len(result) == 1
|
||||
assert result[0].id == "f1"
|
||||
assert result[0].name == "Filter 1"
|
||||
assert result[0].filter_type == "filter_select"
|
||||
|
||||
def test_skips_non_dict_entries(self):
|
||||
metadata = json_dumps(
|
||||
{"native_filter_configuration": [{"id": "f1", "name": "ok"}, "bad", 123]}
|
||||
)
|
||||
result = _extract_native_filters(metadata)
|
||||
assert len(result) == 1
|
||||
|
||||
def test_non_dict_top_level_json(self):
|
||||
"""json_metadata that parses to a list/number should return empty."""
|
||||
assert _extract_native_filters("[]") == []
|
||||
assert _extract_native_filters("123") == []
|
||||
assert _extract_native_filters('"just a string"') == []
|
||||
|
||||
|
||||
class TestExtractCrossFiltersEnabled:
|
||||
"""Tests for _extract_cross_filters_enabled helper."""
|
||||
|
||||
def test_none_input(self):
|
||||
assert _extract_cross_filters_enabled(None) is None
|
||||
|
||||
def test_empty_json(self):
|
||||
assert _extract_cross_filters_enabled("{}") is None
|
||||
|
||||
def test_true(self):
|
||||
assert _extract_cross_filters_enabled('{"cross_filters_enabled": true}') is True
|
||||
|
||||
def test_false(self):
|
||||
assert (
|
||||
_extract_cross_filters_enabled('{"cross_filters_enabled": false}') is False
|
||||
)
|
||||
|
||||
def test_non_bool_value(self):
|
||||
assert (
|
||||
_extract_cross_filters_enabled('{"cross_filters_enabled": "yes"}') is None
|
||||
)
|
||||
|
||||
def test_non_dict_top_level_json(self):
|
||||
"""json_metadata that parses to a list/number should return None."""
|
||||
assert _extract_cross_filters_enabled("[]") is None
|
||||
assert _extract_cross_filters_enabled("123") is None
|
||||
assert _extract_cross_filters_enabled('"just a string"') is None
|
||||
|
||||
|
||||
class TestOmittedFieldsBuilder:
|
||||
"""Tests for the shared OmittedFieldsBuilder utility."""
|
||||
|
||||
def test_builder_basic(self):
|
||||
from superset.mcp_service.utils.response_utils import OmittedFieldsBuilder
|
||||
|
||||
result = (
|
||||
OmittedFieldsBuilder()
|
||||
.add_raw_field("big_field", "x" * 2048, "Too large for context.")
|
||||
.add_extracted_field("meta_field", "y" * 512, "Useful parts above.")
|
||||
.build()
|
||||
)
|
||||
assert "big_field" in result
|
||||
assert "~2 KB" in result["big_field"]
|
||||
assert "Too large" in result["big_field"]
|
||||
assert "meta_field" in result
|
||||
assert "extracted" in result["meta_field"]
|
||||
|
||||
def test_builder_none_values(self):
|
||||
from superset.mcp_service.utils.response_utils import OmittedFieldsBuilder
|
||||
|
||||
result = (
|
||||
OmittedFieldsBuilder()
|
||||
.add_raw_field("empty_field", None, "Was not set.")
|
||||
.add_extracted_field("also_empty", None, "Nothing to extract.")
|
||||
.build()
|
||||
)
|
||||
assert "empty" in result["empty_field"]
|
||||
assert "empty" in result["also_empty"]
|
||||
|
||||
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
|
||||
def test_omitted_fields_in_serialized_dashboard(self, mock_base_url):
|
||||
"""omitted_fields should describe what was stripped and include sizes."""
|
||||
mock_base_url.return_value = "http://localhost:8088"
|
||||
|
||||
dashboard = _mock_dashboard(id=1)
|
||||
dashboard.json_metadata = json_dumps(
|
||||
{"color_scheme": "preset", "native_filter_configuration": []}
|
||||
)
|
||||
dashboard.position_json = json_dumps({"ROOT_ID": {"children": ["GRID_ID"]}})
|
||||
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert "json_metadata" in result.omitted_fields
|
||||
assert "position_json" in result.omitted_fields
|
||||
assert "extracted" in result.omitted_fields["json_metadata"]
|
||||
assert "layout tree" in result.omitted_fields["position_json"].lower()
|
||||
|
||||
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
|
||||
def test_omitted_fields_with_none_values(self, mock_base_url):
|
||||
"""omitted_fields should still be present when raw fields are None."""
|
||||
mock_base_url.return_value = "http://localhost:8088"
|
||||
|
||||
dashboard = _mock_dashboard(id=1)
|
||||
result = serialize_dashboard_object(dashboard)
|
||||
|
||||
assert "json_metadata" in result.omitted_fields
|
||||
assert "position_json" in result.omitted_fields
|
||||
|
||||
@@ -76,7 +76,6 @@ async def test_list_dashboards_basic(mock_list, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
dashboard.uuid = "test-dashboard-uuid-1"
|
||||
@@ -143,7 +142,6 @@ async def test_list_dashboards_with_filters(mock_list, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
dashboard.uuid = None
|
||||
@@ -237,7 +235,6 @@ async def test_list_dashboards_with_search(mock_list, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
dashboard.uuid = None
|
||||
@@ -304,7 +301,6 @@ async def test_get_dashboard_info_success(mock_info, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.published = True
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
@@ -384,7 +380,6 @@ async def test_get_dashboard_info_by_uuid(mock_find_object, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = "{}"
|
||||
dashboard.position_json = "{}"
|
||||
dashboard.published = True
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
@@ -424,7 +419,6 @@ async def test_get_dashboard_info_by_slug(mock_find_object, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = "{}"
|
||||
dashboard.position_json = "{}"
|
||||
dashboard.published = True
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
@@ -476,7 +470,6 @@ async def test_list_dashboards_custom_uuid_slug_columns(mock_list, mcp_server):
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
dashboard.thumbnail_url = None
|
||||
@@ -543,8 +536,8 @@ class TestDashboardDefaultColumnFiltering:
|
||||
# Heavy columns should NOT be in defaults
|
||||
assert "charts" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "published" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "json_metadata" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "position_json" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "native_filters" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "cross_filters_enabled" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
assert "uuid" not in DASHBOARD_DEFAULT_COLUMNS
|
||||
|
||||
def test_empty_select_columns_default(self):
|
||||
@@ -588,7 +581,6 @@ class TestDashboardDefaultColumnFiltering:
|
||||
dashboard.certified_by = None
|
||||
dashboard.certification_details = None
|
||||
dashboard.json_metadata = None
|
||||
dashboard.position_json = None
|
||||
dashboard.is_managed_externally = False
|
||||
dashboard.external_url = None
|
||||
dashboard.thumbnail_url = None
|
||||
@@ -611,8 +603,8 @@ class TestDashboardDefaultColumnFiltering:
|
||||
assert "changed_on_humanized" in data["columns_requested"]
|
||||
|
||||
# Verify heavy columns are NOT in columns_loaded by default
|
||||
assert "json_metadata" not in data["columns_loaded"]
|
||||
assert "position_json" not in data["columns_loaded"]
|
||||
assert "native_filters" not in data["columns_loaded"]
|
||||
assert "cross_filters_enabled" not in data["columns_loaded"]
|
||||
|
||||
|
||||
class TestDashboardSortableColumns:
|
||||
|
||||
Reference in New Issue
Block a user