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 342a8f1e4e0 fix(mcp): report a chart's live dataset id and name
(#44681)
342a8f1e4e0 is described below
commit 342a8f1e4e0361833a42cabc907fb92af5e0bbdc
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Fri Oct 2 13:19:32 2026 +1000
fix(mcp): report a chart's live dataset id and name (#44681)
---
docs/admin_docs/configuration/mcp-server.mdx | 17 +
superset/mcp_service/chart/schemas.py | 61 +-
superset/mcp_service/chart/tool/get_chart_info.py | 18 +-
superset/mcp_service/chart/tool/get_chart_sql.py | 5 +-
superset/mcp_service/chart/tool/list_charts.py | 118 +++-
superset/mcp_service/common/schema_discovery.py | 2 +-
superset/mcp_service/dashboard/schemas.py | 29 +-
superset/mcp_service/privacy.py | 1 +
.../chart/test_live_chart_datasource.py | 709 +++++++++++++++++++++
.../dashboard/test_dashboard_schemas.py | 18 +
10 files changed, 959 insertions(+), 19 deletions(-)
diff --git a/docs/admin_docs/configuration/mcp-server.mdx
b/docs/admin_docs/configuration/mcp-server.mdx
index 6c2c6889244..84119429973 100644
--- a/docs/admin_docs/configuration/mcp-server.mdx
+++ b/docs/admin_docs/configuration/mcp-server.mdx
@@ -1195,6 +1195,23 @@ while page["next_cursor"]:
)
```
+## Chart datasource metadata
+
+For roles allowed to view data-model metadata, `get_chart_info` includes
+`datasource_id` in its default columns. Dashboard chart summaries include
+`datasource_type` alongside `datasource_id`, which is only unique within its
type.
+Both fields are redacted for users without data-model metadata access.
+Chart and dashboard chart summaries resolve dataset names from the live table
+or semantic view, so renames are reflected without resaving the chart. Query
and
+saved-query charts retain their stored datasource names; missing tables or
+semantic views do not fall back to stale names. Dataset-access validation
remains
+unchanged.
+
+`list_charts` filters on `datasource_name` use these same live names before
+pagination and counting. For tables with a schema, filter using
+`schema.table_name`. Data-model metadata permission checks also apply to these
+filters and returned identifiers.
+
## Trash Listing (Soft-Deleted Objects)
When the `SOFT_DELETE` feature flag is enabled, deleting a chart or dashboard
moves it to trash rather than removing it outright. The `list_charts` and
`list_dashboards` tools accept a `deleted_state` parameter to surface those
trashed objects:
diff --git a/superset/mcp_service/chart/schemas.py
b/superset/mcp_service/chart/schemas.py
index 992a1d7d2ad..9b14c9ab8c2 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -95,6 +95,7 @@ class ChartLike(Protocol):
id: int
slice_name: str | None
viz_type: str | None
+ datasource_id: int | None
datasource_name: str | None
datasource_type: str | None
url: str | None
@@ -131,7 +132,16 @@ class ChartInfo(BaseModel):
"fall back to viz_type when this field is null."
),
)
- datasource_name: str | None = Field(None, description="Datasource name")
+ datasource_id: int | None = Field(
+ None, description="ID of the dataset (or semantic view) the chart
queries"
+ )
+ datasource_name: str | None = Field(
+ None,
+ description=(
+ "Current name of the dataset (or semantic view) the chart queries,
"
+ "resolved from the live datasource"
+ ),
+ )
datasource_type: str | None = Field(None, description="Datasource type")
url: str | None = Field(None, description="Chart explore page URL")
description: str | None = Field(None, description="Chart description")
@@ -304,6 +314,7 @@ class VersionedResponse(BaseModel):
DEFAULT_GET_CHART_INFO_COLUMNS: List[str] = [
"id",
+ "datasource_id",
"slice_name",
"viz_type",
"datasource_name",
@@ -552,7 +563,44 @@ CHART_FORM_DATA_EXCLUDED_FIELD_NAMES = frozenset(
)
-def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
+def resolve_chart_datasource_id(chart: Any) -> int | None:
+ """Return the ID of the datasource a chart is joined to."""
+ datasource_id = getattr(chart, "datasource_id", None)
+ if isinstance(datasource_id, int) and not isinstance(datasource_id, bool):
+ return datasource_id
+ return None
+
+
+def resolve_chart_datasource_name(chart: Any) -> str | None:
+ """Return the chart's datasource name, read from the live datasource.
+
+ ``Slice.datasource_name`` is a stored, denormalized column that is not
+ refreshed when a dataset is renamed or a chart is re-pointed, so it can
+ name a table the chart no longer queries. ``Slice.datasource_name_text``
+ resolves the name through the type-guarded ``table`` / ``semantic_view``
+ relationships instead, and yields ``None`` when the datasource no longer
+ exists. Query and saved-query charts, and objects without that resolver
+ (row tuples, lightweight stand-ins), fall back to the stored value.
+ """
+ resolver = getattr(chart, "datasource_name_text", None)
+ if callable(resolver) and getattr(chart, "datasource_type", None) not in {
+ "query",
+ "saved_query",
+ }:
+ live_name = resolver()
+ if live_name is None or isinstance(live_name, str):
+ return live_name
+ stored_name = getattr(chart, "datasource_name", None)
+ return stored_name if isinstance(stored_name, str) else None
+
+
+def serialize_chart_object(
+ chart: ChartLike | None, *, select_columns: list[str] | None = None
+) -> ChartInfo | None:
+ """Serialize a chart, loading collection relationships only when requested.
+
+ Omitting ``select_columns`` preserves full-object serialization.
+ """
if not chart:
return None
@@ -598,7 +646,8 @@ def serialize_chart_object(chart: ChartLike | None) ->
ChartInfo | None:
slice_name=getattr(chart, "slice_name", None),
viz_type=_viz_type,
chart_type_display_name=_display_name,
- datasource_name=getattr(chart, "datasource_name", None),
+ datasource_id=resolve_chart_datasource_id(chart),
+ datasource_name=resolve_chart_datasource_name(chart),
datasource_type=getattr(chart, "datasource_type", None),
url=chart_url,
description=getattr(chart, "description", None),
@@ -617,14 +666,16 @@ def serialize_chart_object(chart: ChartLike | None) ->
ChartInfo | None:
TagInfo.model_validate(tag, from_attributes=True)
for tag in getattr(chart, "tags", [])
]
- if getattr(chart, "tags", None)
+ if (select_columns is None or "tags" in select_columns)
+ and getattr(chart, "tags", None)
else [],
editors=[
info
for editor in getattr(chart, "editors", [])
if (info := serialize_subject_object(editor)) is not None
]
- if getattr(chart, "editors", None)
+ if (select_columns is None or "editors" in select_columns)
+ and getattr(chart, "editors", None)
else [],
)
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py
b/superset/mcp_service/chart/tool/get_chart_info.py
index 7dc5f41a947..b2728a9b13f 100644
--- a/superset/mcp_service/chart/tool/get_chart_info.py
+++ b/superset/mcp_service/chart/tool/get_chart_info.py
@@ -163,8 +163,8 @@ def _permalink_form_data(permalink: ExplorePermalinkValue)
-> dict[str, Any]:
def _permalink_datasource(
permalink: ExplorePermalinkValue,
-) -> tuple[str | None, str | None]:
- """Return the (name, type) of the datasource a permalink was built on.
+) -> tuple[int | None, str | None, str | None]:
+ """Return the (id, name, type) of the datasource a permalink was built on.
A permalink's form_data carries the datasource as an opaque "<id>__<type>"
string, so the name has to be resolved from the ids the permalink stores
@@ -174,7 +174,7 @@ def _permalink_datasource(
datasource_type = permalink.get("datasourceType") or
DatasourceType.TABLE.value
datasource_id = permalink.get("datasourceId") or permalink.get("datasetId")
if not datasource_id:
- return None, str(datasource_type)
+ return None, None, str(datasource_type)
try:
from superset.daos.datasource import DatasourceDAO
@@ -190,21 +190,27 @@ def _permalink_datasource(
datasource_id,
datasource_type,
)
- return None, str(datasource_type)
+ return None, None, str(datasource_type)
# A SQL Lab query labels itself with ``name``; datasets use
# ``datasource_name``.
name = getattr(datasource, "datasource_name", None) or getattr(
datasource, "name", None
)
- return name, str(datasource_type)
+ resolved_id = getattr(datasource, "id", None)
+ return (
+ resolved_id if isinstance(resolved_id, int) else None,
+ name,
+ str(datasource_type),
+ )
def _build_permalink_chart_info(
permalink_key: str, permalink: ExplorePermalinkValue, form_data: dict[str,
Any]
) -> ChartInfo:
"""Build a ChartInfo from a permalink that is not tied to a saved chart."""
- datasource_name, datasource_type = _permalink_datasource(permalink)
+ datasource_id, datasource_name, datasource_type =
_permalink_datasource(permalink)
result = ChartInfo(
+ datasource_id=datasource_id,
datasource_name=datasource_name,
datasource_type=datasource_type,
form_data=form_data,
diff --git a/superset/mcp_service/chart/tool/get_chart_sql.py
b/superset/mcp_service/chart/tool/get_chart_sql.py
index d441b3a02df..76871028909 100644
--- a/superset/mcp_service/chart/tool/get_chart_sql.py
+++ b/superset/mcp_service/chart/tool/get_chart_sql.py
@@ -49,6 +49,7 @@ from superset.mcp_service.chart.schemas import (
ChartError,
ChartSql,
GetChartSqlRequest,
+ resolve_chart_datasource_name,
)
logger = logging.getLogger(__name__)
@@ -247,7 +248,7 @@ def _sql_from_saved_query_context(
result,
chart.id,
chart.slice_name,
- chart.datasource_name,
+ resolve_chart_datasource_name(chart),
extra_form_data=extra_form_data,
)
except SupersetSecurityException:
@@ -266,7 +267,7 @@ def _resolve_datasource_name(
from form_data so that the response includes a meaningful name.
"""
if chart:
- return getattr(chart, "datasource_name", None)
+ return resolve_chart_datasource_name(chart)
# Unsaved chart — resolve from form_data
datasource_id = form_data.get("datasource_id")
diff --git a/superset/mcp_service/chart/tool/list_charts.py
b/superset/mcp_service/chart/tool/list_charts.py
index 09185249ba9..4a41df1c8d2 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 cast, TYPE_CHECKING
+from typing import Any, cast, TYPE_CHECKING
from fastmcp import Context
+from sqlalchemy import case, select
from superset_core.mcp.decorators import tool, ToolAnnotations
if TYPE_CHECKING:
@@ -63,6 +64,109 @@ DEFAULT_CHART_COLUMNS = [
_DEFAULT_LIST_CHARTS_REQUEST = ListChartsRequest()
+# Relationships that resolve a chart's live datasource. ``datasource_name`` is
+# read through them rather than from the stored ``Slice.datasource_name``
+# column, which can be stale after a dataset rename or a chart re-point.
+_LIVE_DATASOURCE_RELATIONSHIPS = ("table", "semantic_view")
+
+
+class _LiveDatasourceNameFilter:
+ """Apply name operators to the same type-guarded datasource as
serialization."""
+
+ def __init__(self, filters: list[ChartFilter]) -> None:
+ """Bind the validated name filters for the DAO invocation."""
+ self.filters = filters
+
+ def apply(self, query: Any, value: Any) -> Any:
+ """Filter in SQL so counts and pagination reflect the live names."""
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.slice import Slice
+ from superset.semantic_layers.models import SemanticView
+
+ # Keep this SQL expression aligned with SqlaTable.name, which supplies
+ # the table name used by resolve_chart_datasource_name during
serialization.
+ table_name = case(
+ (
+ (SqlaTable.schema.isnot(None)) & (SqlaTable.schema != ""),
+ SqlaTable.schema + "." + SqlaTable.table_name,
+ ),
+ else_=SqlaTable.table_name,
+ )
+ live_name = case(
+ (
+ Slice.datasource_type == "table",
+ select(table_name)
+ .where(SqlaTable.id == Slice.datasource_id)
+ .correlate(Slice)
+ .scalar_subquery(),
+ ),
+ (
+ Slice.datasource_type == "semantic_view",
+ select(SemanticView.name)
+ .where(SemanticView.id == Slice.datasource_id)
+ .correlate(Slice)
+ .scalar_subquery(),
+ ),
+ (
+ Slice.datasource_type.in_(["query", "saved_query"]),
+ Slice.datasource_name,
+ ),
+ else_=None,
+ )
+ for name_filter in self.filters:
+ query = query.filter(name_filter.opr.apply(live_name,
name_filter.value))
+ return query
+
+
+class _ChartListCore(ModelListCore[ChartList]):
+ """List core that loads the live datasource whenever its name is requested.
+
+ Requesting only plain columns makes the DAO return row tuples, which cannot
+ reach the ``table`` / ``semantic_view`` relationships. Adding them to the
+ DAO load list switches it to full model instances with the relationships
+ eager-loaded, so the live name costs no per-chart query.
+ """
+
+ def _call_dao_list(
+ self,
+ filters: Any,
+ order_column: str,
+ order_direction: str,
+ page: int,
+ page_size: int,
+ search: str | None,
+ columns_to_load: list[str],
+ custom_filters: dict[str, Any] | None = None,
+ ) -> tuple[list[Any], int]:
+ name_filters = [
+ item for item in (filters or []) if item.col == "datasource_name"
+ ]
+ if name_filters:
+ filters = [item for item in filters if item.col !=
"datasource_name"]
+ custom_filters = {
+ **(custom_filters or {}),
+ "live_datasource_name":
_LiveDatasourceNameFilter(name_filters),
+ }
+ if "datasource_name" in columns_to_load:
+ columns_to_load = [
+ *columns_to_load,
+ *(
+ rel
+ for rel in _LIVE_DATASOURCE_RELATIONSHIPS
+ if rel not in columns_to_load
+ ),
+ ]
+ return super()._call_dao_list(
+ filters=filters,
+ order_column=order_column,
+ order_direction=order_direction,
+ page=page,
+ page_size=page_size,
+ search=search,
+ columns_to_load=columns_to_load,
+ custom_filters=custom_filters,
+ )
+
@tool(
tags=["core"],
@@ -161,9 +265,9 @@ async def list_charts(
obj: "Slice | None", cols: list[str] | None
) -> ChartInfo | None:
"""Serialize chart object (field filtering handled by
model_serializer)."""
- return serialize_chart_object(cast(ChartLike | None, obj))
+ return serialize_chart_object(cast(ChartLike | None, obj),
select_columns=cols)
- tool = ModelListCore(
+ tool = _ChartListCore(
dao_class=ChartDAO,
output_schema=ChartInfo,
item_serializer=_serialize_chart,
@@ -214,6 +318,14 @@ async def list_charts(
# Always use columns_requested (either explicit select_columns or
defaults)
# This triggers ChartInfo._filter_fields_by_context for each chart
columns_to_filter = result.columns_requested
+ # Report only fields that the serialized charts actually carry: drop
+ # model columns ChartInfo does not expose (e.g. params, perm) and
+ # dependencies loaded solely to compute another field.
+ result.columns_loaded = [
+ column
+ for column in result.columns_loaded
+ if column in columns_to_filter and column in ChartInfo.model_fields
+ ]
await ctx.debug(
"Applying field filtering via serialization context: columns=%s"
% (columns_to_filter,)
diff --git a/superset/mcp_service/common/schema_discovery.py
b/superset/mcp_service/common/schema_discovery.py
index 9e1ee130a8f..e6229677f99 100644
--- a/superset/mcp_service/common/schema_discovery.py
+++ b/superset/mcp_service/common/schema_discovery.py
@@ -264,7 +264,7 @@ CHART_SEARCH_COLUMNS = ["slice_name", "description"]
CHART_EXTRA_COLUMNS: dict[str, ColumnMetadata] = {
"datasource_name": ColumnMetadata(
name="datasource_name",
- description="Data source name",
+ description="Current name of the chart's dataset or semantic view",
type="str",
is_default=False,
),
diff --git a/superset/mcp_service/dashboard/schemas.py
b/superset/mcp_service/dashboard/schemas.py
index 82073216082..6b5d0d419a0 100644
--- a/superset/mcp_service/dashboard/schemas.py
+++ b/superset/mcp_service/dashboard/schemas.py
@@ -90,6 +90,10 @@ if TYPE_CHECKING:
from superset.daos.base import ColumnOperator, ColumnOperatorEnum
from superset.exceptions import SupersetSecurityException
+from superset.mcp_service.chart.schemas import (
+ resolve_chart_datasource_id,
+ resolve_chart_datasource_name,
+)
from superset.mcp_service.common.cache_schemas import (
CreatedByMeMixin,
EditedByMeMixin,
@@ -429,7 +433,22 @@ class DashboardChartSummary(BaseModel):
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")
+ datasource_id: int | None = Field(
+ None, description="ID of the dataset (or semantic view) the chart
queries"
+ )
+ datasource_type: str | None = Field(
+ None,
+ description=(
+ "Type of the datasource; datasource_id is only unique within this
type"
+ ),
+ )
+ datasource_name: str | None = Field(
+ None,
+ description=(
+ "Current name of the dataset (or semantic view) the chart queries,
"
+ "resolved from the live datasource"
+ ),
+ )
url: str | None = Field(None, description="Chart explore page URL")
description: str | None = Field(None, description="Chart description")
@@ -1867,7 +1886,13 @@ def serialize_chart_summary(
id=chart_id,
slice_name=getattr(chart, "slice_name", None),
viz_type=getattr(chart, "viz_type", None),
- datasource_name=getattr(chart, "datasource_name", None)
+ datasource_id=resolve_chart_datasource_id(chart)
+ if include_data_model_metadata
+ else None,
+ datasource_type=getattr(chart, "datasource_type", None)
+ if include_data_model_metadata
+ else None,
+ datasource_name=resolve_chart_datasource_name(chart)
if include_data_model_metadata
else None,
url=chart_url,
diff --git a/superset/mcp_service/privacy.py b/superset/mcp_service/privacy.py
index a007772ad69..a82d5115e33 100644
--- a/superset/mcp_service/privacy.py
+++ b/superset/mcp_service/privacy.py
@@ -183,6 +183,7 @@ def redact_chart_data_model_fields(chart_info: Any) -> Any:
if isinstance(chart_info, ChartInfo):
return chart_info.model_copy(
update={
+ "datasource_id": None,
"datasource_name": None,
"datasource_type": None,
"filters": None,
diff --git a/tests/unit_tests/mcp_service/chart/test_live_chart_datasource.py
b/tests/unit_tests/mcp_service/chart/test_live_chart_datasource.py
new file mode 100644
index 00000000000..72a85a25898
--- /dev/null
+++ b/tests/unit_tests/mcp_service/chart/test_live_chart_datasource.py
@@ -0,0 +1,709 @@
+# 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.
+"""Chart responses report the chart's live dataset, not its stored name.
+
+``Slice.datasource_name`` is a denormalized column that is not refreshed when
+a dataset is renamed or a chart is re-pointed. These tests persist charts whose
+stored name disagrees with the dataset (or semantic view) they are joined to,
+reload them from a cold identity map, and check that every MCP chart surface
+reports the live id and name.
+"""
+
+from collections.abc import Iterator
+from contextlib import nullcontext
+from dataclasses import dataclass
+from importlib import import_module
+from types import SimpleNamespace
+from typing import Any
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+from sqlalchemy import create_engine
+from sqlalchemy.engine import Engine
+from sqlalchemy.orm import Load, Session
+from sqlalchemy.pool import StaticPool
+
+from superset.connectors.sqla.models import SqlaTable
+from superset.mcp_service.app import mcp
+from superset.mcp_service.chart.schemas import (
+ ChartInfo,
+ resolve_chart_datasource_id,
+ resolve_chart_datasource_name,
+ serialize_chart_object,
+)
+from superset.mcp_service.chart.tool.get_chart_sql import
_resolve_datasource_name
+from superset.mcp_service.dashboard.schemas import serialize_chart_summary
+from superset.mcp_service.privacy import redact_chart_data_model_fields
+from superset.models.core import Database
+from superset.models.slice import Slice
+from superset.semantic_layers.models import SemanticLayer, SemanticView
+from superset.utils import json
+
+get_chart_info_module =
import_module("superset.mcp_service.chart.tool.get_chart_info")
+list_charts_module =
import_module("superset.mcp_service.chart.tool.list_charts")
+
+STALE_TABLE_NAME = "superset_events_production.superset_events"
+STALE_VIEW_NAME = "retired_view"
+
+
+@dataclass(frozen=True)
+class PersistedCharts:
+ table_chart_id: int
+ view_chart_id: int
+ orphan_chart_id: int
+ table_id: int
+ view_id: int
+
+
[email protected]
+def mcp_server() -> object:
+ return mcp
+
+
[email protected]
+def session_engine() -> Engine:
+ """Let the tool worker thread use the ``session`` built by this test."""
+ # The injected Session is handed from setup to a single tool worker
+ # sequentially; concurrent ownership is covered with scoped sessions.
+ return create_engine(
+ "sqlite://", connect_args={"check_same_thread": False},
poolclass=StaticPool
+ )
+
+
[email protected](autouse=True)
+def mock_auth() -> Iterator[None]:
+ with patch("superset.mcp_service.auth.get_user_from_request") as
mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield
+
+
[email protected]
+def charts(session: Session) -> PersistedCharts:
+ """Persist charts whose stored ``datasource_name`` is stale."""
+ engine = session.get_bind()
+ assert isinstance(engine, Engine)
+ Slice.metadata.create_all(engine)
+
+ database = Database(database_name="analytics", sqlalchemy_uri="sqlite://")
+ table = SqlaTable(
+ table_name="hs_flat_customer_events",
+ schema="hubspot_customers",
+ database=database,
+ )
+ layer = SemanticLayer(name="Jaffle Shop", type="test")
+ session.add_all([database, table, layer])
+ session.flush()
+ view = SemanticView(name="Orders", semantic_layer_uuid=layer.uuid)
+ session.add(view)
+ session.flush()
+
+ table_chart = Slice(
+ slice_name="Customer events",
+ viz_type="table",
+ datasource_type="table",
+ datasource_id=table.id,
+ datasource_name=STALE_TABLE_NAME,
+ )
+ view_chart = Slice(
+ slice_name="Orders by day",
+ viz_type="table",
+ datasource_type="semantic_view",
+ datasource_id=view.id,
+ datasource_name=STALE_VIEW_NAME,
+ )
+ orphan_chart = Slice(
+ slice_name="Dataset deleted",
+ viz_type="table",
+ datasource_type="table",
+ datasource_id=table.id + view.id + 100,
+ datasource_name=STALE_TABLE_NAME,
+ )
+ session.add_all([table_chart, view_chart, orphan_chart])
+ session.flush()
+ persisted = PersistedCharts(
+ table_chart_id=table_chart.id,
+ view_chart_id=view_chart.id,
+ orphan_chart_id=orphan_chart.id,
+ table_id=table.id,
+ view_id=view.id,
+ )
+ # The same id for a dataset and a semantic view checks that each chart is
+ # resolved through the relationship matching its own datasource type.
+ assert persisted.table_id == persisted.view_id
+ session.expunge_all()
+ return persisted
+
+
+def _load(session: Session, chart_id: int) -> Slice:
+ return session.query(Slice).filter(Slice.id == chart_id).one()
+
+
+def test_serialize_chart_object_reports_live_dataset(
+ session: Session, charts: PersistedCharts
+) -> None:
+ info = serialize_chart_object(_load(session, charts.table_chart_id))
+
+ assert info is not None
+ assert info.datasource_id == charts.table_id
+ assert info.datasource_name == "hubspot_customers.hs_flat_customer_events"
+
+
+def test_serialize_chart_object_reports_live_semantic_view(
+ session: Session, charts: PersistedCharts
+) -> None:
+ info = serialize_chart_object(_load(session, charts.view_chart_id))
+
+ assert info is not None
+ assert info.datasource_id == charts.view_id
+ assert info.datasource_name == "Orders"
+ assert info.datasource_type == "semantic_view"
+
+
+def test_missing_dataset_reports_no_name_rather_than_stale_one(
+ session: Session, charts: PersistedCharts
+) -> None:
+ chart = _load(session, charts.orphan_chart_id)
+
+ assert resolve_chart_datasource_name(chart) is None
+ assert resolve_chart_datasource_id(chart) == chart.datasource_id
+
+
+def test_get_chart_sql_names_the_live_dataset(
+ session: Session, charts: PersistedCharts
+) -> None:
+ chart = _load(session, charts.table_chart_id)
+
+ assert (
+ _resolve_datasource_name({}, chart)
+ == "hubspot_customers.hs_flat_customer_events"
+ )
+
+
+def test_objects_without_live_resolver_fall_back_to_stored_name() -> None:
+ row = SimpleNamespace(datasource_id=7, datasource_name="stored")
+
+ assert resolve_chart_datasource_name(row) == "stored"
+ assert resolve_chart_datasource_id(row) == 7
+
+
[email protected](
+ ("chart_attr", "id_attr", "expected_name", "expected_type"),
+ [
+ (
+ "table_chart_id",
+ "table_id",
+ "hubspot_customers.hs_flat_customer_events",
+ "table",
+ ),
+ ("view_chart_id", "view_id", "Orders", "semantic_view"),
+ ],
+)
+def test_dashboard_chart_summary_reports_live_dataset(
+ session: Session,
+ charts: PersistedCharts,
+ chart_attr: str,
+ id_attr: str,
+ expected_name: str,
+ expected_type: str,
+) -> None:
+ chart = _load(session, getattr(charts, chart_attr))
+
+ summary = serialize_chart_summary(chart, include_data_model_metadata=True)
+ redacted = serialize_chart_summary(chart,
include_data_model_metadata=False)
+
+ assert summary is not None
+ assert summary.datasource_id == getattr(charts, id_attr)
+ assert summary.datasource_type == expected_type
+ assert summary.model_dump()["datasource_type"] == expected_type
+ assert summary.datasource_name == expected_name
+ assert redacted is not None
+ assert redacted.datasource_id is None
+ assert redacted.datasource_type is None
+ assert redacted.datasource_name is None
+
+
+def test_redaction_clears_datasource_id() -> None:
+ redacted = redact_chart_data_model_fields(
+ ChartInfo(
+ id=1, datasource_id=14, datasource_type="table",
datasource_name="orders"
+ )
+ )
+
+ assert redacted.datasource_id is None
+ assert redacted.datasource_type is None
+ assert redacted.datasource_name is None
+
+
+def _find_by_id(session: Session) -> Any:
+ def find(
+ identifier: int, query_options: list[Load] | None = None, **_: Any
+ ) -> Slice | None:
+ return (
+ session.query(Slice)
+ .options(*(query_options or []))
+ .filter(Slice.id == identifier)
+ .one_or_none()
+ )
+
+ return find
+
+
+async def _call_get_chart_info(
+ session: Session,
+ chart_id: int,
+ *,
+ can_view_data_model: bool,
+ use_defaults: bool = False,
+) -> dict[str, Any]:
+ with (
+ patch.object(
+ get_chart_info_module.event_logger,
+ "log_context",
+ return_value=nullcontext(),
+ ),
+ patch.object(
+ get_chart_info_module,
+ "user_can_view_data_model_metadata",
+ return_value=can_view_data_model,
+ ),
+ patch.object(
+ get_chart_info_module,
+ "validate_chart_dataset",
+ return_value=SimpleNamespace(is_valid=True, warnings=[]),
+ ),
+ patch(
+ "superset.daos.chart.ChartDAO.find_by_id",
+ side_effect=_find_by_id(session),
+ ),
+ patch("superset.mcp_service.auth.check_tool_permission",
return_value=True),
+ ):
+ async with Client(mcp) as client:
+ response = await client.call_tool(
+ "get_chart_info",
+ {
+ "request": {
+ "identifier": chart_id,
+ **(
+ {}
+ if use_defaults
+ else {
+ "select_columns": [
+ "id",
+ "slice_name",
+ "datasource_id",
+ "datasource_name",
+ ]
+ }
+ ),
+ }
+ },
+ )
+ return json.loads(response.content[0].text)
+
+
[email protected]
+async def test_get_chart_info_returns_live_datasource_id_and_name(
+ session: Session, charts: PersistedCharts
+) -> None:
+ result = await _call_get_chart_info(
+ session, charts.table_chart_id, can_view_data_model=True
+ )
+
+ assert result == {
+ "id": charts.table_chart_id,
+ "slice_name": "Customer events",
+ "datasource_id": charts.table_id,
+ "datasource_name": "hubspot_customers.hs_flat_customer_events",
+ }
+
+
[email protected]
+async def test_get_chart_info_redacts_datasource_id_without_data_model_access(
+ session: Session, charts: PersistedCharts
+) -> None:
+ result = await _call_get_chart_info(
+ session, charts.table_chart_id, can_view_data_model=False
+ )
+
+ assert result["datasource_id"] is None
+ assert result["datasource_name"] is None
+
+
+def _list_charts_dao(session: Session, calls: list[list[str]]) -> Any:
+ def list_(*_: Any, columns: list[str] | None = None, **__: Any) -> Any:
+ calls.append(list(columns or []))
+ rows = session.query(Slice).order_by(Slice.id).all()
+ return rows, len(rows)
+
+ return list_
+
+
+async def _call_list_charts(
+ session: Session,
+ select_columns: list[str],
+ *,
+ can_view_data_model: bool,
+) -> tuple[dict[str, Any], list[list[str]]]:
+ calls: list[list[str]] = []
+ with (
+ patch.object(
+ list_charts_module,
+ "user_can_view_data_model_metadata",
+ return_value=can_view_data_model,
+ ),
+ patch(
+ "superset.daos.chart.ChartDAO.list",
+ side_effect=_list_charts_dao(session, calls),
+ ),
+ ):
+ async with Client(mcp) as client:
+ response = await client.call_tool(
+ "list_charts",
+ {"request": {"select_columns": select_columns}},
+ )
+ return json.loads(response.content[0].text), calls
+
+
[email protected]
+async def test_list_charts_returns_live_datasource_id_and_name(
+ session: Session, charts: PersistedCharts
+) -> None:
+ data, calls = await _call_list_charts(
+ session,
+ ["id", "datasource_id", "datasource_name", "params"],
+ can_view_data_model=True,
+ )
+
+ # The live-datasource relationships are eager-loaded with the charts.
+ assert {"table", "semantic_view"} <= set(calls[0])
+ by_id = {chart["id"]: chart for chart in data["charts"]}
+ assert by_id[charts.table_chart_id] == {
+ "id": charts.table_chart_id,
+ "datasource_id": charts.table_id,
+ "datasource_name": "hubspot_customers.hs_flat_customer_events",
+ }
+ assert by_id[charts.view_chart_id] == {
+ "id": charts.view_chart_id,
+ "datasource_id": charts.view_id,
+ "datasource_name": "Orders",
+ }
+ assert by_id[charts.orphan_chart_id]["datasource_name"] is None
+ # ``params`` is a model column that ChartInfo does not expose.
+ assert data["columns_loaded"] == ["id", "datasource_id", "datasource_name"]
+
+
[email protected]
+async def test_list_charts_omits_datasource_id_without_data_model_access(
+ session: Session, charts: PersistedCharts
+) -> None:
+ data, _ = await _call_list_charts(
+ session,
+ ["id", "slice_name", "datasource_id", "datasource_name"],
+ can_view_data_model=False,
+ )
+
+ assert data["columns_loaded"] == ["id", "slice_name"]
+ for chart in data["charts"]:
+ assert set(chart) == {"id", "slice_name"}
+
+
[email protected]
+async def test_list_charts_columns_loaded_excludes_computation_dependencies(
+ session: Session, charts: PersistedCharts
+) -> None:
+ data, calls = await _call_list_charts(
+ session, ["id", "changed_on_humanized"], can_view_data_model=True
+ )
+
+ assert "changed_on" in calls[0]
+ assert data["columns_loaded"] == ["id", "changed_on_humanized"]
+ assert "table" not in calls[0]
+
+
[email protected]
+async def test_list_charts_live_datasource_query_count_is_bounded(
+ session: Session, charts: PersistedCharts
+) -> None:
+ """Real DAO queries must not lazy-load relationships for each chart."""
+ from sqlalchemy import event
+
+ from superset.daos.chart import ChartDAO
+
+ statements: list[str] = []
+
+ def record_statement(
+ conn: Any,
+ cursor: Any,
+ statement: str,
+ parameters: Any,
+ context: Any,
+ executemany: bool,
+ ) -> None:
+ """Count SELECTs during the tool call, excluding fixture setup."""
+ if statement.lstrip().upper().startswith("SELECT"):
+ statements.append(statement)
+
+ counts: list[int] = []
+ with (
+ patch.object(
+ list_charts_module,
+ "user_can_view_data_model_metadata",
+ return_value=True,
+ ),
+ # Isolate authorization; keep the DAO's query construction and
execution.
+ patch.object(
+ ChartDAO, "_apply_base_filter", side_effect=lambda query, **_:
query
+ ),
+ ):
+ async with Client(mcp) as client:
+ for page_size in (1, 3):
+ session.expunge_all()
+ statements.clear()
+ engine = session.get_bind()
+ event.listen(engine, "before_cursor_execute", record_statement)
+ try:
+ response = await client.call_tool(
+ "list_charts",
+ {
+ "request": {
+ "select_columns": [
+ "id",
+ "datasource_id",
+ "datasource_name",
+ "params",
+ ],
+ "order_column": "id",
+ "order_direction": "asc",
+ "page_size": page_size,
+ }
+ },
+ )
+ finally:
+ event.remove(engine, "before_cursor_execute",
record_statement)
+ data = json.loads(response.content[0].text)
+ assert len(data["charts"]) == page_size
+ assert data["charts"][0]["datasource_id"] == charts.table_id
+ assert data["charts"][0]["datasource_name"] == (
+ "hubspot_customers.hs_flat_customer_events"
+ )
+ counts.append(len(statements))
+
+ assert counts == [2, 2], f"SELECT counts for 1 and 3 charts: {counts}"
+
+
[email protected](
+ "select_columns", [None, ["tags"], ["editors"], ["tags", "editors"]]
+)
+def test_serialize_chart_preserves_requested_collections(
+ select_columns: list[str] | None,
+) -> None:
+ """Full-object and explicit collection reads retain their existing
shape."""
+ from superset.subjects.models import Subject
+ from superset.tags.models import Tag
+
+ chart = Slice(
+ tags=[Tag(id=7, name="sales")],
+ editors=[Subject(id=9, label="Chart editor", active=True)],
+ )
+ info = serialize_chart_object(chart, select_columns=select_columns)
+
+ assert info is not None
+ assert [tag.model_dump() for tag in info.tags] == (
+ [{"id": 7, "name": "sales", "type": None, "description": None}]
+ if select_columns is None or "tags" in select_columns
+ else []
+ )
+ assert [editor.model_dump() for editor in info.editors] == (
+ [{"id": 9, "label": "Chart editor", "type": None, "active": True}]
+ if select_columns is None or "editors" in select_columns
+ else []
+ )
+
+
[email protected]("datasource_type", ["query", "saved_query"])
[email protected]("surface", ["chart", "dashboard"])
+def test_non_dataset_chart_keeps_stored_name(
+ datasource_type: str, surface: str
+) -> None:
+ """Query-backed charts retain their names on both metadata surfaces."""
+ chart = Slice(
+ id=123,
+ datasource_type=datasource_type,
+ datasource_id=7,
+ datasource_name="Saved analysis",
+ )
+ if surface == "chart":
+ result = serialize_chart_object(chart)
+ else:
+ result = serialize_chart_summary(chart,
include_data_model_metadata=True)
+ assert result is not None
+ assert result.datasource_name == "Saved analysis"
+ assert result.datasource_id == 7
+ assert result.datasource_type == datasource_type
+ if surface == "dashboard":
+ redacted = serialize_chart_summary(chart)
+ assert redacted is not None
+ assert redacted.datasource_id is None
+ assert redacted.datasource_type is None
+ assert redacted.datasource_name is None
+
+
+def test_default_chart_info_includes_datasource_id() -> None:
+ """Default column selection includes the dataset identifier."""
+ from superset.mcp_service.chart.schemas import GetChartInfoRequest
+
+ assert "datasource_id" in GetChartInfoRequest(identifier=1).select_columns
+
+
[email protected]
[email protected](
+ ("operator", "value", "expected"),
+ [
+ ("eq", "hubspot_customers.hs_flat_customer_events", "table"),
+ ("eq", STALE_TABLE_NAME, "none"),
+ ("eq", "Orders", "view"),
+ ("eq", STALE_VIEW_NAME, "none"),
+ ("ct", "flat_customer", "table"),
+ ("sw", "hubspot_customers.", "table"),
+ ("in", ["Orders", "hubspot_customers.hs_flat_customer_events"],
"both"),
+ ("is_null", "", "orphan"),
+ ("ne", "Orders", "table"),
+ ],
+)
+async def test_list_filters_live_datasource_names(
+ session: Session,
+ charts: PersistedCharts,
+ operator: str,
+ value: str | list[str],
+ expected: str,
+) -> None:
+ """The real DAO filters live names before counting and pagination."""
+ from superset.daos.chart import ChartDAO
+
+ expected_ids = {
+ "table": [charts.table_chart_id],
+ "view": [charts.view_chart_id],
+ "both": [charts.table_chart_id, charts.view_chart_id],
+ "orphan": [charts.orphan_chart_id],
+ "none": [],
+ }[expected]
+ with (
+ patch.object(
+ list_charts_module, "user_can_view_data_model_metadata",
return_value=True
+ ),
+ patch.object(
+ ChartDAO, "_apply_base_filter", side_effect=lambda query, **_:
query
+ ),
+ patch.object(
+ list_charts_module.event_logger, "log_context",
return_value=nullcontext()
+ ),
+ ):
+ async with Client(mcp) as client:
+ response = await client.call_tool(
+ "list_charts",
+ {
+ "request": {
+ "select_columns": ["id"],
+ "filters": [
+ {"col": "datasource_name", "opr": operator,
"value": value}
+ ],
+ "order_column": "id",
+ "page_size": 1,
+ }
+ },
+ )
+ result = json.loads(response.content[0].text)
+ assert result["total_count"] == len(expected_ids)
+ assert [chart["id"] for chart in result["charts"]] == expected_ids[:1]
+
+
[email protected]
[email protected]("can_view_data_model", [True, False])
+async def test_default_chart_info_dataset_identifier_visibility(
+ session: Session, charts: PersistedCharts, can_view_data_model: bool
+) -> None:
+ """Default responses include the live identifier only for authorized
roles."""
+ result = await _call_get_chart_info(
+ session,
+ charts.table_chart_id,
+ can_view_data_model=can_view_data_model,
+ use_defaults=True,
+ )
+ assert result["datasource_id"] == (charts.table_id if can_view_data_model
else None)
+
+
[email protected]
[email protected]("tool_name", ["get_chart_info", "get_chart_sql"])
+async def test_deleted_dataset_keeps_access_error(
+ session: Session, charts: PersistedCharts, tool_name: str
+) -> None:
+ """Missing datasets keep the pre-existing tool validation error."""
+ from superset.daos.dataset import DatasetDAO
+
+ module = import_module(f"superset.mcp_service.chart.tool.{tool_name}")
+ with (
+ patch.object(module.event_logger, "log_context",
return_value=nullcontext()),
+ patch.object(
+ get_chart_info_module,
+ "user_can_view_data_model_metadata",
+ return_value=True,
+ ),
+ patch(
+ "superset.daos.chart.ChartDAO.find_by_id",
+ side_effect=_find_by_id(session),
+ ),
+ patch.object(
+ DatasetDAO, "_apply_base_filter", side_effect=lambda query, *_,
**__: query
+ ),
+ ):
+ async with Client(mcp) as client:
+ response = await client.call_tool(
+ tool_name, {"request": {"identifier": charts.orphan_chart_id}}
+ )
+ result = json.loads(response.content[0].text)
+ assert result["error_type"] == "DatasetNotAccessible"
+ assert "has been deleted or does not exist" in result["error"]
+
+
[email protected]("schema", [None, "", "analytics"])
[email protected]("catalog", [None, "warehouse"])
+def test_live_name_filter_matches_sqla_table_name(
+ session: Session,
+ charts: PersistedCharts,
+ schema: str | None,
+ catalog: str | None,
+) -> None:
+ """Keep the SQL filter in parity with the Python datasource naming rule."""
+ from superset.mcp_service.chart.schemas import ChartFilter
+
+ table = session.query(SqlaTable).filter(SqlaTable.id ==
charts.table_id).one()
+ table.schema = schema
+ table.catalog = catalog
+ session.flush()
+ chart = _load(session, charts.table_chart_id)
+ expected_name = table.name
+ assert resolve_chart_datasource_name(chart) == expected_name
+
+ name_filter = list_charts_module._LiveDatasourceNameFilter(
+ [ChartFilter(col="datasource_name", opr="eq", value=expected_name)]
+ )
+ matches = name_filter.apply(session.query(Slice), None).all()
+ assert [match.id for match in matches] == [charts.table_chart_id]
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 47e642e2673..06bde86dfc4 100644
--- a/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
+++ b/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
@@ -269,6 +269,8 @@ class TestSerializeDashboardObject:
chart.id = 5
chart.slice_name = "Revenue Chart"
chart.viz_type = "echarts_timeseries_bar"
+ chart.datasource_type = "table"
+ chart.datasource_id = 3
chart.datasource_name = "sales"
chart.description = "Monthly revenue"
@@ -279,6 +281,8 @@ class TestSerializeDashboardObject:
assert result.charts[0].id == 5
assert result.charts[0].slice_name == _wrapped("Revenue Chart")
assert result.charts[0].viz_type == "echarts_timeseries_bar"
+ assert result.charts[0].datasource_id == 3
+ assert result.charts[0].datasource_type == "table"
assert result.charts[0].datasource_name == "sales"
assert result.charts[0].url ==
"http://localhost:8088/explore/?slice_id=5";
# Verify no heavy fields
@@ -300,6 +304,8 @@ class TestSerializeDashboardObject:
chart.id = 5
chart.slice_name = "Revenue Chart"
chart.viz_type = "echarts_timeseries_bar"
+ chart.datasource_type = "table"
+ chart.datasource_id = 3
chart.datasource_name = "sales"
chart.description = "Monthly revenue"
@@ -309,6 +315,8 @@ class TestSerializeDashboardObject:
assert len(result.charts) == 1
assert result.charts[0].slice_name == _wrapped("Revenue Chart")
assert result.charts[0].viz_type == "echarts_timeseries_bar"
+ assert result.charts[0].datasource_id is None
+ assert result.charts[0].datasource_type is None
assert result.charts[0].datasource_name is None
assert result.charts[0].url ==
"http://localhost:8088/explore/?slice_id=5";
@@ -326,6 +334,8 @@ class TestSerializeDashboardObject:
chart.id = 5
chart.slice_name = "Revenue Chart"
chart.viz_type = "echarts_timeseries_bar"
+ chart.datasource_type = "table"
+ chart.datasource_id = 3
chart.datasource_name = "sales"
chart.description = "Monthly revenue"
@@ -348,6 +358,8 @@ class TestSerializeDashboardObject:
result = dashboard_serializer(dashboard)
+ assert result.charts[0].datasource_id is None
+ assert result.charts[0].datasource_type is None
assert result.charts[0].datasource_name is None
assert result.native_filters[0].targets == []
@@ -366,6 +378,8 @@ class TestSerializeDashboardObject:
chart.id = 5
chart.slice_name = "Revenue Chart"
chart.viz_type = "echarts_timeseries_bar"
+ chart.datasource_type = "table"
+ chart.datasource_id = 3
chart.datasource_name = "sales"
chart.description = "Monthly revenue"
@@ -523,12 +537,16 @@ class TestSerializeChartSummary:
chart.id = 5
chart.slice_name = "Revenue Chart"
chart.viz_type = "echarts_timeseries_bar"
+ chart.datasource_type = "table"
+ chart.datasource_id = 3
chart.datasource_name = "sales"
chart.description = "Monthly revenue"
result = serialize_chart_summary(chart)
assert result is not None
+ assert result.datasource_id is None
+ assert result.datasource_type is None
assert result.datasource_name is None
