QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-05-12 19:35:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): add description and certification fields to default list tool columns (#39017)

Browse Source 
(cherry picked from commit 7be2acb2f3)
This commit is contained in:
Amin Ghadersohi
2026-04-06 13:37:52 -04:00
committed by Michael S. Molina
parent 4764eea1dc
commit b9b48fb1e4
13 changed files with 198 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
View File

@@ -363,6 +363,8 @@ def _make_mock_chart(chart_id: int = 42) -> Mock:
chart.datasource_name = "test_table"
chart.datasource_type = "table"
chart.description = None
chart.certified_by = None
chart.certification_details = None
chart.cache_timeout = None
chart.changed_by = None
chart.changed_by_name = "admin"
@@ -394,6 +396,20 @@ class TestChartSerializationEagerLoading:
assert result.tags == []
assert result.owners == []
def test_serialize_chart_object_with_certification_fields(self):
"""serialize_chart_object correctly serializes non-None certification values."""
from superset.mcp_service.chart.schemas import serialize_chart_object
chart = _make_mock_chart()
chart.certified_by = "Data Team"
chart.certification_details = "Verified Q1 2026 metrics"
result = serialize_chart_object(chart)
assert result is not None
assert result.certified_by == "Data Team"
assert result.certification_details == "Verified Q1 2026 metrics"
def test_serialize_chart_object_fails_on_detached_instance(self):
"""serialize_chart_object raises when accessing lazy attrs on detached
instance — this is the bug scenario that the eager-loading fix prevents."""

11
tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
View File

@@ -46,6 +46,8 @@ def mock_chart():
chart.datasource_name = "test_dataset"
chart.datasource_type = "table"
chart.description = "Test chart"
chart.certified_by = None
chart.certification_details = None
chart.url = "/chart/1"
chart.changed_by_name = "admin"
chart.changed_on = None
@@ -197,20 +199,21 @@ class TestChartDefaultColumnFiltering:
"""Test that minimal default columns are properly defined."""
from superset.mcp_service.common.schema_discovery import CHART_DEFAULT_COLUMNS
# Required minimal columns must be present
required_columns = {
assert set(CHART_DEFAULT_COLUMNS) == {
"id",
"slice_name",
"viz_type",
"description",
"certified_by",
"certification_details",
"url",
"changed_on",
"changed_on_humanized",
}
assert required_columns.issubset(set(CHART_DEFAULT_COLUMNS))
# Heavy columns should NOT be in defaults
assert "form_data" not in CHART_DEFAULT_COLUMNS
assert "query_context" not in CHART_DEFAULT_COLUMNS
assert "description" not in CHART_DEFAULT_COLUMNS
assert "datasource_name" not in CHART_DEFAULT_COLUMNS
assert "uuid" not in CHART_DEFAULT_COLUMNS

2
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
View File

@@ -87,6 +87,8 @@ def _mock_chart(id: int = 1, slice_name: str = "Test Chart") -> Mock:
chart.datasource_name = None
chart.datasource_type = None
chart.description = None
chart.certified_by = None
chart.certification_details = None
chart.cache_timeout = None
chart.changed_by = None
chart.changed_by_name = None

6
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
View File

@@ -529,13 +529,15 @@ class TestDashboardDefaultColumnFiltering:
DASHBOARD_DEFAULT_COLUMNS,
)
# Should have exactly 5 minimal columns
assert len(DASHBOARD_DEFAULT_COLUMNS) == 5
assert set(DASHBOARD_DEFAULT_COLUMNS) == {
"id",
"dashboard_title",
"slug",
"description",
"certified_by",
"certification_details",
"url",
"changed_on",
"changed_on_humanized",
}

70
tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
View File

@@ -46,6 +46,8 @@ def create_mock_dataset(
dataset.table_name = table_name
dataset.schema = schema
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -106,6 +108,8 @@ async def test_list_datasets_basic(mock_list, mcp_server):
dataset.table_name = "Test DatasetInfo"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -212,6 +216,8 @@ async def test_list_datasets_custom_uuid_columns(mock_list, mcp_server):
dataset.table_name = "custom_dataset"
dataset.schema = "public"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -290,6 +296,8 @@ async def test_list_datasets_with_filters(mock_list, mcp_server):
dataset.table_name = "Filtered Dataset"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -391,6 +399,8 @@ async def test_list_datasets_with_string_filters(mock_list, mcp_server):
dataset.table_name = "String Filter Dataset"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -468,6 +478,8 @@ async def test_list_datasets_with_search(mock_list, mcp_server):
dataset.database_name = "test_db"
dataset.database = None
dataset.description = "A test dataset"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by = "admin"
dataset.changed_by_name = "admin"
dataset.changed_on = None
@@ -559,6 +571,8 @@ async def test_list_datasets_simple_with_search(mock_list, mcp_server):
dataset.database_name = "analytics_db"
dataset.database = None
dataset.description = "Another test dataset"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by = "user"
dataset.changed_by_name = "user"
dataset.changed_on = None
@@ -648,6 +662,8 @@ async def test_list_datasets_simple_basic(mock_list, mcp_server):
dataset.table_name = "Test DatasetInfo"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -743,6 +759,8 @@ async def test_list_datasets_simple_with_filters(mock_list, mcp_server):
dataset.table_name = "Sales Dataset"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -853,6 +871,8 @@ async def test_get_dataset_info_success(mock_info, mcp_server):
dataset.table_name = "Test DatasetInfo"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -990,6 +1010,8 @@ async def test_get_dataset_info_includes_columns_and_metrics(mock_info, mcp_serv
dataset.database = MagicMock()
dataset.database.database_name = "examples"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -1080,6 +1102,8 @@ async def test_list_datasets_includes_columns_and_metrics(mock_list, mcp_server)
dataset.database = MagicMock()
dataset.database.database_name = "examples"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -1154,6 +1178,8 @@ async def test_get_dataset_info_by_uuid(mock_find_object, mcp_server):
dataset.table_name = "Test Dataset UUID"
dataset.schema = "main"
dataset.description = "desc"
dataset.certified_by = None
dataset.certification_details = None
dataset.changed_by_name = "admin"
dataset.changed_on = None
dataset.changed_on_humanized = None
@@ -1190,6 +1216,36 @@ async def test_get_dataset_info_by_uuid(mock_find_object, mcp_server):
assert data["table_name"] == "Test Dataset UUID"
class TestDatasetCertificationSerialization:
"""Test certification fields flow through dataset serialization."""
def test_serialize_dataset_with_certification_fields(self):
"""Serializes non-None certification values."""
from superset.mcp_service.dataset.schemas import serialize_dataset_object
dataset = create_mock_dataset()
dataset.certified_by = "Analytics Engineering"
dataset.certification_details = "Production-ready, SLA-backed"
result = serialize_dataset_object(dataset)
assert result is not None
assert result.certified_by == "Analytics Engineering"
assert result.certification_details == "Production-ready, SLA-backed"
def test_serialize_dataset_with_none_certification(self):
"""serialize_dataset_object handles None certification fields."""
from superset.mcp_service.dataset.schemas import serialize_dataset_object
dataset = create_mock_dataset()
result = serialize_dataset_object(dataset)
assert result is not None
assert result.certified_by is None
assert result.certification_details is None
class TestDatasetDefaultColumnFiltering:
"""Test default column filtering behavior for datasets."""
@@ -1197,12 +1253,14 @@ class TestDatasetDefaultColumnFiltering:
"""Test that minimal default columns are properly defined."""
from superset.mcp_service.common.schema_discovery import DATASET_DEFAULT_COLUMNS
# Should have exactly 4 minimal columns
assert len(DATASET_DEFAULT_COLUMNS) == 4
assert set(DATASET_DEFAULT_COLUMNS) == {
"id",
"table_name",
"schema",
"description",
"certified_by",
"certification_details",
"changed_on",
"changed_on_humanized",
}
assert "uuid" not in DATASET_DEFAULT_COLUMNS
@@ -1210,7 +1268,6 @@ class TestDatasetDefaultColumnFiltering:
# Heavy columns should NOT be in defaults
assert "columns" not in DATASET_DEFAULT_COLUMNS
assert "metrics" not in DATASET_DEFAULT_COLUMNS
assert "description" not in DATASET_DEFAULT_COLUMNS
assert "database_name" not in DATASET_DEFAULT_COLUMNS
@patch("superset.daos.dataset.DatasetDAO.list")
@@ -1236,6 +1293,9 @@ class TestDatasetDefaultColumnFiltering:
"id",
"table_name",
"schema",
"description",
"certified_by",
"certification_details",
"changed_on",
"changed_on_humanized",
}
@@ -1324,6 +1384,9 @@ class TestDatasetDefaultColumnFiltering:
"id",
"table_name",
"schema",
"description",
"certified_by",
"certification_details",
"changed_on",
"changed_on_humanized",
}
@@ -1339,7 +1402,6 @@ class TestDatasetDefaultColumnFiltering:
# Verify non-default columns are NOT present (not even with null values)
non_default_columns = [
"description",
"database_name",
"changed_by",
"columns",

19
tests/unit_tests/mcp_service/system/tool/test_get_schema.py
View File

@@ -153,16 +153,18 @@ class TestModelSchemaInfo:
def test_chart_default_columns(self):
"""Test chart default columns include required minimal set."""
required_columns = {
assert set(CHART_DEFAULT_COLUMNS) == {
"id",
"slice_name",
"viz_type",
"description",
"certified_by",
"certification_details",
"url",
"changed_on",
"changed_on_humanized",
}
assert required_columns.issubset(set(CHART_DEFAULT_COLUMNS))
# These should NOT be in defaults
assert "description" not in CHART_DEFAULT_COLUMNS
# Heavy columns should NOT be in defaults
assert "form_data" not in CHART_DEFAULT_COLUMNS
assert "uuid" not in CHART_DEFAULT_COLUMNS
@@ -291,12 +293,17 @@ class TestGetSchemaToolViaClient:
assert id_col["type"] == "int"
assert id_col["is_default"] is True
# Find a non-default column (description is on the model)
# description is now a default column
desc_col = next(
(c for c in select_cols if c["name"] == "description"), None
)
assert desc_col is not None
assert desc_col["is_default"] is False
assert desc_col["is_default"] is True
# Find a non-default column (uuid is on the model but not default)
uuid_col = next((c for c in select_cols if c["name"] == "uuid"), None)
assert uuid_col is not None
assert uuid_col["is_default"] is False
class TestGetSchemaEdgeCases: