aminghadersohi commented on code in PR #44152:
URL: https://github.com/apache/superset/pull/44152#discussion_r3985134841
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1276,6 +1379,16 @@ def map_treemap_config(config: TreemapChartConfig) ->
Dict[str, Any]:
"row_limit": config.row_limit,
"color_scheme": config.color_scheme or "supersetColors",
}
+ for key in _TREEMAP_PRESENTATION_KEYS | {
+ "time_range",
+ "granularity_sqla",
+ "template_params",
+ }:
+ value = getattr(config, key)
+ if value is not None:
+ form_data[key] = (
+ value.model_dump() if hasattr(value, "model_dump") else value
+ )
Review Comment:
Confirmed and fixed in 6a47480. The shared Treemap query builder now maps
the selected granularity_sqla to QueryObject granularity, preserving normalized
dashboard override precedence. Six query-construction cases exercise
selected/cleared chart columns and present/null/omitted dashboard overrides;
null dashboard extras retain the shared no-override semantics. The
selected-column/override regression failed before the fix. All 96 Treemap
completeness cases pass, including real Vega rendering. Full branch-file
pre-commit passes; full MCP/common and exact-head CI are running.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1410,9 +1415,58 @@ class TreemapChartConfig(BaseChartConfig):
max_length=100,
)
+ show_labels: bool = True
+ show_upper_labels: bool = True
+ label_type: Literal["key", "Key", "value", "key_value"] = "key_value"
+ label_position: Literal[
+ "top",
+ "left",
+ "right",
+ "bottom",
+ "inside",
+ "insideLeft",
+ "insideRight",
+ "insideTop",
+ "insideBottom",
+ "insideTopLeft",
+ "insideBottomLeft",
+ "insideTopRight",
+ "insideBottomRight",
+ ] = "insideTopLeft"
+ number_format: str = Field("SMART_NUMBER", max_length=100)
+ date_format: str = Field("smart_date", max_length=100)
+ currency_format: CurrencyFormat | None = None
+ time_range: str | None = Field(None, max_length=1000)
+ granularity_sqla: str | None = Field(None, min_length=1, max_length=255)
+ template_params: str | None = Field(None, max_length=10000)
+
+ @model_validator(mode="before")
+ @classmethod
+ def adapt_native_form_data(cls, data: Any) -> Any:
+ """Accept native hierarchy and saved, SIMPLE, and SQL metric inputs."""
+ return _adapt_native_single_metric_form_data(data)
Review Comment:
Checked against the actual Pydantic/FastMCP entry paths: this is not
reproduced. test_native_request_roundtrip passes native FORM_DATA containing
viz_type and datasource through GenerateChartRequest and
UpdateChartPreviewRequest for saved, SIMPLE and SQL metrics.
test_registered_generate_chart_native_roundtrip additionally exercises native
viz_type/groupby/metric through the registered FastMCP entry. Subclass before
validators adapt the native envelope before the inherited unknown-field check.
Hostile/unknown controls remain rejected by the adjacent negative regressions;
no relaxation is needed.
##########
superset/mcp_service/chart/treemap_preview.py:
##########
@@ -0,0 +1,293 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Bounded slice-and-dice Treemap previews using explicit rectangle
geometry."""
+
+from __future__ import annotations
+
+import math
+from typing import Any
+
+from superset.mcp_service.chart.query_result import (
+ metric_result_label,
+ normalize_chart_query_result,
+)
+from superset.mcp_service.chart.schemas import ChartError, VegaLitePreview
+
+# Match the built-in frontend categorical schemes. Unknown schemes are not
guessed.
+_PALETTES = {
+ "supersetColors": [
+ "#1FA8C9",
+ "#454E7C",
+ "#5AC189",
+ "#FF7F44",
+ "#666666",
+ "#E04355",
+ "#FCC700",
+ "#A868B7",
+ "#3CCCCB",
+ "#A38F79",
+ "#8FD3E4",
+ "#A1A6BD",
+ "#ACE1C4",
+ "#FEC0A1",
+ "#B2B2B2",
+ "#EFA1AA",
+ "#FDE380",
+ "#D3B3DA",
+ "#9EE5E5",
+ "#D1C6BC",
+ ],
+ "lyftColors": [
+ "#EA0B8C",
+ "#6C838E",
+ "#29ABE2",
+ "#33D9C1",
+ "#9DACB9",
+ "#7560AA",
+ "#2D5584",
+ "#831C4A",
+ "#333D47",
+ "#AC2077",
+ ],
+}
+_MAX_ROWS = 1000
+
+
+def treemap_ascii(
+ data: list[dict[str, Any]], form_data: dict[str, Any], width: int = 80
+) -> str | ChartError:
+ """Show the ordered hierarchy and values rather than unrelated bar
geometry."""
+ checked = normalize_chart_query_result({"queries": [{"data": data}]},
form_data)
+ if isinstance(checked, ChartError):
+ return checked
+ label = metric_result_label(form_data["metric"])
+ assert label is not None
+ lines = [f"Treemap hierarchy | {label}"]
+ for row in data[:_MAX_ROWS]:
+ path = " > ".join(str(row[column]) for column in form_data["groupby"])
+ lines.append(f"{path} | {row[label]}")
+ if len(data) > _MAX_ROWS:
+ lines.append(f"Showing {_MAX_ROWS} of {len(data)} rows")
+ return "\n".join(line[:width] for line in lines)
+
+
+def treemap_vega_lite( # noqa: C901
+ data: list[dict[str, Any]], form_data: dict[str, Any]
+) -> VegaLitePreview | ChartError:
+ """Render nested metric-proportional rectangles; never substitute
scatter/bar marks.
+
+ Vega-Lite has no hierarchy transform. A bounded slice-and-dice layout is
+ computed here and sent as explicit coordinates, without executable hooks.
+ Native ECharts layout and dashboard interactions remain available in
Explore.
+ """
+ checked = normalize_chart_query_result({"queries": [{"data": data}]},
form_data)
+ if isinstance(checked, ChartError):
+ return checked
+ label = metric_result_label(form_data["metric"])
+ assert label is not None
+ if not data:
+ return ChartError(error="No Treemap data available.",
error_type="NoDataError")
+ scheme = form_data.get("color_scheme") or "supersetColors"
+ if (
+ scheme not in _PALETTES
+ or form_data.get("currency_format")
+ or form_data.get("label_position", "insideTopLeft") != "insideTopLeft"
+ ):
+ return ChartError(
+ error=(
+ "This Treemap color/currency/label format requires the native "
+ "Explore renderer; use url or table preview."
+ ),
+ error_type="UnsupportedTreemapPreview",
+ )
+ if (
+ len(data) > _MAX_ROWS
+ or any(row[label] < 0 for row in data)
+ or not any(row[label] > 0 for row in data)
+ ):
+ return ChartError(
+ error=(
+ "Treemap geometry requires at most 1000 nonnegative rows and "
+ "a positive total; use table or url preview."
+ ),
+ error_type="UnsupportedTreemapPreview",
+ )
+ hierarchy = form_data["groupby"]
+ if len(hierarchy) > 20 or not math.isfinite(sum(float(row[label]) for row
in data)):
+ return ChartError(
+ error="Treemap preview requires at most 20 levels and a finite
total.",
+ error_type="UnsupportedTreemapPreview",
+ )
+ nodes: list[dict[str, Any]] = []
+
+ def layout(
+ rows: list[dict[str, Any]],
+ depth: int,
+ path: list[str],
+ x: float,
+ y: float,
+ width: float,
+ height: float,
+ ) -> None:
+ """Recursively partition the parent rectangle in hierarchy order."""
+ groups: dict[tuple[str, str], list[dict[str, Any]]] = {}
+ for row in rows:
+ value = row[hierarchy[depth]]
+ groups.setdefault((type(value).__name__, str(value)),
[]).append(row)
Review Comment:
Compared with the current frontend: utils/treeBuilder.ts groups by raw Map
keys, so integer 1 and string "1" remain separate; Treemap/transformProps.ts
then formats the name and calls colorFn(name, sliceId). Keeping distinct
geometry with the same display name/color therefore matches the frontend rather
than inventing a different categorical encoding. Added
test_treemap_mixed_type_categories_remain_separate in 6a47480 to lock in the
two distinct metric groups. The test passes alongside renderer geometry checks.
A frontend-wide type-disambiguation UX would be a separate behavior change, not
a preview-only fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
