You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/08/12 04:12:45 UTC
(superset) branch master updated: feat(mcp): filter asset lists by certification (#42656)
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 56573fa2cda feat(mcp): filter asset lists by certification (#42656)
56573fa2cda is described below
commit 56573fa2cda46da56f2499ce073127472dc86225
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Tue Aug 11 22:12:38 2026 -0600
feat(mcp): filter asset lists by certification (#42656)
---
superset/mcp_service/chart/schemas.py | 14 +++++
superset/mcp_service/chart/tool/list_charts.py | 14 ++++-
superset/mcp_service/dataset/schemas.py | 14 +++++
superset/mcp_service/dataset/tool/list_datasets.py | 13 ++++-
superset/mcp_service/mcp_core.py | 38 ++++++++-----
.../mcp_service/chart/tool/test_list_charts.py | 65 ++++++++++++++++++++++
.../mcp_service/dataset/tool/test_dataset_tools.py | 55 +++++++++++++++++-
7 files changed, 196 insertions(+), 17 deletions(-)
diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py
index 12bf88e769a..1ea3f330f71 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -36,6 +36,7 @@ from pydantic import (
field_validator,
model_serializer,
model_validator,
+ StrictBool,
ValidationError,
)
from typing_extensions import Self
@@ -2202,6 +2203,19 @@ class ListChartsRequest(
):
"""Request schema for list_charts with clear, unambiguous types."""
+ certified: Annotated[
+ StrictBool | None,
+ Field(
+ default=None,
+ description=(
+ "Filter by governance certification status. Use true to return "
+ "only certified charts (preferred when selecting governed "
+ "assets), false to return only uncertified charts, or omit to "
+ "return both (default)."
+ ),
+ ),
+ ]
+
deleted_state: Annotated[
Literal["include", "only"] | None,
Field(
diff --git a/superset/mcp_service/chart/tool/list_charts.py b/superset/mcp_service/chart/tool/list_charts.py
index 3c1b117597b..42c780b70c7 100644
--- a/superset/mcp_service/chart/tool/list_charts.py
+++ b/superset/mcp_service/chart/tool/list_charts.py
@@ -80,7 +80,9 @@ async def list_charts(
"""List charts with filtering and search.
Returns chart metadata including id, name, viz_type, URL, and last
- modified time.
+ modified time. Set ``request.certified`` to true to return only governed
+ charts; false returns only uncertified charts, while omitting it preserves
+ the unfiltered behavior.
**IMPORTANT**: All parameters must be wrapped in a ``request`` object.
Do NOT pass ``search``, ``page``, ``page_size``, etc. as top-level
@@ -124,7 +126,7 @@ async def list_charts(
)
)
- from superset.charts.filters import ChartDeletedStateFilter
+ from superset.charts.filters import ChartCertifiedFilter, ChartDeletedStateFilter
from superset.daos.chart import ChartDAO
from superset.mcp_service.common.schema_discovery import (
CHART_SORTABLE_COLUMNS,
@@ -179,6 +181,13 @@ async def list_charts(
try:
with event_logger.log_context(action="mcp.list_charts.query"):
+ custom_filters = None
+ if request.certified is not None:
+ custom_filters = {
+ "certified": tool.build_bound_filter(
+ ChartCertifiedFilter, request.certified
+ )
+ }
result = tool.run_tool(
filters=request.filters,
search=request.search,
@@ -190,6 +199,7 @@ async def list_charts(
created_by_me=request.created_by_me,
edited_by_me=request.edited_by_me,
deleted_state=request.deleted_state,
+ custom_filters=custom_filters,
)
count = len(result.charts) if hasattr(result, "charts") else 0
total_pages = getattr(result, "total_pages", None)
diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py
index 67b4e22a92b..89e43a036da 100644
--- a/superset/mcp_service/dataset/schemas.py
+++ b/superset/mcp_service/dataset/schemas.py
@@ -32,6 +32,7 @@ from pydantic import (
field_validator,
model_serializer,
model_validator,
+ StrictBool,
)
from superset.daos.base import ColumnOperator, ColumnOperatorEnum
@@ -246,6 +247,19 @@ class ListDatasetsRequest(
test_list_datasets_with_string_filters.
"""
+ certified: Annotated[
+ StrictBool | None,
+ Field(
+ default=None,
+ description=(
+ "Filter by governance certification status. Use true to return "
+ "only certified datasets (preferred when selecting governed "
+ "semantic-layer assets), false to return only uncertified "
+ "datasets, or omit to return both (default)."
+ ),
+ ),
+ ]
+
@field_validator("filters", mode="before")
@classmethod
def parse_filters(cls, v: Any) -> Any:
diff --git a/superset/mcp_service/dataset/tool/list_datasets.py b/superset/mcp_service/dataset/tool/list_datasets.py
index de3d22baee9..47cd9200400 100644
--- a/superset/mcp_service/dataset/tool/list_datasets.py
+++ b/superset/mcp_service/dataset/tool/list_datasets.py
@@ -94,7 +94,9 @@ async def list_datasets(
"""List datasets with filtering and search.
Returns dataset metadata including table name, schema, and last modified
- time.
+ time. Set ``request.certified`` to true to return only governed,
+ semantic-layer datasets; false returns only uncertified datasets, while
+ omitting it preserves the unfiltered behavior.
**IMPORTANT**: All parameters must be wrapped in a ``request`` object.
Do NOT pass ``search``, ``page``, ``page_size``, etc. as top-level
@@ -160,6 +162,7 @@ async def list_datasets(
try:
from superset.daos.dataset import DatasetDAO
+ from superset.datasets.filters import DatasetCertifiedFilter
from superset.mcp_service.common.schema_discovery import (
DATASET_SORTABLE_COLUMNS,
get_all_column_names,
@@ -191,6 +194,13 @@ async def list_datasets(
)
with event_logger.log_context(action="mcp.list_datasets.query"):
+ custom_filters = None
+ if request.certified is not None:
+ custom_filters = {
+ "certified": tool.build_bound_filter(
+ DatasetCertifiedFilter, request.certified
+ )
+ }
result = tool.run_tool(
filters=request.filters,
search=request.search,
@@ -201,6 +211,7 @@ async def list_datasets(
page_size=request.page_size,
created_by_me=request.created_by_me,
edited_by_me=request.edited_by_me,
+ custom_filters=custom_filters,
)
await ctx.info(
diff --git a/superset/mcp_service/mcp_core.py b/superset/mcp_service/mcp_core.py
index 6e1bd5fd21e..ac2bdbe7341 100644
--- a/superset/mcp_service/mcp_core.py
+++ b/superset/mcp_service/mcp_core.py
@@ -115,28 +115,30 @@ class BaseCore(ABC):
self.logger.warning(message)
-class DeletedStateBoundFilter:
- """Adapt a FAB deleted-state filter for ``BaseDAO.list`` custom_filters.
+class BoundFilter:
+ """Bind a caller value to a FAB filter used by ``BaseDAO.list``.
``BaseDAO.list`` invokes custom filters as ``apply(query, None)``, but the
- ``BaseDeletedStateFilter`` subclasses interpret ``None`` as "live rows
- only". Binding the value at construction lets the DAO-side invocation
- reach the FAB filter with the caller's actual ``include``/``only`` choice.
-
- ``model`` re-exposes the FAB filter's SoftDeleteMixin model class so the
- caller can scope the session visibility bypass without re-consulting the
- (Optional) filter-class attribute.
+ request value is already available to MCP callers. Binding it at
+ construction preserves that value for the DAO-side invocation.
"""
- def __init__(self, inner: Any, value: str, model: type) -> None:
+ def __init__(self, inner: Any, value: Any) -> None:
self._inner = inner
self._value = value
- self.model = model
def apply(self, query: Any, value: Any) -> Any:
return self._inner.apply(query, self._value)
+class DeletedStateBoundFilter(BoundFilter):
+ """Bound deleted-state filter carrying its visibility-bypass model."""
+
+ def __init__(self, inner: Any, value: str, model: type) -> None:
+ super().__init__(inner, value)
+ self.model = model
+
+
class ModelListCore(BaseCore, Generic[L]):
"""
Generic tool for listing model objects with filtering, search, pagination, and
@@ -340,6 +342,11 @@ class ModelListCore(BaseCore, Generic[L]):
inner = self._deleted_state_filter("id", datamodel)
return DeletedStateBoundFilter(inner, normalized, model)
+ def build_bound_filter(self, filter_class: type, value: Any) -> BoundFilter:
+ """Bind an MCP value to a FAB filter for this core's DAO model."""
+ datamodel = SQLAInterface(self.dao_class.model_cls, db.session)
+ return BoundFilter(filter_class("id", datamodel), value)
+
def run_tool(
self,
filters: Any | None = None,
@@ -352,6 +359,7 @@ class ModelListCore(BaseCore, Generic[L]):
created_by_me: bool = False,
edited_by_me: bool = False,
deleted_state: str | None = None,
+ custom_filters: Dict[str, Any] | None = None,
) -> L:
# Clamp page_size to MAX_PAGE_SIZE as defense-in-depth
page_size = min(page_size, MAX_PAGE_SIZE)
@@ -401,7 +409,9 @@ class ModelListCore(BaseCore, Generic[L]):
"search": search,
"columns_to_load": columns_to_load,
}
+ dao_custom_filters = dict(custom_filters or {})
if deleted_state_bound is not None:
+ dao_custom_filters["deleted_state"] = deleted_state_bound
# The soft-delete ORM listener appends ``deleted_at IS NULL`` at
# execution time, so the session-scoped bypass must span both
# executions inside DAO.list (count + fetch). The context manager
@@ -410,11 +420,13 @@ class ModelListCore(BaseCore, Generic[L]):
# which unhidden rows the caller may actually see.
with skip_visibility_filter(db.session, deleted_state_bound.model):
items, total_count = self._call_dao_list(
- custom_filters={"deleted_state": deleted_state_bound},
+ custom_filters=dao_custom_filters,
**dao_kwargs,
)
else:
- items, total_count = self._call_dao_list(**dao_kwargs)
+ items, total_count = self._call_dao_list(
+ custom_filters=dao_custom_filters or None, **dao_kwargs
+ )
# Serialize items
item_objs = []
for item in items:
diff --git a/tests/unit_tests/mcp_service/chart/tool/test_list_charts.py b/tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
index 4face4607af..0921ffd7bd6 100644
--- a/tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
+++ b/tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
@@ -20,6 +20,7 @@ Tests for the list_charts request schema
"""
import importlib
+from copy import copy
from unittest.mock import Mock, patch
import pytest
@@ -147,6 +148,12 @@ class TestListChartsRequestSchema:
assert request.page == 2
assert request.page_size == 50
+ @pytest.mark.parametrize("value", ["true", "false", 0, 1])
+ def test_certified_requires_json_boolean(self, value):
+ """Reject values that Pydantic's non-strict bool would coerce."""
+ with pytest.raises(ValueError, match="valid boolean"):
+ ListChartsRequest(certified=value)
+
def test_invalid_order_direction(self):
"""Test that invalid order direction raises validation error."""
with pytest.raises(ValueError, match="Input should be 'asc' or 'desc'"):
@@ -354,3 +361,61 @@ async def test_list_charts_no_arguments(mock_list, mcp_server):
result = await client.call_tool("list_charts", {})
data = json.loads(result.content[0].text)
assert "charts" in data
+
+
+@patch("superset.daos.chart.ChartDAO.list")
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+ ("certified", "expected_names"),
+ [
+ (True, ["Certified"]),
+ (False, ["Uncertified"]),
+ (None, ["Certified", "Uncertified"]),
+ ],
+)
+async def test_list_charts_certified_filter(
+ mock_list, mcp_server, mock_chart, certified, expected_names
+):
+ """Certification is opt-in and supports certified, uncertified, and all."""
+ certified_chart = copy(mock_chart)
+ certified_chart.id = 1
+ certified_chart.slice_name = "Certified"
+ certified_chart.certified_by = "Data Governance"
+ certified_chart.deleted_at = None
+ uncertified_chart = mock_chart
+ uncertified_chart.id = 2
+ uncertified_chart.slice_name = "Uncertified"
+ uncertified_chart.deleted_at = None
+ charts = [certified_chart, uncertified_chart]
+
+ def list_side_effect(**kwargs):
+ custom_filter = (kwargs.get("custom_filters") or {}).get("certified")
+ if custom_filter is None:
+ selected = charts
+ else:
+ query = Mock()
+ custom_filter.apply(query, None)
+ predicate = str(query.filter.call_args.args[0])
+ expected_predicate = (
+ "slices.certified_by IS NOT NULL"
+ if certified
+ else "slices.certified_by IS NULL"
+ )
+ assert expected_predicate in predicate
+ selected = [charts[0] if certified else charts[1]]
+ return selected, len(selected)
+
+ mock_list.side_effect = list_side_effect
+ request = ListChartsRequest(certified=certified)
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_charts", {"request": request.model_dump()}
+ )
+
+ data = json.loads(result.content[0].text)
+ actual_names = [chart["slice_name"] for chart in data["charts"]]
+ assert len(actual_names) == len(expected_names)
+ assert all(
+ expected in actual
+ for expected, actual in zip(expected_names, actual_names, strict=False)
+ )
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
index 08bbe9acdb7..09af923e5a0 100644
--- a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
@@ -53,6 +53,13 @@ get_dataset_info_module = importlib.import_module(
)
+@pytest.mark.parametrize("value", ["true", "false", 0, 1])
+def test_list_datasets_certified_requires_json_boolean(value):
+ """Reject values that Pydantic's non-strict bool would coerce."""
+ with pytest.raises(ValueError, match="valid boolean"):
+ ListDatasetsRequest(certified=value)
+
+
def _wrapped(value: str) -> str:
return f"{LLM_CONTEXT_OPEN_DELIMITER}\n{value}\n{LLM_CONTEXT_CLOSE_DELIMITER}"
@@ -183,7 +190,7 @@ def mock_auth():
@pytest.fixture(autouse=True)
-def allow_data_model_metadata():
+def allow_data_model_metadata(): # noqa: PT004
"""Keep dataset tests in the normal metadata-allowed path by default."""
with (
patch.object(
@@ -314,6 +321,52 @@ async def test_list_datasets_basic(mock_list, mcp_server):
assert "changed_on_humanized" in data["columns_loaded"]
+@patch("superset.daos.dataset.DatasetDAO.list")
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+ ("certified", "expected_names"),
+ [
+ (True, ["Certified"]),
+ (False, ["Uncertified"]),
+ (None, ["Certified", "Uncertified"]),
+ ],
+)
+async def test_list_datasets_certified_filter(
+ mock_list, mcp_server, certified, expected_names
+):
+ """Certification is opt-in and supports certified, uncertified, and all."""
+ certified_dataset = create_mock_dataset(1, "Certified")
+ certified_dataset.extra = '{"certification": {"certified_by": "Governance"}}'
+ uncertified_dataset = create_mock_dataset(2, "Uncertified")
+ datasets = [certified_dataset, uncertified_dataset]
+
+ def list_side_effect(**kwargs):
+ custom_filter = (kwargs.get("custom_filters") or {}).get("certified")
+ if custom_filter is None:
+ selected = datasets
+ else:
+ query = MagicMock()
+ custom_filter.apply(query, None)
+ predicate = str(query.filter.call_args.args[0])
+ if certified:
+ assert "lower(tables.extra) LIKE lower" in predicate
+ else:
+ assert "tables.extra NOT LIKE" in predicate
+ assert "tables.extra IS NULL" in predicate
+ selected = [datasets[0] if certified else datasets[1]]
+ return selected, len(selected)
+
+ mock_list.side_effect = list_side_effect
+ request = ListDatasetsRequest(certified=certified)
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_datasets", {"request": request.model_dump()}
+ )
+
+ data = json.loads(result.content[0].text)
+ assert [dataset["table_name"] for dataset in data["datasets"]] == expected_names
+
+
@patch("superset.daos.dataset.DatasetDAO.list")
@pytest.mark.asyncio
async def test_list_datasets_custom_uuid_columns(mock_list, mcp_server):