codeant-ai-for-open-source[bot] commented on code in PR #40961:
URL: https://github.com/apache/superset/pull/40961#discussion_r3482399402
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1512,3 +1523,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")
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Keep the integer dataset id in MCP dashboard schema responses alongside
uuid, because native-filter targets and related MCP workflows reference
datasets by integer id; do not remove it just because a uuid is also present.
**Applied to:**
- `superset/mcp_service/dashboard/**`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1512,3 +1523,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")
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> For MCP dashboard schemas, keep the integer `id` alongside `uuid` when it
is needed for consistency with `DashboardInfo` and for chaining into id-based
dashboard tools; do not require UUID-only identifiers in these responses.
**Applied to:**
- `superset/mcp_service/dashboard/**`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
tests/unit_tests/mcp_service/dashboard/tool/test_get_dashboard_datasets.py:
##########
@@ -0,0 +1,355 @@
+# 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.
+
+"""Unit tests for the MCP get_dashboard_datasets tool."""
+
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.utils.sanitization import (
+ LLM_CONTEXT_CLOSE_DELIMITER,
+ LLM_CONTEXT_OPEN_DELIMITER,
+)
+from superset.utils import json
+
+
+def _wrapped(value: str) -> str:
+ return
f"{LLM_CONTEXT_OPEN_DELIMITER}\n{value}\n{LLM_CONTEXT_CLOSE_DELIMITER}"
+
+
+def _build_column_mock(
+ name: str,
+ *,
+ verbose_name: str | None = None,
+ type_: str | None = "VARCHAR",
+ is_dttm: bool = False,
+) -> Mock:
+ column = Mock()
+ column.column_name = name
+ column.verbose_name = verbose_name
+ column.type = type_
+ column.is_dttm = is_dttm
+ return column
+
+
+def _build_metric_mock(
+ name: str,
+ *,
+ verbose_name: str | None = None,
+ expression: str | None = None,
+) -> Mock:
+ metric = Mock()
+ metric.metric_name = name
+ metric.verbose_name = verbose_name
+ metric.expression = expression
+ return metric
+
+
+def _build_database_mock(
+ *, database_id: int = 7, name: str = "examples", backend: str =
"postgresql"
+) -> Mock:
+ database = Mock()
+ database.id = database_id
+ database.database_name = name
+ database.backend = backend
+ return database
+
+
+def _build_datasource_mock(
+ *,
+ dataset_id: int,
+ uuid: str | None = None,
+ table_name: str = "my_table",
+ schema: str | None = "public",
+ database: Mock | None = None,
+ columns: list[Mock] | None = None,
+ metrics: list[Mock] | None = None,
+) -> Mock:
+ datasource = Mock()
+ datasource.id = dataset_id
+ datasource.uuid = uuid
+ datasource.table_name = table_name
+ datasource.schema = schema
+ datasource.database = database
+ datasource.columns = columns or []
+ datasource.metrics = metrics or []
+ return datasource
+
+
+def _build_slice_mock(datasource: Mock) -> Mock:
+ slc = Mock()
+ slc.datasource_id = datasource.id
+ slc.datasource = datasource
+ return slc
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag missing docstrings or typing nits for pytest fixtures and
helper builder functions in mcp_service test files; follow the existing
convention used in this test area.
**Applied to:**
- `tests/unit_tests/mcp_service/**`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
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]
