diff --git a/superset/mcp_service/common/schema_discovery.py b/superset/mcp_service/common/schema_discovery.py index dda1a0d5412c..2fd13bc8314e 100644 --- a/superset/mcp_service/common/schema_discovery.py +++ b/superset/mcp_service/common/schema_discovery.py @@ -169,9 +169,9 @@ def _get_sqlalchemy_type_name(col_type: Any) -> 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", } diff --git a/superset/mcp_service/dashboard/schemas.py b/superset/mcp_service/dashboard/schemas.py index e76cde368271..e61573dd7ad1 100644 --- a/superset/mcp_service/dashboard/schemas.py +++ b/superset/mcp_service/dashboard/schemas.py @@ -28,7 +28,7 @@ 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 @@ from __future__ import annotations +import logging from datetime import datetime from typing import Annotated, Any, Dict, List, Literal, TYPE_CHECKING @@ -83,7 +84,6 @@ 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 ( @@ -97,6 +97,7 @@ _remove_dangerous_unicode, _strip_html_tags, ) +from superset.utils.json import loads as json_loads class DashboardError(BaseModel): @@ -298,6 +299,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 @@ -306,8 +344,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 @@ -323,7 +359,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( @@ -473,6 +534,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 @@ -488,8 +664,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, @@ -506,6 +680,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 @@ -524,7 +708,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 [], ) @@ -551,6 +739,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), @@ -571,8 +762,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", "")) @@ -599,7 +791,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 [], diff --git a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py index 888f2423fcc3..d3d9293ce5b3 100644 --- a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py +++ b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py @@ -31,7 +31,6 @@ from superset.commands.exceptions import CommandException from superset.extensions import event_logger -from superset.mcp_service.chart.schemas import serialize_chart_object from superset.mcp_service.dashboard.constants import ( generate_id, GRID_COLUMN_COUNT, @@ -41,6 +40,7 @@ AddChartToDashboardRequest, AddChartToDashboardResponse, DashboardInfo, + serialize_chart_summary, ) from superset.mcp_service.utils.url_utils import get_superset_base_url from superset.utils import json @@ -525,7 +525,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 ], ) diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py b/superset/mcp_service/dashboard/tool/generate_dashboard.py index 1b0f457771f8..1cec383f6435 100644 --- a/superset/mcp_service/dashboard/tool/generate_dashboard.py +++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py @@ -29,7 +29,6 @@ from superset_core.mcp.decorators import tool, ToolAnnotations from superset.extensions import event_logger -from superset.mcp_service.chart.schemas import serialize_chart_object from superset.mcp_service.dashboard.constants import ( generate_id, GRID_COLUMN_COUNT, @@ -395,6 +394,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, ) @@ -426,7 +426,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 ], ) diff --git a/superset/mcp_service/utils/permissions_utils.py b/superset/mcp_service/utils/permissions_utils.py index a3864206532e..62d9c3d08404 100644 --- a/superset/mcp_service/utils/permissions_utils.py +++ b/superset/mcp_service/utils/permissions_utils.py @@ -44,8 +44,6 @@ "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 @@ "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 } diff --git a/superset/mcp_service/utils/response_utils.py b/superset/mcp_service/utils/response_utils.py new file mode 100644 index 000000000000..777c619cfb2b --- /dev/null +++ b/superset/mcp_service/utils/response_utils.py @@ -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) diff --git a/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py b/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py index d5ace35054d7..98fe464a24fe 100644 --- a/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py +++ b/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py @@ -24,7 +24,12 @@ 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 @@ def test_url_uses_slug_when_available(self, mock_base_url): 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 diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py index fea9a05e3f1c..21d301492e19 100644 --- a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py +++ b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py @@ -75,7 +75,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" @@ -142,7 +141,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 @@ -236,7 +234,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 @@ -303,7 +300,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 @@ -383,7 +379,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 @@ -423,7 +418,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 @@ -475,7 +469,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 @@ -542,8 +535,8 @@ def test_minimal_default_columns_constant(self): # 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): @@ -587,7 +580,6 @@ async def test_default_columns_in_response(self, 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 @@ -610,8 +602,8 @@ async def test_default_columns_in_response(self, mock_list, mcp_server): 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: