diff --git a/superset/mcp_service/chart/chart_helpers.py b/superset/mcp_service/chart/chart_helpers.py index b6f2b5b30dc5..66023c1d69b3 100644 --- a/superset/mcp_service/chart/chart_helpers.py +++ b/superset/mcp_service/chart/chart_helpers.py @@ -260,6 +260,140 @@ def merge_extra_form_data_filters_into_query( merge_form_data_filters_into_query(query, extra_query_form_data) +def _deck_gl_spatial_cols(spatial: dict[str, Any] | None) -> list[str]: + """Return the column names referenced by a single Deck.gl spatial control.""" + if not isinstance(spatial, dict): + return [] + spatial_type = spatial.get("type") + if spatial_type == "latlong": + return [c for c in [spatial.get("lonCol"), spatial.get("latCol")] if c] + if spatial_type == "delimited": + return [c for c in [spatial.get("lonlatCol")] if c] + if spatial_type == "geohash": + return [c for c in [spatial.get("geohashCol")] if c] + return [] + + +def _deck_gl_tooltip_cols(tooltip_contents: list[Any] | None) -> list[str]: + """Return column names from Deck.gl tooltip_contents config.""" + cols: list[str] = [] + for item in tooltip_contents or []: + if isinstance(item, str): + cols.append(item) + elif isinstance(item, dict) and item.get("item_type") == "column": + col = item.get("column_name") + if isinstance(col, str) and col: + cols.append(col) + return cols + + +def _is_metric_ref(value: Any) -> bool: + """Return True if value is a metric reference (dict or non-numeric string). + + Deck.gl size/metric fields hold either a dict metric definition or a + simple saved-metric string key (e.g. "count"). Scalar numeric strings + like "100" are fixed display settings and must not be treated as metrics. + """ + if isinstance(value, dict): + return True + if isinstance(value, str) and value: + try: + float(value) + return False + except ValueError: + return True + return False + + +def _deck_gl_null_filters(form_data: dict[str, Any]) -> list[dict[str, Any]]: + """Build IS NOT NULL simple filters for Deck.gl spatial and data columns. + + Mirrors BaseDeckGLViz.add_null_filters() behavior: spatial control columns, + line_column, and the geojson column are filtered for non-null values by + default. + """ + seen: set[str] = set() + result: list[dict[str, Any]] = [] + for key in ("spatial", "start_spatial", "end_spatial"): + for col in _deck_gl_spatial_cols(form_data.get(key)): + if col not in seen: + seen.add(col) + result.append({"col": col, "op": "IS NOT NULL", "val": ""}) + for field in ("line_column", "geojson"): + data_col = form_data.get(field) + if isinstance(data_col, str) and data_col and data_col not in seen: + seen.add(data_col) + result.append({"col": data_col, "op": "IS NOT NULL", "val": ""}) + return result + + +def _resolve_deck_gl_metrics( + form_data: dict[str, Any], viz_type: str = "" +) -> list[Any]: + """Extract metrics for Deck.gl chart types. + + deck_geojson.query_obj() forces metrics=[] regardless of form_data. + For other types, size/metric values are included when they are metric + references (dicts or non-numeric strings); numeric scalars like "100" + are fixed display settings and are excluded. + deck_scatter and deck_polygon can additionally store metric-backed + values in point_radius_fixed (radius for scatter, elevation for polygon). + """ + if viz_type == "deck_geojson": + return [] + metrics: list[Any] = [] + for field in ("size", "metric"): + m = form_data.get(field) + if _is_metric_ref(m): + metrics.append(m) + prf = form_data.get("point_radius_fixed") + if isinstance(prf, dict) and prf.get("type") == "metric": + value = prf.get("value") + if value: + metrics.append(value) + elif isinstance(prf, str) and _is_metric_ref(prf): + # Legacy deck_scatter: point_radius_fixed as a bare non-numeric metric key + metrics.append(prf) + return metrics + + +def resolve_deck_gl_columns(form_data: dict[str, Any]) -> list[str]: + """Extract SQL column names for Deck.gl chart types from form_data. + + Deck.gl charts use spatial controls (lat/lon pairs, geohash, etc.) + rather than the standard metrics/groupby structure. This function + maps those spatial control configs to the actual column names + needed by the SQL query. + """ + seen: set[str] = set() + columns: list[str] = [] + + def _add(col: str | None) -> None: + if col and isinstance(col, str) and col not in seen: + seen.add(col) + columns.append(col) + + # Most Deck.gl types use "spatial"; arc charts use start/end spatial + for key in ("spatial", "start_spatial", "end_spatial"): + for col in _deck_gl_spatial_cols(form_data.get(key)): + _add(col) + + # deck_path / deck_polygon use a line column; deck_geojson uses geojson + for field in ("line_column", "geojson", "dimension"): + _add(form_data.get(field)) + + for col in form_data.get("js_columns") or []: + if isinstance(col, str): + _add(col) + + for col in _deck_gl_tooltip_cols(form_data.get("tooltip_contents")): + _add(col) + + _add(form_data.get("cross_filter_column")) + + return columns + + def resolve_metrics(form_data: dict[str, Any], viz_type: str) -> list[Any]: """Extract metrics from form_data, handling chart-type-specific fields.""" if viz_type == "bubble": @@ -423,6 +557,25 @@ def build_query_dicts_from_form_data( or (getattr(chart, "viz_type", "") if chart else "") or "" ) + + # Deck.gl charts use spatial column configs rather than the standard + # metrics / groupby fields. Extract columns from the spatial controls. + if viz_type.startswith("deck_"): + deck_columns = resolve_deck_gl_columns(form_data) + deck_metrics = _resolve_deck_gl_metrics(form_data, viz_type) + qd = _build_single_query_dict( + form_data, + deck_columns, + deck_metrics, + row_limit=row_limit, + order_desc=order_desc, + ) + if form_data.get("filter_nulls", True): + null_filters = _deck_gl_null_filters(form_data) + if null_filters: + qd["filters"] = [*(qd.get("filters") or []), *null_filters] + return [qd] + is_timeseries = ( viz_type.startswith("echarts_timeseries") or viz_type == "mixed_timeseries" ) diff --git a/superset/mcp_service/chart/tool/get_chart_data.py b/superset/mcp_service/chart/tool/get_chart_data.py index 5b6c0b3a5a55..96809d9cd962 100644 --- a/superset/mcp_service/chart/tool/get_chart_data.py +++ b/superset/mcp_service/chart/tool/get_chart_data.py @@ -340,31 +340,10 @@ async def get_chart_data( # noqa: C901 # groupby-like fields (entity, series, columns): # world_map, treemap_v2, sunburst_v2, gauge_chart # Bubble charts use x/y/size as separate metric fields. + # Deck.gl charts (deck_arc, deck_scatter, etc.) use spatial + # column configs (lat/lon, geohash, etc.) instead. viz_type = chart.viz_type or "" - # Deck.gl chart types store spatial data (lat/lon) - # rather than traditional metrics/groupby. They - # require a saved query_context to retrieve data. - # Match by prefix to cover all current and future - # deck.gl viz types (deck_arc, deck_scatter, etc.). - if viz_type.startswith("deck_"): - await ctx.warning( - "Chart %s is a deck.gl visualization (%s) with no " - "saved query_context. Data retrieval requires " - "re-saving the chart in Superset." % (chart.id, viz_type) - ) - return ChartError( - error=( - f"Chart {chart.id} is a deck.gl visualization " - f"(type: {viz_type}) with no saved query_context. " - f"Deck.gl charts use spatial data (lat/lon) that " - f"cannot be reconstructed from form_data alone. " - f"Please open this chart in Superset and re-save " - f"it to generate a query_context." - ), - error_type="MissingQueryContext", - ) - fallback_queries = build_query_dicts_from_form_data( form_data, chart.datasource_id, diff --git a/tests/unit_tests/mcp_service/chart/test_chart_helpers.py b/tests/unit_tests/mcp_service/chart/test_chart_helpers.py index 964226ab0128..c3563fc8e33c 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_helpers.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_helpers.py @@ -18,6 +18,10 @@ from unittest.mock import MagicMock, patch from superset.mcp_service.chart.chart_helpers import ( + _deck_gl_null_filters, + _deck_gl_tooltip_cols, + _is_metric_ref, + _resolve_deck_gl_metrics, apply_form_data_filters_to_query, build_query_dicts_from_form_data, extract_form_data_key_from_url, @@ -26,6 +30,7 @@ merge_extra_form_data_filters_into_query, merge_form_data_filters_into_query, prepare_form_data_for_query, + resolve_deck_gl_columns, ) @@ -285,3 +290,601 @@ def test_merge_extra_form_data_filters_into_query_adds_only_extra_predicates( assert query["time_range"] == "No filter" assert query["granularity"] == "updated_at" assert query["time_grain_sqla"] == "P1D" + + +# --------------------------------------------------------------------------- +# resolve_deck_gl_columns +# --------------------------------------------------------------------------- + + +def test_resolve_deck_gl_columns_latlong(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "longitude", "latCol": "latitude"}, + } + assert resolve_deck_gl_columns(form_data) == ["longitude", "latitude"] + + +def test_resolve_deck_gl_columns_delimited(): + form_data = { + "spatial": {"type": "delimited", "lonlatCol": "coords"}, + } + assert resolve_deck_gl_columns(form_data) == ["coords"] + + +def test_resolve_deck_gl_columns_geohash(): + form_data = { + "spatial": {"type": "geohash", "geohashCol": "geo"}, + } + assert resolve_deck_gl_columns(form_data) == ["geo"] + + +def test_resolve_deck_gl_columns_arc_start_end(): + form_data = { + "start_spatial": { + "type": "latlong", + "lonCol": "start_lon", + "latCol": "start_lat", + }, + "end_spatial": {"type": "latlong", "lonCol": "end_lon", "latCol": "end_lat"}, + } + cols = resolve_deck_gl_columns(form_data) + assert cols == ["start_lon", "start_lat", "end_lon", "end_lat"] + + +def test_resolve_deck_gl_columns_path_line_column(): + form_data = { + "line_column": "path_wkt", + } + assert resolve_deck_gl_columns(form_data) == ["path_wkt"] + + +def test_resolve_deck_gl_columns_geojson(): + form_data = { + "geojson": "geom_col", + } + assert resolve_deck_gl_columns(form_data) == ["geom_col"] + + +def test_resolve_deck_gl_columns_with_dimension_and_js_columns(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "dimension": "category", + "js_columns": ["name", "value"], + } + cols = resolve_deck_gl_columns(form_data) + assert "lon" in cols + assert "lat" in cols + assert "category" in cols + assert "name" in cols + assert "value" in cols + + +def test_resolve_deck_gl_columns_deduplicates(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "dimension": "lon", # same as lonCol — should not duplicate + } + cols = resolve_deck_gl_columns(form_data) + assert cols.count("lon") == 1 + + +def test_resolve_deck_gl_columns_empty(): + assert resolve_deck_gl_columns({}) == [] + + +def test_resolve_deck_gl_columns_ignores_non_string_js_columns(): + form_data = { + "js_columns": [42, None, "valid_col"], + } + assert resolve_deck_gl_columns(form_data) == ["valid_col"] + + +# --------------------------------------------------------------------------- +# build_query_dicts_from_form_data — Deck.gl branch +# --------------------------------------------------------------------------- + + +def test_build_query_dicts_deck_scatter_latlong(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert len(queries) == 1 + assert queries[0]["columns"] == ["lon", "lat"] + assert queries[0]["metrics"] == [] + + +def test_build_query_dicts_deck_scatter_with_size_metric(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + metric = { + "expressionType": "SIMPLE", + "column": {"column_name": "sales"}, + "aggregate": "SUM", + } + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "size": metric, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert len(queries) == 1 + assert queries[0]["columns"] == ["lon", "lat"] + assert queries[0]["metrics"] == [metric] + + +def test_build_query_dicts_deck_arc(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_arc", + "start_spatial": { + "type": "latlong", + "lonCol": "origin_lon", + "latCol": "origin_lat", + }, + "end_spatial": {"type": "latlong", "lonCol": "dest_lon", "latCol": "dest_lat"}, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert len(queries) == 1 + assert queries[0]["columns"] == ["origin_lon", "origin_lat", "dest_lon", "dest_lat"] + assert queries[0]["metrics"] == [] + + +def test_build_query_dicts_deck_geojson(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_geojson", + "geojson": "geometry", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert len(queries) == 1 + assert queries[0]["columns"] == ["geometry"] + assert queries[0]["metrics"] == [] + + +def test_build_query_dicts_deck_hex_geohash(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_hex", + "spatial": {"type": "geohash", "geohashCol": "geohash"}, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert len(queries) == 1 + assert queries[0]["columns"] == ["geohash"] + + +def test_build_query_dicts_deck_path_with_row_limit(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_path", + "line_column": "path_col", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table", row_limit=50) + + assert queries[0]["columns"] == ["path_col"] + assert queries[0]["row_limit"] == 50 + + +# --------------------------------------------------------------------------- +# resolve_deck_gl_columns — tooltip_contents and cross_filter_column (Fix 1) +# --------------------------------------------------------------------------- + + +def test_resolve_deck_gl_columns_with_tooltip_contents_strings(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "tooltip_contents": ["name", "category"], + } + cols = resolve_deck_gl_columns(form_data) + assert "name" in cols + assert "category" in cols + + +def test_resolve_deck_gl_columns_with_tooltip_contents_dict_items(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "tooltip_contents": [ + {"item_type": "column", "column_name": "city"}, + {"item_type": "metric", "key": "sum__sales"}, # metric items ignored + ], + } + cols = resolve_deck_gl_columns(form_data) + assert "city" in cols + assert "sum__sales" not in cols + + +def test_resolve_deck_gl_columns_with_cross_filter_column(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "cross_filter_column": "region", + } + cols = resolve_deck_gl_columns(form_data) + assert "region" in cols + + +def test_resolve_deck_gl_columns_tooltip_deduplicates_with_spatial(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "tooltip_contents": ["lon"], # already in spatial cols + } + cols = resolve_deck_gl_columns(form_data) + assert cols.count("lon") == 1 + + +# --------------------------------------------------------------------------- +# _deck_gl_tooltip_cols +# --------------------------------------------------------------------------- + + +def test_deck_gl_tooltip_cols_strings(): + assert _deck_gl_tooltip_cols(["city", "state"]) == ["city", "state"] + + +def test_deck_gl_tooltip_cols_dict_column_items(): + result = _deck_gl_tooltip_cols([{"item_type": "column", "column_name": "country"}]) + assert result == ["country"] + + +def test_deck_gl_tooltip_cols_skips_metric_items(): + result = _deck_gl_tooltip_cols([{"item_type": "metric", "key": "sum__sales"}]) + assert result == [] + + +def test_deck_gl_tooltip_cols_none(): + assert _deck_gl_tooltip_cols(None) == [] + + +# --------------------------------------------------------------------------- +# _is_metric_ref +# --------------------------------------------------------------------------- + + +def test_is_metric_ref_dict(): + assert _is_metric_ref({"expressionType": "SIMPLE"}) is True + + +def test_is_metric_ref_string_key(): + assert _is_metric_ref("count") is True + assert _is_metric_ref("sum__sales") is True + + +def test_is_metric_ref_numeric_string_excluded(): + assert _is_metric_ref("100") is False + assert _is_metric_ref("3.14") is False + assert _is_metric_ref("0") is False + + +def test_is_metric_ref_integer_excluded(): + assert _is_metric_ref(100) is False + + +def test_is_metric_ref_none_and_empty(): + assert _is_metric_ref(None) is False + assert _is_metric_ref("") is False + + +# --------------------------------------------------------------------------- +# _resolve_deck_gl_metrics (Fix 2) +# --------------------------------------------------------------------------- + + +def test_resolve_deck_gl_metrics_no_metrics(): + assert _resolve_deck_gl_metrics({}) == [] + + +def test_resolve_deck_gl_metrics_size_field(): + metric = {"expressionType": "SIMPLE", "aggregate": "COUNT", "column": None} + result = _resolve_deck_gl_metrics({"size": metric}) + assert result == [metric] + + +def test_resolve_deck_gl_metrics_metric_field(): + metric = {"expressionType": "SIMPLE", "aggregate": "SUM"} + result = _resolve_deck_gl_metrics({"metric": metric}) + assert result == [metric] + + +def test_resolve_deck_gl_metrics_point_radius_fixed_metric(): + prf_metric = {"expressionType": "SIMPLE", "aggregate": "AVG"} + prf = {"type": "metric", "value": prf_metric} + result = _resolve_deck_gl_metrics({"point_radius_fixed": prf}) + assert result == [prf_metric] + + +def test_resolve_deck_gl_metrics_point_radius_fixed_not_metric(): + prf = {"type": "fix", "value": 100} + result = _resolve_deck_gl_metrics({"point_radius_fixed": prf}) + assert result == [] + + +def test_resolve_deck_gl_metrics_polygon_both_metric_and_prf(): + base_metric = {"expressionType": "SIMPLE", "aggregate": "SUM"} + elevation_metric = {"expressionType": "SIMPLE", "aggregate": "AVG"} + prf = {"type": "metric", "value": elevation_metric} + result = _resolve_deck_gl_metrics( + {"metric": base_metric, "point_radius_fixed": prf} + ) + assert result == [base_metric, elevation_metric] + + +def test_resolve_deck_gl_metrics_geojson_returns_empty(): + # deck_geojson.query_obj() forces metrics=[] regardless of form_data + metric = {"expressionType": "SIMPLE", "aggregate": "SUM"} + result = _resolve_deck_gl_metrics( + {"size": metric, "metric": metric}, "deck_geojson" + ) + assert result == [] + + +def test_resolve_deck_gl_metrics_scalar_size_excluded(): + # Numeric string size values (fixed display settings) must not be metrics + result = _resolve_deck_gl_metrics({"size": "100"}, "deck_hex") + assert result == [] + + +def test_resolve_deck_gl_metrics_integer_size_excluded(): + result = _resolve_deck_gl_metrics({"size": 100}, "deck_path") + assert result == [] + + +def test_resolve_deck_gl_metrics_string_metric_included(): + # Non-numeric string metrics (saved metric keys) must be preserved + result = _resolve_deck_gl_metrics({"size": "count"}, "deck_hex") + assert result == ["count"] + + +def test_resolve_deck_gl_metrics_string_metric_field(): + result = _resolve_deck_gl_metrics({"metric": "sum__sales"}, "deck_arc") + assert result == ["sum__sales"] + + +def test_resolve_deck_gl_metrics_string_point_radius_fixed(): + # Legacy deck_scatter: point_radius_fixed as a bare metric key string + result = _resolve_deck_gl_metrics({"point_radius_fixed": "count"}, "deck_scatter") + assert result == ["count"] + + +def test_resolve_deck_gl_metrics_numeric_point_radius_fixed_excluded(): + # Numeric string point_radius_fixed is a fixed pixel radius, not a metric + result = _resolve_deck_gl_metrics({"point_radius_fixed": "100"}, "deck_scatter") + assert result == [] + + +# --------------------------------------------------------------------------- +# _deck_gl_null_filters (Fix 3) +# --------------------------------------------------------------------------- + + +def test_deck_gl_null_filters_latlong(): + form_data = { + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + } + result = _deck_gl_null_filters(form_data) + assert result == [ + {"col": "lon", "op": "IS NOT NULL", "val": ""}, + {"col": "lat", "op": "IS NOT NULL", "val": ""}, + ] + + +def test_deck_gl_null_filters_arc_start_end(): + form_data = { + "start_spatial": {"type": "latlong", "lonCol": "s_lon", "latCol": "s_lat"}, + "end_spatial": {"type": "latlong", "lonCol": "e_lon", "latCol": "e_lat"}, + } + result = _deck_gl_null_filters(form_data) + assert result == [ + {"col": "s_lon", "op": "IS NOT NULL", "val": ""}, + {"col": "s_lat", "op": "IS NOT NULL", "val": ""}, + {"col": "e_lon", "op": "IS NOT NULL", "val": ""}, + {"col": "e_lat", "op": "IS NOT NULL", "val": ""}, + ] + + +def test_deck_gl_null_filters_line_column(): + form_data = {"line_column": "path_col"} + result = _deck_gl_null_filters(form_data) + assert result == [{"col": "path_col", "op": "IS NOT NULL", "val": ""}] + + +def test_deck_gl_null_filters_empty(): + assert _deck_gl_null_filters({}) == [] + + +def test_deck_gl_null_filters_geojson_column(): + # geojson column gets an IS NOT NULL filter just like spatial columns + form_data = {"geojson": "geometry"} + assert _deck_gl_null_filters(form_data) == [ + {"col": "geometry", "op": "IS NOT NULL", "val": ""} + ] + + +# --------------------------------------------------------------------------- +# build_query_dicts_from_form_data — null filters behavior (Fix 3) +# --------------------------------------------------------------------------- + + +def test_build_query_dicts_deck_scatter_adds_null_filters_by_default(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert {"col": "lon", "op": "IS NOT NULL", "val": ""} in queries[0]["filters"] + assert {"col": "lat", "op": "IS NOT NULL", "val": ""} in queries[0]["filters"] + + +def test_build_query_dicts_deck_scatter_filter_nulls_false(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "filter_nulls": False, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + null_filters = [ + f for f in queries[0].get("filters", []) if f.get("op") == "IS NOT NULL" + ] + assert null_filters == [] + + +def test_build_query_dicts_deck_scatter_point_radius_fixed_metric(monkeypatch): + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + radius_metric = { + "expressionType": "SIMPLE", + "aggregate": "AVG", + "column": {"column_name": "radius"}, + } + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "point_radius_fixed": {"type": "metric", "value": radius_metric}, + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert queries[0]["metrics"] == [radius_metric] + + +def test_build_query_dicts_deck_geojson_scalar_size_produces_no_metrics(monkeypatch): + # Regression: deck_geojson fixture has size='100' (scalar, not a metric). + # The fallback must produce metrics=[] to match DeckGeoJson.query_obj(). + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_geojson", + "geojson": "geometry", + "size": "100", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert queries[0]["metrics"] == [] + + +def test_build_query_dicts_deck_path_scalar_size_produces_no_metrics(monkeypatch): + # deck_path fixture also has size='100' — scalar must not become a metric. + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_path", + "line_column": "path_col", + "size": "100", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert queries[0]["metrics"] == [] + + +def test_build_query_dicts_deck_geojson_adds_geojson_null_filter(monkeypatch): + # deck_geojson should add IS NOT NULL on the geojson column when filter_nulls + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_geojson", + "geojson": "geometry_col", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert {"col": "geometry_col", "op": "IS NOT NULL", "val": ""} in queries[0][ + "filters" + ] + + +def test_build_query_dicts_deck_hex_string_metric(monkeypatch): + # Non-numeric string size (saved metric key) must be included as a metric + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_hex", + "spatial": {"type": "geohash", "geohashCol": "geo"}, + "size": "count", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert queries[0]["metrics"] == ["count"] + + +def test_build_query_dicts_deck_scatter_string_point_radius_fixed(monkeypatch): + # Legacy deck_scatter with point_radius_fixed as a bare metric key string + monkeypatch.setattr( + "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine", + lambda datasource_id, datasource_type: "base", + ) + form_data = { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "lon", "latCol": "lat"}, + "point_radius_fixed": "count", + "adhoc_filters": [], + } + + queries = build_query_dicts_from_form_data(form_data, 1, "table") + + assert queries[0]["metrics"] == ["count"]