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/04/01 18:51:39 UTC

(superset) 04/15: fix(mcp): add TEMPORAL_RANGE filter for temporal x-axis in generate_chart (#38978)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 6.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 27c9d806f1e5c61d2a305d8cbad37fa02239f462
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Tue Mar 31 18:39:08 2026 +0200

    fix(mcp): add TEMPORAL_RANGE filter for temporal x-axis in generate_chart (#38978)
    
    (cherry picked from commit c37a3ec292dc3e0152879819ea3072a1caa0c38a)
---
 superset/mcp_service/chart/chart_utils.py          |  33 +++++
 .../mcp_service/chart/test_chart_utils.py          | 134 ++++++++++++++++++++-
 .../explore/tool/test_generate_explore_link.py     |   6 +-
 3 files changed, 166 insertions(+), 7 deletions(-)

diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py
index 62704a0b5ef..b4fdfd3b13e 100644
--- a/superset/mcp_service/chart/chart_utils.py
+++ b/superset/mcp_service/chart/chart_utils.py
@@ -26,6 +26,7 @@ import logging
 from dataclasses import dataclass
 from typing import Any, Dict
 
+from superset.constants import NO_TIME_RANGE
 from superset.mcp_service.chart.schemas import (
     BigNumberChartConfig,
     ChartCapabilities,
@@ -41,6 +42,7 @@ from superset.mcp_service.chart.schemas import (
 )
 from superset.mcp_service.utils.url_utils import get_superset_base_url
 from superset.utils import json
+from superset.utils.core import FilterOperator
 
 logger = logging.getLogger(__name__)
 
@@ -545,6 +547,7 @@ def configure_temporal_handling(
     Stores any warnings in ``form_data["_mcp_warnings"]``.
     """
     if x_is_temporal:
+        form_data["granularity_sqla"] = form_data.get("x_axis")
         if time_grain:
             form_data["time_grain_sqla"] = time_grain
     else:
@@ -561,6 +564,33 @@ def configure_temporal_handling(
             )
 
 
+def _ensure_temporal_adhoc_filter(form_data: Dict[str, Any], column: str) -> None:
+    """Ensure a TEMPORAL_RANGE adhoc filter exists for the given column.
+
+    Mirrors the Explore UI behavior: when a temporal column is set as
+    the x-axis, a TEMPORAL_RANGE filter must be present so dashboard
+    time-range filters can bind to it.  Without this filter, Explore
+    shows a warning dialog asking the user to add it manually.
+    """
+    existing = form_data.get("adhoc_filters", [])
+    if any(
+        f.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+        and f.get("subject") == column
+        for f in existing
+    ):
+        return
+    existing.append(
+        {
+            "clause": "WHERE",
+            "expressionType": "SIMPLE",
+            "subject": column,
+            "operator": FilterOperator.TEMPORAL_RANGE.value,
+            "comparator": NO_TIME_RANGE,
+        }
+    )
+    form_data["adhoc_filters"] = existing
+
+
 def map_xy_config(
     config: XYChartConfig, dataset_id: int | str | None = None
 ) -> Dict[str, Any]:
@@ -618,6 +648,9 @@ def map_xy_config(
 
     _add_adhoc_filters(form_data, config.filters)
 
+    if x_is_temporal:
+        _ensure_temporal_adhoc_filter(form_data, config.x.name)
+
     form_data["row_limit"] = config.row_limit
 
     # Add stacking configuration
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 53ff6847983..293adfb09c3 100644
--- a/tests/unit_tests/mcp_service/chart/test_chart_utils.py
+++ b/tests/unit_tests/mcp_service/chart/test_chart_utils.py
@@ -22,8 +22,10 @@ from unittest.mock import MagicMock, patch
 
 import pytest
 
+from superset.constants import NO_TIME_RANGE
 from superset.mcp_service.chart.chart_utils import (
     _add_adhoc_filters,
+    _ensure_temporal_adhoc_filter,
     adhoc_filters_to_query_filters,
     configure_temporal_handling,
     create_metric_object,
@@ -44,7 +46,7 @@ from superset.mcp_service.chart.schemas import (
     TableChartConfig,
     XYChartConfig,
 )
-from superset.utils.core import GenericDataType
+from superset.utils.core import FilterOperator, GenericDataType
 
 
 class TestCreateMetricObject:
@@ -665,10 +667,13 @@ class TestMapXYConfig:
         result = map_xy_config(config)
 
         assert "adhoc_filters" in result
-        assert len(result["adhoc_filters"]) == 1
+        # User filter + auto-added TEMPORAL_RANGE filter for temporal x-axis
+        assert len(result["adhoc_filters"]) == 2
         assert result["adhoc_filters"][0]["subject"] == "region"
         assert result["adhoc_filters"][0]["operator"] == "=="
         assert result["adhoc_filters"][0]["comparator"] == "US"
+        assert result["adhoc_filters"][1]["operator"] == "TEMPORAL_RANGE"
+        assert result["adhoc_filters"][1]["subject"] == "date"
 
     @patch("superset.mcp_service.chart.chart_utils.is_column_truly_temporal")
     def test_map_xy_config_row_limit(self, mock_is_temporal) -> None:
@@ -1284,16 +1289,18 @@ class TestConfigureTemporalHandling:
     """Test configure_temporal_handling function"""
 
     def test_temporal_column_with_time_grain(self) -> None:
-        """Test temporal column sets time_grain_sqla"""
-        form_data: dict[str, Any] = {}
+        """Test temporal column sets time_grain_sqla and granularity_sqla"""
+        form_data: dict[str, Any] = {"x_axis": "order_date"}
         configure_temporal_handling(form_data, x_is_temporal=True, time_grain="P1M")
         assert form_data["time_grain_sqla"] == "P1M"
+        assert form_data["granularity_sqla"] == "order_date"
 
     def test_temporal_column_without_time_grain(self) -> None:
-        """Test temporal column without time_grain doesn't set time_grain_sqla"""
-        form_data: dict[str, Any] = {}
+        """Test temporal column sets granularity_sqla but not time_grain_sqla"""
+        form_data: dict[str, Any] = {"x_axis": "order_date"}
         configure_temporal_handling(form_data, x_is_temporal=True, time_grain=None)
         assert "time_grain_sqla" not in form_data
+        assert form_data["granularity_sqla"] == "order_date"
 
     def test_non_temporal_column_sets_categorical_config(self) -> None:
         """Test non-temporal column sets categorical configuration"""
@@ -1355,7 +1362,16 @@ class TestMapXYConfigWithNonTemporalColumn:
 
         assert result["x_axis"] == "created_at"
         assert result["time_grain_sqla"] == "P1W"
+        assert result["granularity_sqla"] == "created_at"
         assert "x_axis_sort_series_type" not in result
+        # Temporal x-axis should have a TEMPORAL_RANGE adhoc filter
+        temporal_filters = [
+            f
+            for f in result.get("adhoc_filters", [])
+            if f.get("operator") == "TEMPORAL_RANGE"
+        ]
+        assert len(temporal_filters) == 1
+        assert temporal_filters[0]["subject"] == "created_at"
 
     @patch("superset.mcp_service.chart.chart_utils.is_column_truly_temporal")
     def test_non_temporal_ignores_time_grain_param(self, mock_is_temporal) -> None:
@@ -1377,6 +1393,112 @@ class TestMapXYConfigWithNonTemporalColumn:
         assert result["x_axis_sort_series_type"] == "name"
 
 
+class TestEnsureTemporalAdhocFilter:
+    """Test _ensure_temporal_adhoc_filter helper and its integration in map_xy_config"""
+
+    def test_adds_filter_to_empty_form_data(self) -> None:
+        """Test adds TEMPORAL_RANGE filter when no adhoc_filters exist"""
+        form_data: dict[str, Any] = {}
+        _ensure_temporal_adhoc_filter(form_data, "order_date")
+
+        assert len(form_data["adhoc_filters"]) == 1
+        f = form_data["adhoc_filters"][0]
+        assert f["operator"] == FilterOperator.TEMPORAL_RANGE.value
+        assert f["subject"] == "order_date"
+        assert f["comparator"] == NO_TIME_RANGE
+        assert f["expressionType"] == "SIMPLE"
+        assert f["clause"] == "WHERE"
+
+    def test_appends_to_existing_filters(self) -> None:
+        """Test appends temporal filter after existing user filters"""
+        form_data: dict[str, Any] = {
+            "adhoc_filters": [
+                {"subject": "region", "operator": "==", "comparator": "US"}
+            ]
+        }
+        _ensure_temporal_adhoc_filter(form_data, "order_date")
+
+        assert len(form_data["adhoc_filters"]) == 2
+        assert form_data["adhoc_filters"][0]["subject"] == "region"
+        assert (
+            form_data["adhoc_filters"][1]["operator"]
+            == FilterOperator.TEMPORAL_RANGE.value
+        )
+
+    def test_does_not_duplicate_existing_temporal_filter(self) -> None:
+        """Test skips adding if a TEMPORAL_RANGE filter already exists for the column"""
+        form_data: dict[str, Any] = {
+            "adhoc_filters": [
+                {
+                    "subject": "order_date",
+                    "operator": FilterOperator.TEMPORAL_RANGE.value,
+                    "comparator": "Last 7 days",
+                }
+            ]
+        }
+        _ensure_temporal_adhoc_filter(form_data, "order_date")
+
+        # Should still be just 1 filter (no duplicate)
+        assert len(form_data["adhoc_filters"]) == 1
+
+    def test_adds_filter_for_different_column(self) -> None:
+        """Test adds filter when existing temporal filter is on a different column"""
+        form_data: dict[str, Any] = {
+            "adhoc_filters": [
+                {
+                    "subject": "created_at",
+                    "operator": FilterOperator.TEMPORAL_RANGE.value,
+                    "comparator": NO_TIME_RANGE,
+                }
+            ]
+        }
+        _ensure_temporal_adhoc_filter(form_data, "order_date")
+
+        assert len(form_data["adhoc_filters"]) == 2
+
+    @patch("superset.mcp_service.chart.chart_utils.is_column_truly_temporal")
+    def test_temporal_x_axis_adds_filter_in_map_xy(self, mock_is_temporal) -> None:
+        """Test map_xy_config adds TEMPORAL_RANGE filter for temporal x-axis"""
+        mock_is_temporal.return_value = True
+        config = XYChartConfig(
+            chart_type="xy",
+            x=ColumnRef(name="order_date"),
+            y=[ColumnRef(name="revenue", aggregate="SUM")],
+            kind="bar",
+        )
+
+        result = map_xy_config(config, dataset_id=123)
+
+        temporal_filters = [
+            f
+            for f in result.get("adhoc_filters", [])
+            if f.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+        ]
+        assert len(temporal_filters) == 1
+        assert temporal_filters[0]["subject"] == "order_date"
+        assert temporal_filters[0]["comparator"] == NO_TIME_RANGE
+
+    @patch("superset.mcp_service.chart.chart_utils.is_column_truly_temporal")
+    def test_non_temporal_x_axis_no_temporal_filter(self, mock_is_temporal) -> None:
+        """Test non-temporal x-axis skips TEMPORAL_RANGE filter"""
+        mock_is_temporal.return_value = False
+        config = XYChartConfig(
+            chart_type="xy",
+            x=ColumnRef(name="year"),
+            y=[ColumnRef(name="sales", aggregate="SUM")],
+            kind="bar",
+        )
+
+        result = map_xy_config(config, dataset_id=123)
+
+        temporal_filters = [
+            f
+            for f in result.get("adhoc_filters", [])
+            if f.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+        ]
+        assert len(temporal_filters) == 0
+
+
 class TestFilterConfigValidation:
     """Test FilterConfig validation for new operators"""
 
diff --git a/tests/unit_tests/mcp_service/explore/tool/test_generate_explore_link.py b/tests/unit_tests/mcp_service/explore/tool/test_generate_explore_link.py
index 264868168af..fb8aee539b0 100644
--- a/tests/unit_tests/mcp_service/explore/tool/test_generate_explore_link.py
+++ b/tests/unit_tests/mcp_service/explore/tool/test_generate_explore_link.py
@@ -879,8 +879,12 @@ class TestGenerateExploreLinkColumnNormalization:
             assert form_data["x_axis"] == "OrderDate"
             # filter subject normalized to match x-axis
             adhoc_filters = form_data.get("adhoc_filters", [])
-            assert len(adhoc_filters) == 1
+            # User filter + auto-added TEMPORAL_RANGE for temporal x-axis
+            assert len(adhoc_filters) == 2
             assert adhoc_filters[0]["subject"] == "OrderDate"
+            assert adhoc_filters[0]["operator"] == ">"
+            assert adhoc_filters[1]["operator"] == "TEMPORAL_RANGE"
+            assert adhoc_filters[1]["subject"] == "OrderDate"
 
     @patch(
         "superset.mcp_service.chart.validation.dataset_validator.DatasetValidator._get_dataset_context"