aminghadersohi commented on code in PR #40961:
URL: https://github.com/apache/superset/pull/40961#discussion_r3493777881
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1528,3 +1543,225 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+# Per-dataset caps keep responses small enough for LLM context: wide
+# datasets can have hundreds of columns, which would dwarf the fields an
+# agent actually needs to configure native filters.
+MAX_DASHBOARD_DATASET_COLUMNS = 100
+MAX_DASHBOARD_DATASET_METRICS = 50
Review Comment:
Fixed — added `: int` annotations to both `MAX_DASHBOARD_DATASET_COLUMNS`
and `MAX_DASHBOARD_DATASET_METRICS`.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1528,3 +1543,225 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+# Per-dataset caps keep responses small enough for LLM context: wide
+# datasets can have hundreds of columns, which would dwarf the fields an
+# agent actually needs to configure native filters.
+MAX_DASHBOARD_DATASET_COLUMNS = 100
+MAX_DASHBOARD_DATASET_METRICS = 50
+
+
+class DashboardDatasetColumn(BaseModel):
+ """Lean column representation for dashboard dataset context."""
+
+ column_name: str = Field(..., description="Column name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ type: str | None = Field(None, description="Column data type")
+ is_dttm: bool | None = Field(None, description="Is datetime column")
+
+
+class DashboardDatasetMetric(BaseModel):
+ """Lean metric representation for dashboard dataset context."""
+
+ metric_name: str = Field(..., description="Saved metric name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ expression: str | None = Field(None, description="SQL expression")
+
+
+class DashboardDatasetDatabaseInfo(BaseModel):
+ """Database connection summary for a dashboard dataset."""
+
+ id: int | None = Field(None, description="Database ID")
+ name: str | None = Field(None, description="Database name")
+ backend: str | None = Field(None, description="Database backend (engine)")
+
+
+class DashboardDatasetSummary(BaseModel):
+ """A dataset used by a dashboard's charts, with columns and metrics."""
+
+ id: int | None = Field(None, description="Dataset ID")
+ uuid: str | None = Field(None, description="Dataset UUID")
+ table_name: str | None = Field(None, description="Table name")
+ schema_name: str | None = Field(None, description="Schema name")
+ database: DashboardDatasetDatabaseInfo | None = Field(
+ None, description="Database the dataset belongs to"
+ )
+ chart_count: int = Field(
+ 0, description="Number of charts on the dashboard using this dataset"
+ )
+ columns: List[DashboardDatasetColumn] = Field(
+ default_factory=list, description="Dataset columns"
+ )
+ metrics: List[DashboardDatasetMetric] = Field(
+ default_factory=list, description="Dataset metrics"
+ )
+ total_column_count: int = Field(
+ 0, description="Total number of columns on the dataset"
+ )
+ total_metric_count: int = Field(
+ 0, description="Total number of metrics on the dataset"
+ )
+ columns_truncated: bool = Field(
+ False,
+ description=(
+ "True when the columns list was truncated to keep the response
small"
+ ),
+ )
+ metrics_truncated: bool = Field(
+ False,
+ description=(
+ "True when the metrics list was truncated to keep the response
small"
+ ),
+ )
+
+ @model_serializer(mode="wrap")
+ def _rename_schema_field(self, serializer: Any, info: Any) -> Dict[str,
Any]:
+ """Serialize 'schema_name' as 'schema' to match API conventions."""
+ data = serializer(self)
+ if "schema_name" in data:
+ data["schema"] = data.pop("schema_name")
+ return data
+
+
+class DashboardDatasets(BaseModel):
+ """Response schema for get_dashboard_datasets."""
+
+ id: int | None = Field(None, description="Dashboard ID")
+ dashboard_title: str | None = Field(None, description="Dashboard title")
+ uuid: str | None = Field(None, description="Dashboard UUID")
+ dataset_count: int = Field(
+ 0, description="Number of accessible datasets used by the dashboard"
+ )
+ inaccessible_dataset_count: int = Field(
+ 0,
+ description=(
+ "Number of datasets used by the dashboard that the current user "
+ "cannot access (excluded from 'datasets')"
+ ),
+ )
+ datasets: List[DashboardDatasetSummary] = Field(
+ default_factory=list,
+ description="Datasets used by the dashboard's charts",
+ )
+
+
+def _serialize_dashboard_dataset(
+ datasource: Any, chart_count: int
+) -> DashboardDatasetSummary:
+ """Serialize a datasource to a lean, LLM-safe dataset summary."""
+ all_columns = list(getattr(datasource, "columns", None) or [])
+ all_metrics = list(getattr(datasource, "metrics", None) or [])
+
+ columns = [
+ DashboardDatasetColumn(
+ column_name=escape_llm_context_delimiters(
+ getattr(column, "column_name", None) or ""
+ ),
+ verbose_name=sanitize_for_llm_context(
+ getattr(column, "verbose_name", None),
+ field_path=("columns", str(index), "verbose_name"),
+ ),
+ type=getattr(column, "type", None),
+ is_dttm=getattr(column, "is_dttm", None),
+ )
+ for index, column in
enumerate(all_columns[:MAX_DASHBOARD_DATASET_COLUMNS])
+ ]
+ metrics = [
+ DashboardDatasetMetric(
+ metric_name=escape_llm_context_delimiters(
+ getattr(metric, "metric_name", None) or ""
+ ),
+ verbose_name=sanitize_for_llm_context(
+ getattr(metric, "verbose_name", None),
+ field_path=("metrics", str(index), "verbose_name"),
+ ),
+ expression=sanitize_for_llm_context(
+ getattr(metric, "expression", None),
+ field_path=("metrics", str(index), "expression"),
+ ),
+ )
+ for index, metric in
enumerate(all_metrics[:MAX_DASHBOARD_DATASET_METRICS])
+ ]
+
+ database = getattr(datasource, "database", None)
+ database_info = (
+ DashboardDatasetDatabaseInfo(
+ id=getattr(database, "id", None),
+ name=escape_llm_context_delimiters(
+ getattr(database, "database_name", None)
+ ),
+ backend=getattr(database, "backend", None),
+ )
+ if database is not None
+ else None
+ )
+
+ dataset_uuid = getattr(datasource, "uuid", None)
+ return DashboardDatasetSummary(
+ id=getattr(datasource, "id", None),
+ uuid=str(dataset_uuid) if dataset_uuid else None,
+ table_name=escape_llm_context_delimiters(
+ getattr(datasource, "table_name", None)
+ ),
+ schema_name=escape_llm_context_delimiters(getattr(datasource,
"schema", None)),
+ database=database_info,
+ chart_count=chart_count,
+ columns=columns,
+ metrics=metrics,
+ total_column_count=len(all_columns),
+ total_metric_count=len(all_metrics),
+ columns_truncated=len(all_columns) > MAX_DASHBOARD_DATASET_COLUMNS,
+ metrics_truncated=len(all_metrics) > MAX_DASHBOARD_DATASET_METRICS,
+ )
+
+
+def dashboard_datasets_serializer(dashboard: "Dashboard") -> DashboardDatasets:
+ """Serialize a Dashboard model to the datasets used by its charts.
+
+ Groups the dashboard's charts by datasource (mirroring
+ ``Dashboard.datasets_trimmed_for_slices``) but keeps the full column and
+ metric lists (capped) since native-filter configuration regularly needs
+ columns that no chart references. Datasets the current user cannot
+ access are excluded and only counted.
+ """
+ from superset.mcp_service.auth import has_dataset_access
+
+ slices_by_datasource: Dict[int, List[Any]] = {}
+ for slc in getattr(dashboard, "slices", None) or []:
+ datasource_id = getattr(slc, "datasource_id", None)
+ if datasource_id is None:
+ continue
+ slices_by_datasource.setdefault(datasource_id, []).append(slc)
+
+ datasets: List[DashboardDatasetSummary] = []
+ inaccessible_count = 0
Review Comment:
Fixed — added `: int` annotation to `inaccessible_count`.
##########
superset/mcp_service/dashboard/tool/get_dashboard_datasets.py:
##########
@@ -0,0 +1,156 @@
+# 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.
+
+"""
+Get dashboard datasets FastMCP tool
+
+Returns the datasets used by a dashboard's charts, including columns and
+metrics. This is the prerequisite context an agent needs before configuring
+native filters on a dashboard (e.g. picking filter target columns).
+"""
+
+import logging
+from datetime import datetime, timezone
+
+from fastmcp import Context
+from sqlalchemy.orm import subqueryload
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.dashboard.schemas import (
+ dashboard_datasets_serializer,
+ DashboardDatasets,
+ DashboardError,
+ GetDashboardDatasetsRequest,
+)
+from superset.mcp_service.mcp_core import ModelGetInfoCore
+from superset.mcp_service.privacy import (
+ DATA_MODEL_METADATA_ERROR_TYPE,
+ requires_data_model_metadata_access,
+ user_can_view_data_model_metadata,
+)
+
+logger = logging.getLogger(__name__)
Review Comment:
Declining — sibling dashboard tools (`get_dashboard_info.py:50`,
`get_dashboard_layout.py:42`) use the same unannotated `logger =
logging.getLogger(__name__)` pattern. Annotating only the new file would be
inconsistent with those.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
