diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py index 1d507909d91..41190554b3d 100644 --- a/superset/mcp_service/chart/chart_utils.py +++ b/superset/mcp_service/chart/chart_utils.py @@ -345,6 +345,27 @@ def _add_adhoc_filters( ] +def adhoc_filters_to_query_filters( + adhoc_filters: list[Dict[str, Any]], +) -> list[Dict[str, Any]]: + """Convert adhoc filter format to QueryObject filter format. + + Adhoc filters use ``{subject, operator, comparator}`` keys while + ``QueryContextFactory`` expects ``{col, op, val}`` (QueryObjectFilterClause). + """ + result: list[Dict[str, Any]] = [] + for f in adhoc_filters: + if f.get("expressionType") == "SIMPLE": + result.append( + { + "col": f.get("subject"), + "op": f.get("operator"), + "val": f.get("comparator"), + } + ) + return result + + def map_table_config(config: TableChartConfig) -> Dict[str, Any]: """Map table chart config to form_data with defensive validation.""" # Early validation to prevent empty charts diff --git a/superset/mcp_service/chart/preview_utils.py b/superset/mcp_service/chart/preview_utils.py index 5b930797b06..dcc8642f000 100644 --- a/superset/mcp_service/chart/preview_utils.py +++ b/superset/mcp_service/chart/preview_utils.py @@ -81,11 +81,17 @@ def generate_preview_from_form_data( # Create query context from form data using factory from superset.common.query_context_factory import QueryContextFactory + from superset.mcp_service.chart.chart_utils import ( + adhoc_filters_to_query_filters, + ) # Build columns list: include x_axis and groupby for XY charts, # fall back to form_data "columns" for table charts columns = _build_query_columns(form_data) + query_filters = adhoc_filters_to_query_filters( + form_data.get("adhoc_filters", []) + ) factory = QueryContextFactory() query_context_obj = factory.create( datasource={"id": dataset_id, "type": "table"}, @@ -95,7 +101,7 @@ def generate_preview_from_form_data( "metrics": form_data.get("metrics", []), "orderby": form_data.get("orderby", []), "row_limit": form_data.get("row_limit", 100), - "filters": form_data.get("adhoc_filters", []), + "filters": query_filters, "time_range": form_data.get("time_range", "No filter"), } ], diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py index 64f9e03804f..4a439d3bd74 100644 --- a/superset/mcp_service/chart/tool/generate_chart.py +++ b/superset/mcp_service/chart/tool/generate_chart.py @@ -81,10 +81,14 @@ def _compile_chart( ChartDataQueryFailedError, ) from superset.common.query_context_factory import QueryContextFactory + from superset.mcp_service.chart.chart_utils import adhoc_filters_to_query_filters from superset.mcp_service.chart.preview_utils import _build_query_columns try: columns = _build_query_columns(form_data) + query_filters = adhoc_filters_to_query_filters( + form_data.get("adhoc_filters", []) + ) factory = QueryContextFactory() query_context = factory.create( datasource={"id": dataset_id, "type": "table"}, @@ -94,7 +98,7 @@ def _compile_chart( "metrics": form_data.get("metrics", []), "orderby": form_data.get("orderby", []), "row_limit": 2, - "filters": form_data.get("adhoc_filters", []), + "filters": query_filters, "time_range": form_data.get("time_range", "No filter"), } ], diff --git a/tests/unit_tests/mcp_service/chart/test_chart_utils.py b/tests/unit_tests/mcp_service/chart/test_chart_utils.py index 480399f5cec..1a247bf1e89 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_utils.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_utils.py @@ -24,6 +24,7 @@ import pytest from superset.mcp_service.chart.chart_utils import ( _add_adhoc_filters, + adhoc_filters_to_query_filters, configure_temporal_handling, create_metric_object, generate_chart_name, @@ -1447,3 +1448,56 @@ class TestValidateChartDataset: mock_find.side_effect = SQLAlchemyError("db gone") url = generate_explore_link(5, {"viz_type": "table"}) assert "datasource_id=5" in url + + +class TestAdhocFiltersToQueryFilters: + """Tests for adhoc_filters_to_query_filters conversion.""" + + def test_converts_simple_filters(self) -> None: + adhoc = [ + { + "clause": "WHERE", + "expressionType": "SIMPLE", + "subject": "genre", + "operator": "==", + "comparator": "Action", + } + ] + result = adhoc_filters_to_query_filters(adhoc) + assert result == [{"col": "genre", "op": "==", "val": "Action"}] + + def test_converts_multiple_filters(self) -> None: + adhoc = [ + { + "clause": "WHERE", + "expressionType": "SIMPLE", + "subject": "genre", + "operator": "==", + "comparator": "Action", + }, + { + "clause": "WHERE", + "expressionType": "SIMPLE", + "subject": "year", + "operator": ">=", + "comparator": "2010", + }, + ] + result = adhoc_filters_to_query_filters(adhoc) + assert len(result) == 2 + assert result[0] == {"col": "genre", "op": "==", "val": "Action"} + assert result[1] == {"col": "year", "op": ">=", "val": "2010"} + + def test_empty_list(self) -> None: + assert adhoc_filters_to_query_filters([]) == [] + + def test_skips_non_simple_expression_types(self) -> None: + adhoc = [ + { + "clause": "WHERE", + "expressionType": "SQL", + "sqlExpression": "col > 5", + } + ] + result = adhoc_filters_to_query_filters(adhoc) + assert result == []