Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions superset/mcp_service/chart/chart_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from dataclasses import dataclass
from typing import Any, Dict

from superset.constants import NO_TIME_RANGE
from superset.mcp_service.chart.schemas import (
ChartCapabilities,
ChartSemantics,
Expand All @@ -40,6 +41,7 @@
)
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__)

Expand Down Expand Up @@ -535,6 +537,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:
Expand All @@ -551,6 +554,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]:
Expand Down Expand Up @@ -608,6 +638,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
Expand Down
134 changes: 128 additions & 6 deletions tests/unit_tests/mcp_service/chart/test_chart_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@

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,
Expand All @@ -44,7 +46,7 @@
TableChartConfig,
XYChartConfig,
)
from superset.utils.core import GenericDataType
from superset.utils.core import FilterOperator, GenericDataType


class TestCreateMetricObject:
Expand Down Expand Up @@ -665,10 +667,13 @@ def test_map_xy_config_with_filters(self, mock_is_temporal) -> None:
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:
Expand Down Expand Up @@ -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"""
Expand Down Expand Up @@ -1355,7 +1362,16 @@ def test_temporal_column_allows_time_grain(self, mock_is_temporal) -> None:

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:
Expand All @@ -1377,6 +1393,112 @@ def test_non_temporal_ignores_time_grain_param(self, mock_is_temporal) -> None:
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"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -879,8 +879,12 @@ async def test_filter_column_normalized_in_form_data(
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"
Expand Down
Loading