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 88efac09c67 fix(mcp): align histogram and waterfall query contracts
(#44744)
88efac09c67 is described below
commit 88efac09c670f2214df9bb91e9c3beaf8e40932d
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Thu Oct 1 06:13:38 2026 +1000
fix(mcp): align histogram and waterfall query contracts (#44744)
---
superset/mcp_service/chart/plugins/waterfall.py | 8 ++
.../chart/tool/get_chart_type_schema.py | 4 +-
.../mcp_service/chart/test_chart_helpers.py | 109 +++++++++++++++++++++
.../mcp_service/chart/test_preview_utils.py | 38 +++++++
.../chart/tool/test_get_chart_type_schema.py | 10 ++
5 files changed, 167 insertions(+), 2 deletions(-)
diff --git a/superset/mcp_service/chart/plugins/waterfall.py
b/superset/mcp_service/chart/plugins/waterfall.py
index 3a5f58a059b..c6f2e4969ed 100644
--- a/superset/mcp_service/chart/plugins/waterfall.py
+++ b/superset/mcp_service/chart/plugins/waterfall.py
@@ -210,4 +210,12 @@ class WaterfallChartPlugin(BaseChartPlugin):
form_data, columns, resolve_shared_metrics(form_data),
row_limit=row_limit
)
query["orderby"] = [(column, True) for column in columns]
+ # Bind the time grain to the SQL time column, as extractExtras does.
+ granularity = form_data.get("granularity",
form_data.get("granularity_sqla"))
+ if granularity is not None:
+ query["granularity"] = granularity
+ if form_data.get("time_grain_sqla") is not None:
+ query.setdefault("extras", {})["time_grain_sqla"] = form_data[
+ "time_grain_sqla"
+ ]
return [query]
diff --git a/superset/mcp_service/chart/tool/get_chart_type_schema.py
b/superset/mcp_service/chart/tool/get_chart_type_schema.py
index 3e09d888e1b..36b45d784c9 100644
--- a/superset/mcp_service/chart/tool/get_chart_type_schema.py
+++ b/superset/mcp_service/chart/tool/get_chart_type_schema.py
@@ -260,8 +260,8 @@ _CHART_EXAMPLES: Dict[str, list[Dict[str, Any]]] = {
},
{
"chart_type": "treemap_v2",
- "groupby": ["region", "product"],
- "metric": "total_revenue",
+ "groupby": [{"name": "region"}, {"name": "product"}],
+ "metric": {"name": "total_revenue", "saved_metric": True},
"show_labels": True,
"show_upper_labels": True,
"label_type": "key_value",
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 6b18150dcf4..d7df374f792 100644
--- a/tests/unit_tests/mcp_service/chart/test_chart_helpers.py
+++ b/tests/unit_tests/mcp_service/chart/test_chart_helpers.py
@@ -1329,3 +1329,112 @@ def
test_shared_query_builder_keeps_mixed_timeseries_ordering_per_query(
expected_secondary["orderby"] = secondary_orderby
assert secondary == expected_secondary
assert form_data["orderby"] == [["count", True]]
+
+
[email protected]("groupby", [[], ["region"]])
[email protected]("having", [False, True])
+def test_histogram_query_matches_frontend_contract(
+ monkeypatch: pytest.MonkeyPatch, groupby: list[str], having: bool
+) -> None:
+ """Histogram queries select raw observations and run the binning
operator."""
+ monkeypatch.setattr(
+ "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine",
+ lambda *_: "base",
+ )
+ form_data = {
+ "viz_type": "histogram_v2",
+ "column": "value",
+ "groupby": groupby,
+ "bins": 3,
+ "normalize": True,
+ "cumulative": True,
+ "row_limit": 100,
+ "adhoc_filters": [
+ {
+ "clause": "HAVING",
+ "expressionType": "SQL",
+ "sqlExpression": "COUNT(*) > 0",
+ }
+ ]
+ if having
+ else [],
+ }
+ query = build_query_dicts_from_form_data(form_data, 1, "table")[0]
+ assert query["columns"] == [*groupby, "value"]
+ assert bool(query["metrics"]) is having
+ assert query["post_processing"] == [
+ {
+ "operation": "histogram",
+ "options": {
+ "column": "value",
+ "groupby": groupby,
+ "bins": 3,
+ "normalize": True,
+ "cumulative": True,
+ },
+ }
+ ]
+ assert query["row_limit"] == 100
+
+
[email protected]("axis_key", ["x_axis", "granularity_sqla"])
+def test_waterfall_query_preserves_category_and_order(
+ monkeypatch: pytest.MonkeyPatch, axis_key: str
+) -> None:
+ """Waterfall queries retain the axis and breakdown instead of a grand
total."""
+ monkeypatch.setattr(
+ "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine",
+ lambda *_: "base",
+ )
+ query = build_query_dicts_from_form_data(
+ {
+ "viz_type": "waterfall",
+ axis_key: "category",
+ "groupby": ["region"],
+ "metric": "revenue",
+ "row_limit": 20,
+ },
+ 1,
+ "table",
+ )[0]
+ assert query["columns"] == ["category", "region"]
+ assert query["metrics"] == ["revenue"]
+ assert query["orderby"] == [("category", True), ("region", True)]
+
+
[email protected]("legacy_axis", [False, True])
[email protected]("time_grain", ["P1M", None])
+def test_waterfall_query_preserves_temporal_binding(
+ monkeypatch: pytest.MonkeyPatch, legacy_axis: bool, time_grain: str | None
+) -> None:
+ """A typed Waterfall's grain stays bound to its selected temporal
column."""
+ from superset.mcp_service.chart.chart_utils import map_waterfall_config
+ from superset.mcp_service.chart.schemas import WaterfallChartConfig
+
+ monkeypatch.setattr(
+ "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine",
+ lambda *_: "base",
+ )
+ form_data = map_waterfall_config(
+ WaterfallChartConfig.model_validate(
+ {
+ "x_axis": {"name": "event_time"},
+ "metric": {"name": "revenue", "aggregate": "SUM"},
+ "breakdown": {"name": "region"},
+ "time_grain": time_grain,
+ }
+ )
+ )
+ if legacy_axis:
+ form_data["granularity_sqla"] = form_data.pop("x_axis")
+
+ query = build_query_dicts_from_form_data(form_data, 1, "table")[0]
+
+ assert query["columns"] == ["event_time", "region"]
+ assert query["orderby"] == [("event_time", True), ("region", True)]
+ assert query["metrics"] == [form_data["metric"]]
+ if time_grain or legacy_axis:
+ assert query["granularity"] == "event_time"
+ else:
+ assert "granularity" not in query
+ assert query.get("extras", {}).get("time_grain_sqla") == time_grain
diff --git a/tests/unit_tests/mcp_service/chart/test_preview_utils.py
b/tests/unit_tests/mcp_service/chart/test_preview_utils.py
index e269cb32cee..1852c900333 100644
--- a/tests/unit_tests/mcp_service/chart/test_preview_utils.py
+++ b/tests/unit_tests/mcp_service/chart/test_preview_utils.py
@@ -276,6 +276,44 @@ def test_unsaved_gauge_preview_surfaces_query_error(
assert "bad metric" in result.error
+def test_histogram_preview_uses_postprocessed_bins() -> None:
+ """The histogram preview must not bin or count already aggregated bin
counts."""
+ result = preview_utils._generate_vega_lite_preview_from_data(
+ [{"0 - 10": 4, "10 - 20": 7}], {"viz_type": "histogram_v2"}
+ )
+ assert result.specification["data"]["values"] == [
+ {"bin": "0 - 10", "value": 4, "series": "All"},
+ {"bin": "10 - 20", "value": 7, "series": "All"},
+ ]
+ encoding = result.specification["encoding"]
+ assert encoding["x"]["field"] == "bin"
+ assert "bin" not in encoding["x"]
+ assert encoding["y"]["field"] == "value"
+ assert "aggregate" not in encoding["y"]
+
+
[email protected]("viz_type", ["sankey_v2", "radar"])
+def test_vega_preview_rejects_unsupported_native_geometry(viz_type: str) ->
None:
+ """Unsupported geometry must not silently become an unrelated scatter
plot."""
+ result = preview_utils._generate_vega_lite_preview_from_data(
+ [{"source": "A", "target": "B", "value": 10}], {"viz_type": viz_type}
+ )
+ assert result.error_type == "UnsupportedFormat"
+ assert "Explore" in result.error
+
+
+def test_funnel_preview_binds_stage_and_metric() -> None:
+ """A funnel preview must encode its stages and values rather than a
scatter."""
+ result = preview_utils._generate_vega_lite_preview_from_data(
+ [{"stage": "Won", "SUM(value)": 10}],
+ {"viz_type": "funnel", "groupby": ["stage"], "metric": "SUM(value)"},
+ )
+ spec = result.specification
+ assert spec["mark"] == "bar"
+ assert spec["encoding"]["y"]["field"] == "stage"
+ assert spec["encoding"]["x"]["field"] == "SUM(value)"
+
+
@pytest.mark.parametrize("stage", [None, 42, {}, {"expressionType": "SQL"}])
def test_funnel_preview_rejects_invalid_stage(stage: object) -> None:
"""Malformed stage references return the invalid-form-data contract."""
diff --git
a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_type_schema.py
b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_type_schema.py
index 27b22ae76ce..8e250e0ebb6 100644
--- a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_type_schema.py
+++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_type_schema.py
@@ -174,3 +174,13 @@ def
test_schema_cache_compiles_once_and_isolates_response_mutation(
assert "groupby" in second["schema"]["properties"]
assert calls == 1
_compiled_chart_schema.cache_clear()
+
+
[email protected]("chart_type", VALID_CHART_TYPES)
+def test_discovery_examples_conform_to_advertised_json_schema(chart_type: str)
-> None:
+ """Clients can submit examples using the advertised schema without
coercion."""
+ from jsonschema import validate
+
+ response = _call_schema(chart_type)
+ for example in response["examples"]:
+ validate(example, response["schema"])
