aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r3927155012
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -700,6 +701,126 @@ def _bar_chart_spec(
},
}
+ def _bullet_chart_spec(
+ self, fields: List[str], field_types: Dict[str, str] | None = None
+ ) -> Dict[str, Any]:
+ """Create a horizontal Bullet spec from saved native form_data."""
+ from superset.mcp_service.chart.preview_utils import (
+ _bullet_numeric_tokens,
+ _form_column_label,
+ _form_metric_label,
+ )
+ from superset.utils import json as utils_json
+
+ field_types = field_types or {}
+ form_data = self._get_form_data() or {}
+
+ def result_field(label: str | None) -> str | None:
+ if label in fields:
+ return label
+ if label:
+ matches = [
+ field for field in fields if field.casefold() ==
label.casefold()
+ ]
+ if len(matches) == 1:
+ return matches[0]
+ return None
+
+ metric_field =
result_field(_form_metric_label(form_data.get("metric")))
+ if metric_field is None:
+ metric_field = next(
+ (
+ field
+ for field in reversed(fields)
+ if field_types.get(field) == "quantitative"
+ ),
+ fields[-1] if fields else "metric",
+ )
+ dimensions = [
+ field
+ for column in form_data.get("groupby") or []
+ if (field := result_field(_form_column_label(column)))
+ ]
+ if not dimensions:
+ dimensions = [field for field in fields if field != metric_field]
+
+ category_field = "__mcp_bullet_category"
+ calculate = (
+ " + ', ' + ".join(
+ f"toString(datum[{utils_json.dumps(field)}])" for field in
dimensions
+ )
+ if dimensions
+ else "'Measure'"
+ )
+ y_encoding = {
+ "field": category_field,
+ "type": "nominal",
+ "title": ", ".join(dimensions) if dimensions else None,
+ "sort": None,
+ }
+ layers: list[dict[str, Any]] = []
+ for index, threshold in enumerate(
+ sorted(_bullet_numeric_tokens(form_data.get("ranges")),
reverse=True)
+ ):
+ layers.append(
+ {
+ "mark": {
+ "type": "rect",
+ "opacity": max(0.08, 0.28 - index * 0.04),
+ },
+ "encoding": {
+ "x": {"datum": 0, "type": "quantitative"},
+ "x2": {"datum": threshold},
+ },
+ }
+ )
Review Comment:
Fixed in 703bb6cd. The divergent saved-chart `_bullet_chart_spec` was
removed, so every saved/unsaved/cache Bullet Vega path now uses
`_generate_bullet_vega_lite_preview`; each range rect (and optional range
label) carries the same collision-safe category `y` encoding as the bar.
Covered by grouped saved-preview and public FastMCP Vega assertions; the
affected focused suite passed (1,451 tests).
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -323,6 +325,102 @@ def _generate_safe_ascii_bar_chart(data: List[Dict[str,
Any]]) -> str:
return "\n".join(lines)
+def _form_metric_label(metric: Any) -> str | None:
+ """Return the result-column label for a native QueryFormMetric."""
+ if isinstance(metric, str):
+ return metric
+ if not isinstance(metric, dict):
+ return None
+ if label := metric.get("label"):
+ return label if isinstance(label, str) else None
+ column = metric.get("column")
+ column_name = column.get("column_name") if isinstance(column, dict) else
column
+ aggregate = metric.get("aggregate")
+ if isinstance(column_name, str) and isinstance(aggregate, str):
+ return f"{aggregate}({column_name})"
+ return None
+
+
+def _form_column_label(column: Any) -> str | None:
+ """Return the result-column label for a native QueryFormColumn."""
+ if isinstance(column, str):
+ return column
+ if not isinstance(column, dict):
+ return None
+ for key in ("label", "column_name"):
+ if isinstance(value := column.get(key), str) and value:
+ return value
+ return None
+
+
+def _canonical_result_field(label: str | None, row: Dict[str, Any]) -> str |
None:
+ """Resolve a query result field by exact match, then unambiguous
casefold."""
+ if label is None:
+ return None
+ if label in row:
+ return label
+ matches = [field for field in row if field.casefold() == label.casefold()]
+ return matches[0] if len(matches) == 1 else None
+
+
+def _bullet_result_roles(
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> tuple[str | None, list[str]]:
+ """Resolve Bullet metric and full category hierarchy in query output."""
+ if not data:
+ return None, []
+ first_row = data[0]
+ metric_field = _canonical_result_field(
+ _form_metric_label(form_data.get("metric")), first_row
+ )
+ if metric_field is None:
+ metric_field = next(
+ (
+ field
+ for field, value in first_row.items()
+ if isinstance(value, (int, float)) and not _is_nan(value)
+ ),
+ None,
+ )
Review Comment:
Already fixed at the reviewed head and retained in 703bb6cd.
`resolve_bullet_render_model` calls `_require_result_field` for the declared
metric alias and fails missing/ambiguous aliases; it never selects a first
numeric field. `test_bullet_result_validation_rejects_malformed_rows` covers a
numeric unrelated field (`other: 123`) and exact/casefold ambiguity, and the
full MCP suite passed (4,386 tests).
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -469,11 +567,126 @@ def _is_nan(value: Any) -> bool:
return False
+def _bullet_numeric_tokens(value: Any) -> list[float]:
+ """Parse native comma-separated Bullet threshold controls."""
+ if isinstance(value, str):
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif isinstance(value, list):
+ tokens = value
+ else:
+ return []
+ result: list[float] = []
+ for token in tokens:
+ try:
+ number = float(token)
+ except (TypeError, ValueError):
+ continue
+ if not _is_nan(number) and math.isfinite(number):
+ result.append(number)
+ return result
+
+
+def _generate_bullet_vega_lite_preview(
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> VegaLitePreview | None:
+ """Build a horizontal layered preview faithful to Bullet result roles."""
+ metric_field, dimensions = _bullet_result_roles(data, form_data)
+ if metric_field is None:
+ return None
+
+ category_field = "__mcp_bullet_category"
+ values = []
+ for index, row in enumerate(data):
+ copied = dict(row)
+ copied[category_field] = (
+ ", ".join(str(row.get(field, "")) for field in dimensions)
+ if dimensions
+ else str(index + 1)
+ )
+ values.append(copied)
+
+ y_encoding = {
+ "field": category_field,
+ "type": "nominal",
+ "title": ", ".join(dimensions) if dimensions else None,
+ "sort": None,
+ }
+ tooltip = [
+ *({"field": field, "type": "nominal"} for field in dimensions),
+ {"field": metric_field, "type": "quantitative"},
+ ]
+ layers: list[dict[str, Any]] = []
+ for index, threshold in enumerate(
+ sorted(_bullet_numeric_tokens(form_data.get("ranges")), reverse=True)
+ ):
+ layers.append(
+ {
+ "mark": {
+ "type": "rect",
+ "opacity": max(0.08, 0.28 - index * 0.04),
+ },
+ "encoding": {
+ "x": {"datum": 0, "type": "quantitative"},
+ "x2": {"datum": threshold},
Review Comment:
Fixed in 703bb6cd. The shared strict Bullet renderer assigns `"y":
y_encoding` to every range rect and range-label layer, and the old duplicate
saved renderer was removed. Direct saved-preview and public FastMCP grouped
tests assert every rect uses the bar category encoding.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2014,6 +2016,385 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class BulletChartConfig(BaseChartConfig):
+ """Typed contract for the ECharts Bullet visualization (viz_type
``bullet``).
+
+ Semantic field names are exposed to MCP clients while validation aliases
and
+ the native adapter accept saved Explore ``form_data`` without weakening the
+ unknown-field checks that catch misspelled controls.
+ """
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["bullet"] = "bullet"
+ metric: ColumnRef = Field(
+ ...,
+ description=(
+ "Numeric measure shown by each bullet bar. Use aggregate for a
SIMPLE "
+ "metric, saved_metric=True for a dataset metric, or sql_expression
"
+ "with a unique label."
+ ),
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ validation_alias=AliasChoices("dimensions", "groupby"),
+ description=(
+ "Optional category hierarchy; the frontend renders one bullet row
per "
+ "unique combination (native form_data: groupby). Omit to preserve
a "
+ "saved hierarchy on update; pass [] to clear it."
+ ),
+ max_length=20,
+ )
+ filters: List[FilterConfig] | None = Field(
+ None,
+ description=(
+ "Structured WHERE filters. Native SIMPLE adhoc_filters are
accepted; "
+ "free-form SQL filters are rejected."
+ ),
+ max_length=100,
+ )
+ time_range: str | None = Field(
+ None,
+ min_length=1,
+ max_length=1000,
+ description=(
+ "Optional Superset time range such as 'Last 30 days' or "
+ "'2025-01-01 : 2025-12-31'. Set temporal_column to choose its
column."
+ ),
+ )
+ row_limit: int = Field(
+ 10000,
+ ge=1,
+ le=50000,
+ description="Maximum grouped bullet rows returned by the query",
+ )
+ order_by: List[SortByConfig] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("order_by", "orderby", "order_by_cols"),
+ max_length=20,
+ description=(
+ "Stable row ordering by a dimension name or by the metric's output
"
+ "label/name. Native orderby pairs and order_by_cols JSON pairs are
"
+ "accepted for saved-form-data round trips."
+ ),
+ )
+
+ # Presentation fields map one-for-one onto Bullet/transformProps.ts
controls.
+ ranges: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Qualitative range thresholds shaded behind the measure",
+ )
+ range_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("range_labels", "rangeLabels"),
+ max_length=100,
+ )
+ markers: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Target values drawn as point markers",
+ )
+ marker_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_labels", "markerLabels"),
+ max_length=100,
+ )
+ marker_lines: List[float] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_lines", "markerLines"),
+ max_length=100,
+ description="Reference values drawn as vertical lines",
+ )
+ marker_line_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_line_labels",
"markerLineLabels"),
+ max_length=100,
+ )
+ y_axis_format: str = Field(
+ "SMART_NUMBER",
+ validation_alias=AliasChoices("y_axis_format", "yAxisFormat"),
+ max_length=100,
+ )
+ show_labels: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_labels", "showLabels"),
+ )
+ show_legend: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_legend", "showLegend"),
+ )
+
+ @staticmethod
+ def _adapt_native_metric(value: Any) -> Any:
+ """Translate QueryFormMetric shapes into the shared ColumnRef
contract."""
+ if isinstance(value, str):
+ legacy = re.fullmatch(
+ r"(sum|avg|min|max|count|count_distinct)__(.+)",
+ value,
+ flags=re.IGNORECASE,
+ )
+ if legacy:
+ return {
+ "name": legacy.group(2),
+ "aggregate": legacy.group(1).upper(),
+ }
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict) or "expressionType" not in value:
+ return value
+ expression_type = value.get("expressionType")
+ if expression_type == "SQL":
+ return {
+ "sql_expression": value.get("sqlExpression"),
+ "label": value.get("label"),
+ }
+ if expression_type != "SIMPLE":
+ raise ValueError("metric.expressionType must be 'SIMPLE' or 'SQL'")
+ column = value.get("column")
+ if isinstance(column, dict):
+ name = column.get("column_name")
+ else:
+ name = column
+ return {
+ "name": name,
+ "aggregate": value.get("aggregate"),
+ "label": value.get("label"),
+ }
+
+ @staticmethod
+ def _adapt_native_order_by(value: Any) -> Any: # noqa: C901
+ if value is None:
+ return []
+ if not isinstance(value, list):
+ raise ValueError("order_by must be an array")
+ result: list[Any] = []
+ for index, entry in enumerate(value):
+ if isinstance(entry, str):
+ if len(entry) > 2000:
+ raise ValueError(f"order_by[{index}] is too long")
+ try:
+ entry = json.loads(entry)
+ except json.JSONDecodeError:
+ # A bare output/column name is the ergonomic typed form.
+ result.append({"column": entry, "ascending": False})
+ continue
+ if isinstance(entry, dict):
+ result.append(entry)
+ continue
+ if not isinstance(entry, (list, tuple)) or len(entry) != 2:
+ raise ValueError(
+ f"order_by[{index}] must be [column, ascending_boolean]"
+ )
+ target, ascending = entry
+ if isinstance(target, dict):
+ target = target.get("label") or target.get("metric_name")
+ if not isinstance(target, str) or not target:
+ raise ValueError(f"order_by[{index}] needs a column or metric
label")
+ if not isinstance(ascending, bool):
+ raise ValueError(f"order_by[{index}] ascending value must be
boolean")
+ result.append({"column": target, "ascending": ascending})
+ return result
+
+ @staticmethod
+ def _adapt_native_filters(data: dict[str, Any]) -> None: # noqa: C901
+ if "adhoc_filters" not in data:
+ return
+ if "filters" in data:
+ raise ValueError("Use either filters or native adhoc_filters, not
both")
+ raw_filters = data.pop("adhoc_filters")
+ if not isinstance(raw_filters, list):
+ raise ValueError("adhoc_filters must be an array")
+ filters: list[dict[str, Any]] = []
+ for index, raw_filter in enumerate(raw_filters):
+ if not isinstance(raw_filter, dict):
+ raise ValueError(f"adhoc_filters[{index}] must be an object")
+ if raw_filter.get("expressionType") != "SIMPLE":
+ raise ValueError(
+ f"adhoc_filters[{index}] must use expressionType='SIMPLE'"
+ )
+ if raw_filter.get("clause") not in (None, "WHERE"):
+ raise ValueError(f"adhoc_filters[{index}] must use
clause='WHERE'")
+ subject = raw_filter.get("subject")
+ operator = raw_filter.get("operator")
+ comparator = raw_filter.get("comparator")
+ if operator == "TEMPORAL_RANGE":
+ if not isinstance(subject, str) or not subject:
+ raise ValueError(
+ f"adhoc_filters[{index}] temporal filter needs subject"
+ )
+ data.setdefault("temporal_column", subject)
+ if isinstance(comparator, str) and comparator.casefold() !=
"no filter":
+ data.setdefault("time_range", comparator)
+ continue
+ if not isinstance(operator, str):
+ raise ValueError(f"adhoc_filters[{index}] needs an operator")
+ operator_map = {"==": "=", "IS_NOT_NULL": "IS NOT NULL"}
+ operator = operator_map.get(operator, operator)
+ filters.append({"column": subject, "op": operator, "value":
comparator})
+ data["filters"] = filters
+
+ @model_validator(mode="before")
+ @classmethod
+ def adapt_native_form_data(cls, raw: Any) -> Any: # noqa: C901
+ """Accept recognized saved Bullet form_data and reject ambiguous
state."""
+ if not isinstance(raw, dict):
+ return raw
+ data = dict(raw)
+ if data.get("viz_type") == "bullet":
+ data.setdefault("chart_type", "bullet")
+ data.pop("viz_type", None)
+ for key in (
+ "annotation_layers",
+ "dashboards",
+ "datasource",
+ "datasource_id",
+ "datasource_type",
+ "extra_form_data",
+ "slice_id",
+ ):
+ data.pop(key, None)
+
+ marker_key = "_mcp_dashboard_time_filter_subject"
+ if marker := data.pop(marker_key, None):
+ if not isinstance(marker, str):
+ raise ValueError(f"{marker_key} must be a physical column
name")
+ data.setdefault("temporal_column", marker)
+
+ if "metric" in data:
+ data["metric"] = cls._adapt_native_metric(data["metric"])
+ for key in ("groupby", "dimensions"):
+ if key in data:
+ if not isinstance(data[key], list):
+ raise ValueError(f"{key} must be an array")
+ data[key] = [
+ {"name": item} if isinstance(item, str) else item
+ for item in data[key]
Review Comment:
Addressed and verified at 703bb6cd. The pre-validator canonicalizes both
aliases and rejects unequal lists before alias precedence can apply; equivalent
lists are order-independent. A new regression also documents why schema-only
identity remains exact-spelling (`Region` and `region` may be distinct quoted
columns), while dataset-aware resolution remains exact-first and rejects
ambiguous casefold matches.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1065,6 +1066,155 @@ def map_histogram_config(config:
"HistogramChartConfig") -> Dict[str, Any]:
return form_data
+def _bullet_token_list(values: Sequence[str | float]) -> str:
+ """Serialize typed Bullet controls to the frontend's comma-separated
form."""
+ return ",".join(
+ format(value, "g") if isinstance(value, float) else value for value in
values
+ )
+
+
+def map_bullet_config(config: BulletChartConfig) -> Dict[str, Any]: # noqa:
C901
+ """Map typed Bullet config to ``Bullet/buildQuery`` and transformProps.
+
+ The frontend buildQuery replaces the generic query fields with exactly one
+ metric and the groupby hierarchy. Presentation controls stay in native
+ snake_case form_data; the chart plugin camelizes them for transformProps.
+ """
+ metric = create_metric_object(config.metric)
+ form_data: Dict[str, Any] = {
+ "viz_type": "bullet",
+ "metric": metric,
+ }
+
+ # Optional semantic/query fields are emitted only when explicitly supplied.
+ # This lets update_chart and update_chart_preview preserve native saved
state,
+ # while an explicit empty value still clears it through the generic merge
path.
+ if "dimensions" in config.model_fields_set:
+ form_data["groupby"] = [dimension.name for dimension in
config.dimensions or []]
+ if "row_limit" in config.model_fields_set:
+ form_data["row_limit"] = config.row_limit
+ if "time_range" in config.model_fields_set:
+ form_data["time_range"] = config.time_range
+
+ if config.order_by:
+ metric_output = metric if isinstance(metric, str) else
metric.get("label")
+ metric_targets = {
+ candidate.casefold()
+ for candidate in (
+ config.metric.name,
+ config.metric.label,
+ metric_output,
+ )
+ if candidate
+ }
+ dimensions = config.dimensions or []
+ dimension_targets = {
+ candidate.casefold(): dimension.name
+ for dimension in dimensions
+ for candidate in (dimension.name, dimension.label)
+ if candidate and dimension.name
+ }
+ form_data["orderby"] = [
+ [
+ (
+ metric
+ if order.column.casefold() in metric_targets
+ else dimension_targets[order.column.casefold()]
+ ),
+ order.ascending,
+ ]
+ for order in config.order_by
+ ]
+ elif "order_by" in config.model_fields_set:
+ form_data["orderby"] = []
+
+ presentation_fields: dict[str, tuple[str, Any]] = {
+ "ranges": ("ranges", _bullet_token_list(config.ranges)),
+ "range_labels": (
+ "range_labels",
+ _bullet_token_list(config.range_labels),
+ ),
+ "markers": ("markers", _bullet_token_list(config.markers)),
+ "marker_labels": (
+ "marker_labels",
+ _bullet_token_list(config.marker_labels),
+ ),
+ "marker_lines": (
+ "marker_lines",
+ _bullet_token_list(config.marker_lines),
+ ),
+ "marker_line_labels": (
+ "marker_line_labels",
+ _bullet_token_list(config.marker_line_labels),
+ ),
+ "y_axis_format": ("y_axis_format", config.y_axis_format),
+ "show_labels": ("show_labels", config.show_labels),
+ "show_legend": ("show_legend", config.show_legend),
+ }
+ for field_name, (form_key, value) in presentation_fields.items():
+ if field_name in config.model_fields_set:
+ form_data[form_key] = value
+
+ _add_adhoc_filters(form_data, config.filters)
+ if config.filters == [] and "filters" in config.model_fields_set:
+ form_data["adhoc_filters"] = []
+ if config.time_range and config.temporal_column:
+ _ensure_temporal_adhoc_filter(form_data, config.temporal_column)
+ for filter_ in form_data.get("adhoc_filters", []):
+ if (
+ isinstance(filter_, dict)
+ and filter_.get("operator") ==
FilterOperator.TEMPORAL_RANGE.value
+ and filter_.get("subject") == config.temporal_column
+ and filter_.get("comparator") == NO_TIME_RANGE
+ ):
+ filter_["comparator"] = config.time_range
+ return form_data
+
+
+def merge_bullet_form_data(
+ existing_form_data: Mapping[str, Any], new_form_data: Dict[str, Any]
+) -> None:
+ """Preserve omitted native Bullet controls across update tool paths.
+
+ Query roles and every UI control have an explicit typed representation.
+ Mappers emit optional fields only when the caller supplied them, so copying
+ the bounded native keys below preserves omitted state while explicit empty,
+ false, null, and zero-like values remain authoritative.
+ """
+ if (
+ existing_form_data.get("viz_type") != "bullet"
+ or new_form_data.get("viz_type") != "bullet"
+ ):
+ return
+ preserved_keys = {
+ "groupby",
+ "adhoc_filters",
+ "time_range",
+ "row_limit",
+ "orderby",
+ "ranges",
+ "range_labels",
+ "markers",
+ "marker_labels",
+ "marker_lines",
+ "marker_line_labels",
+ "y_axis_format",
+ "show_labels",
+ "show_legend",
+ MCP_DASHBOARD_TIME_FILTER_SUBJECT,
+ }
+ for key in preserved_keys:
+ if (
+ key == MCP_DASHBOARD_TIME_FILTER_SUBJECT
+ and "adhoc_filters" in new_form_data
+ ):
+ # The marker describes a mapper-generated temporal filter. Do not
+ # retain stale provenance when an explicit filter update removed
it.
+ continue
+ if key in existing_form_data and key not in new_form_data:
+ new_form_data[key] = existing_form_data[key]
Review Comment:
Already fixed in the production shared merge path and retained in 703bb6cd.
`merge_update_form_data` treats `time_range` as authoritative, copies the saved
provenance-owned binding, replaces its comparator in place, and retains the
saved subject when `temporal_column` is omitted.
`test_bullet_time_range_only_updates_saved_subject_on_every_update_path` covers
immediate, preview-first, and cached paths for active and cleared ranges. The
unused compatibility wrapper was removed so no tests exercise dead logic.
##########
superset/mcp_service/chart/plugins/bullet.py:
##########
@@ -0,0 +1,272 @@
+# 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.
+
+"""ECharts Bullet chart type plugin."""
+
+from __future__ import annotations
+
+from collections.abc import Mapping
+from typing import Any, ClassVar
+
+from superset.mcp_service.chart.chart_utils import (
+ _summarize_filters,
+ map_bullet_config,
+)
+from superset.mcp_service.chart.plugin import BaseChartPlugin
+from superset.mcp_service.chart.schemas import BulletChartConfig, ColumnRef
+from superset.mcp_service.chart.validation.dataset_validator import (
+ DatasetValidator,
+ is_numeric_column,
+)
+from superset.mcp_service.common.error_schemas import (
+ ChartGenerationError,
+ DatasetContext,
+)
+
+
+def _canonical_reference(
+ name: str,
+ candidates: list[str],
+ role: str,
+) -> str:
+ """Resolve exact/casefold matches without silently choosing ambiguity."""
+ if name in candidates:
+ return name
+ matches = [
+ candidate for candidate in candidates if candidate.casefold() ==
name.casefold()
+ ]
+ if len(matches) > 1:
+ raise ValueError(
+ f"Ambiguous Bullet {role} {name!r}; exact-case matches are: "
+ f"{', '.join(matches)}"
+ )
+ return matches[0] if matches else name
+
+
+class BulletChartPlugin(BaseChartPlugin):
+ """Plugin matching ``plugin-chart-echarts/src/Bullet``."""
+
+ chart_type = "bullet"
+ display_name = "Bullet Chart"
+ native_viz_types: ClassVar[Mapping[str, str]] = {
+ "bullet": "Bullet Chart",
+ }
+
+ def pre_validate(self, config: dict[str, Any]) -> ChartGenerationError |
None:
+ if "metric" in config:
+ return None
+ return ChartGenerationError(
+ error_type="missing_bullet_fields",
+ message="Bullet chart missing required field: metric",
+ details=(
+ "A Bullet chart measures one numeric aggregate or saved/SQL
metric; "
+ "optional dimensions split it into one row per group."
+ ),
+ suggestions=[
+ "Add metric: {'name': 'revenue', 'aggregate': 'SUM'}",
+ "For a saved metric use {'name': 'revenue', 'saved_metric':
true}",
+ "Add dimensions: [{'name': 'region'}] for grouped bullet rows",
+ ],
+ error_code="MISSING_BULLET_FIELDS",
+ )
+
+ def extract_column_refs(self, config: Any) -> list[ColumnRef]:
+ if not isinstance(config, BulletChartConfig):
+ return []
+ refs = [config.metric, *(config.dimensions or [])]
+ refs.extend(ColumnRef(name=filter_.column) for filter_ in
config.filters or [])
+ # order_by is constrained to role outputs by the schema, so those names
+ # are already represented by metric/dimension refs and must not be
+ # reinterpreted as physical columns.
+ return refs
+
+ def to_form_data(
+ self, config: Any, dataset_id: int | str | None = None
+ ) -> dict[str, Any]:
+ if not isinstance(config, BulletChartConfig):
+ raise TypeError("BulletChartPlugin requires BulletChartConfig")
+ return map_bullet_config(config)
+
+ def post_map_validate(
+ self,
+ config: Any,
+ form_data: dict[str, Any],
+ dataset_id: int | str | None = None,
+ ) -> ChartGenerationError | None:
+ """Require an unambiguous numeric metric output for Number(...)."""
+ if not isinstance(config, BulletChartConfig) or dataset_id is None:
+ return None
+ dataset_context = DatasetValidator._get_dataset_context(dataset_id)
+ if dataset_context is None:
+ return None
+
+ columns = [column["name"] for column in
dataset_context.available_columns]
+ metrics = [metric["name"] for metric in
dataset_context.available_metrics]
+ requested: list[tuple[str, list[str], str]] = []
+ if config.metric.name and not config.metric.sql_expression:
+ requested.append(
+ (
+ config.metric.name,
+ metrics if config.metric.saved_metric else columns,
+ "saved metric" if config.metric.saved_metric else "metric
column",
+ )
+ )
+ requested.extend(
+ (dimension.name or "", columns, "dimension")
+ for dimension in config.dimensions or []
+ if dimension.name
+ )
+ requested.extend(
+ (filter_.column, columns, "filter column")
+ for filter_ in config.filters or []
+ )
+ if config.temporal_column:
+ requested.append((config.temporal_column, columns, "temporal
column"))
+
+ for name, candidates, role in requested:
+ if (
+ name not in candidates
+ and sum(
+ candidate.casefold() == name.casefold() for candidate in
candidates
+ )
+ > 1
+ ):
+ return ChartGenerationError(
+ error_type="ambiguous_bullet_reference",
+ message=f"Bullet {role} {name!r} is ambiguous by case",
+ details=(
+ "Multiple dataset fields differ only by case. The
query and "
+ "frontend require an exact canonical field name."
+ ),
+ suggestions=[
+ "Use get_dataset_info and copy the exact-case field
name"
+ ],
+ error_code="AMBIGUOUS_BULLET_REFERENCE",
+ )
+
+ metric = config.metric
+ if metric.saved_metric or metric.sql_expression:
+ # Saved/SQL metric result types are determined by their
expressions;
+ # Tier-2 compile validation remains authoritative.
+ return None
+ if (metric.aggregate or "SUM") in {"COUNT", "COUNT_DISTINCT"}:
+ return None
+ column = next(
+ (
+ column
+ for column in dataset_context.available_columns
+ if column["name"].casefold() == (metric.name or "").casefold()
+ ),
+ None,
+ )
+ if column is None or is_numeric_column(column):
+ return None
Review Comment:
Already fixed at the reviewed head and retained in 703bb6cd. Dataset
resolution is exact-name first; only a unique casefold candidate is accepted.
`test_bullet_exact_case_type_and_role_resolution_is_order_independent` proves
`Revenue` (numeric) and `revenue` (text) select their own types regardless of
metadata order, while `REVENUE` is rejected as ambiguous.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1065,6 +1066,155 @@ def map_histogram_config(config:
"HistogramChartConfig") -> Dict[str, Any]:
return form_data
+def _bullet_token_list(values: Sequence[str | float]) -> str:
+ """Serialize typed Bullet controls to the frontend's comma-separated
form."""
+ return ",".join(
+ format(value, "g") if isinstance(value, float) else value for value in
values
Review Comment:
Already fixed at the reviewed head and retained in 703bb6cd. Presentation
numbers use shortest round-trip-safe finite spelling rather than `format(...,
"g")`; `test_bullet_presentation_numbers_use_shortest_round_trip_safe_tokens`
covers `123456.789`, binary64 extremes, and exact semantic round trips.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1065,6 +1121,660 @@ def map_histogram_config(config:
"HistogramChartConfig") -> Dict[str, Any]:
return form_data
+def _bullet_token_list(values: Sequence[str | int | float]) -> str:
+ """Serialize typed Bullet controls to the frontend's comma-separated
form."""
+ tokens: list[str] = []
+ for value in values:
+ if isinstance(value, float):
+ token = repr(value)
+ # ``100`` parses back to the same binary float as ``100.0`` and
+ # preserves the frontend's established compact integer spelling.
+ if token.endswith(".0") and not (
+ value == 0.0 and math.copysign(1.0, value) < 0
+ ):
+ token = token[:-2]
+ tokens.append(token)
+ else:
+ tokens.append(str(value))
+ return ",".join(tokens)
+
+
+def map_bullet_config(config: BulletChartConfig) -> Dict[str, Any]: # noqa:
C901
+ """Map typed Bullet config to ``Bullet/buildQuery`` and transformProps.
+
+ The frontend buildQuery replaces the generic query fields with exactly one
+ metric and the groupby hierarchy. Presentation controls stay in native
+ snake_case form_data; the chart plugin camelizes them for transformProps.
+ """
+ metric = create_metric_object(config.metric)
+ form_data: Dict[str, Any] = {
+ "viz_type": "bullet",
+ "metric": metric,
+ }
+
+ # Optional semantic/query fields are emitted only when explicitly supplied.
+ # This lets update_chart and update_chart_preview preserve native saved
state,
+ # while an explicit empty value still clears it through the generic merge
path.
+ if "dimensions" in config.model_fields_set:
+ form_data["groupby"] = [dimension.name for dimension in
config.dimensions or []]
+ if "row_limit" in config.model_fields_set:
+ form_data["row_limit"] = config.row_limit
+ if "time_range" in config.model_fields_set:
+ form_data["time_range"] = config.time_range
+
+ if config.order_by:
+ dimensions = config.dimensions or []
+ orderby: list[list[Any]] = []
+ for order in config.order_by:
+ role, index = resolve_bullet_order_target(
+ order.column, dimensions, config.metric
+ )
+ if role == "metric":
+ order_target: Any = metric
+ else:
+ if index is None: # Defensive: resolver pairs dimensions with
indexes.
+ raise ValueError("Bullet dimension order target has no
index")
+ order_target = dimensions[index].name
+ orderby.append([order_target, order.ascending])
+ form_data["orderby"] = orderby
+ elif "order_by" in config.model_fields_set:
+ form_data["orderby"] = []
+
+ presentation_fields: dict[str, tuple[str, Any]] = {
+ "ranges": ("ranges", _bullet_token_list(config.ranges)),
+ "range_labels": (
+ "range_labels",
+ _bullet_token_list(config.range_labels),
+ ),
+ "markers": ("markers", _bullet_token_list(config.markers)),
+ "marker_labels": (
+ "marker_labels",
+ _bullet_token_list(config.marker_labels),
+ ),
+ "marker_lines": (
+ "marker_lines",
+ _bullet_token_list(config.marker_lines),
+ ),
+ "marker_line_labels": (
+ "marker_line_labels",
+ _bullet_token_list(config.marker_line_labels),
+ ),
+ "y_axis_format": ("y_axis_format", config.y_axis_format),
+ "show_labels": ("show_labels", config.show_labels),
+ "show_legend": ("show_legend", config.show_legend),
+ }
+ for field_name, (form_key, value) in presentation_fields.items():
+ if field_name in config.model_fields_set:
+ form_data[form_key] = value
+
+ _add_adhoc_filters(form_data, config.filters)
+ if config.filters == [] and "filters" in config.model_fields_set:
+ form_data["adhoc_filters"] = []
+ if config.time_range and config.temporal_column:
+ _ensure_temporal_adhoc_filter(form_data, config.temporal_column)
+ for filter_ in form_data.get("adhoc_filters", []):
+ if (
+ isinstance(filter_, dict)
+ and filter_.get("operator") ==
FilterOperator.TEMPORAL_RANGE.value
+ and filter_.get("subject") == config.temporal_column
+ and filter_.get("comparator") == NO_TIME_RANGE
+ ):
+ filter_["comparator"] = config.time_range
+ return form_data
+
+
+def merge_bullet_form_data(
+ existing_form_data: Mapping[str, Any], new_form_data: Dict[str, Any]
+) -> None:
+ """Preserve omitted native Bullet controls across update tool paths.
+
+ Query roles and every UI control have an explicit typed representation.
+ Mappers emit optional fields only when the caller supplied them, so copying
+ the bounded native keys below preserves omitted state while explicit empty,
+ false, null, and zero-like values remain authoritative.
+ """
+ if (
+ existing_form_data.get("viz_type") != "bullet"
+ or new_form_data.get("viz_type") != "bullet"
+ ):
+ return
+ preserved_keys = {
+ "groupby",
+ "adhoc_filters",
+ "time_range",
+ "row_limit",
+ "orderby",
+ "ranges",
+ "range_labels",
+ "markers",
+ "marker_labels",
+ "marker_lines",
+ "marker_line_labels",
+ "y_axis_format",
+ "show_labels",
+ "show_legend",
+ MCP_DASHBOARD_TIME_FILTER_SUBJECT,
+ }
+
+ # Threshold and label arrays are one frontend control pair. If callers
+ # replace the values without replacing their labels, clear the stale labels
+ # instead of accidentally reassigning them by position.
+ dependent_controls = {
+ "ranges": "range_labels",
+ "markers": "marker_labels",
+ "marker_lines": "marker_line_labels",
+ }
+ for values_key, labels_key in dependent_controls.items():
+ if values_key in new_form_data and labels_key not in new_form_data:
+ new_form_data[labels_key] = ""
+
+ for key in preserved_keys:
+ if (
+ key == MCP_DASHBOARD_TIME_FILTER_SUBJECT
+ and "adhoc_filters" in new_form_data
+ ):
+ # The marker describes a mapper-generated temporal filter. Do not
+ # retain stale provenance when an explicit filter update removed
it.
+ continue
+ if key in existing_form_data and key not in new_form_data:
+ new_form_data[key] = existing_form_data[key]
+
+
+def _filter_identity(filter_: Any) -> tuple[Any, ...] | None:
+ """Return the native identity used when one filter replaces another."""
+ if not isinstance(filter_, Mapping):
+ return None
+ return (
+ filter_.get("clause"),
+ filter_.get("expressionType"),
+ filter_.get("subject"),
+ filter_.get("operator"),
+ )
+
+
+def _temporal_binding_filter(filters: list[Any], subject: Any) -> dict[str,
Any] | None:
+ """Find the unique filter owned by a recorded MCP temporal marker."""
+ if subject is None:
+ return None
+ if not isinstance(subject, str) or not subject:
+ raise ValueError(
+ "MCP temporal binding provenance subject must be a non-empty
string"
+ )
+ matches = [
+ filter_
+ for filter_ in filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == subject
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ ]
+ if len(matches) != 1:
+ raise ValueError(
+ "MCP temporal binding provenance must match exactly one "
+ f"TEMPORAL_RANGE filter for subject {subject!r}; found
{len(matches)}"
+ )
+ return matches[0]
+
+
+def _append_or_replace_filter(filters: list[Any], filter_: Any) -> None:
+ """Append a filter, replacing the same native role when identifiable."""
+ identity = _filter_identity(filter_)
+ if identity is None:
+ if filter_ not in filters:
+ filters.append(filter_)
+ return
+ filters[:] = [item for item in filters if _filter_identity(item) !=
identity]
+ filters.append(filter_)
+
+
+_NATIVE_TEMPORAL_ROLE_FIELDS: dict[str, frozenset[str]] = {
+ # Typed ``x`` is persisted as native x_axis/granularity_sqla for XY and
+ # Mixed Timeseries. Waterfall exposes the typed field as ``x_axis``.
+ "x_axis": frozenset({"x", "x_axis"}),
+ "granularity_sqla": frozenset({"x", "x_axis", "temporal_column"}),
+ # Chart plugins may designate a chart-specific query role as the implicit
+ # dashboard-time subject.
+ "start_time": frozenset({"start_time"}),
+}
+
+
+def _native_temporal_subject_changed(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ explicit_fields: set[str],
+) -> bool:
+ """Return whether an authoritative native temporal role was replaced.
+
+ Mapping a partial update can propose a dataset fallback binding even when
+ the caller only changed filters. That proposal is not authoritative. A
+ changed x/granularity/chart-specific role is authoritative only when its
+ corresponding typed field was actually supplied.
+ """
+ for native_key, typed_fields in _NATIVE_TEMPORAL_ROLE_FIELDS.items():
+ if explicit_fields.isdisjoint(typed_fields):
+ continue
+ existing_value = existing_form_data.get(native_key)
+ incoming_value = new_form_data.get(native_key)
+ if existing_value != incoming_value:
+ return True
+ return False
+
+
+def _native_temporal_binding(
+ form_data: Mapping[str, Any], filters: list[Any]
+) -> tuple[str | None, dict[str, Any] | None]:
+ """Resolve one binding for a trusted native temporal role, if present."""
+ for native_key in _NATIVE_TEMPORAL_ROLE_FIELDS:
+ subject = form_data.get(native_key)
+ if not isinstance(subject, str) or not subject:
+ continue
+ matches = [
+ filter_
+ for filter_ in filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == subject
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ ]
+ if len(matches) > 1:
+ raise ValueError(
+ "An authoritative native temporal subject must match at most
one "
+ f"TEMPORAL_RANGE filter for subject {subject!r}; found "
+ f"{len(matches)}"
+ )
+ if matches:
+ return subject, matches[0]
+ return None, None
+
+
+def merge_update_form_data( # noqa: C901
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Apply the shared omission/provenance contract for chart updates.
+
+ Mapper-generated neutral temporal bindings are infrastructure, not evidence
+ that the caller supplied ``filters`` or changed a saved time-range binding.
+ This helper is used by immediate saves, preview-first saved updates, and
+ cached-preview updates so omission, clear, replacement, and temporal
+ overrides have identical behavior.
+ """
+ existing_filters = list(existing_form_data.get("adhoc_filters") or [])
+ incoming_filters = list(new_form_data.get("adhoc_filters") or [])
+ existing_subject =
existing_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ incoming_subject = new_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ existing_binding = _temporal_binding_filter(existing_filters,
existing_subject)
+ incoming_binding = _temporal_binding_filter(incoming_filters,
incoming_subject)
+
+ explicit_fields = set(getattr(config, "model_fields_set", set()))
+ filters_explicit = "filters" in explicit_fields
+ range_explicit = "time_range" in explicit_fields
+ subject_explicit = "temporal_column" in explicit_fields
+ native_subject_changed = _native_temporal_subject_changed(
+ existing_form_data, new_form_data, explicit_fields
+ )
+ subject_authoritative = subject_explicit or native_subject_changed
+ if incoming_binding is None:
+ native_subject, native_binding = _native_temporal_binding(
+ new_form_data, incoming_filters
+ )
+ if native_binding is not None:
+ incoming_subject = native_subject
+ incoming_binding = native_binding
+ incoming_user_filters = [
+ filter_ for filter_ in incoming_filters if filter_ is not
incoming_binding
+ ]
+ temporal_explicit = range_explicit or subject_authoritative
+
+ chosen_binding: dict[str, Any] | None = None
+ chosen_subject: Any = None
+ if not filters_explicit:
+ # Omission is byte-faithful: keep the native sequence in its exact
order,
+ # including SQL/HAVING objects and a provenance-owned binding at any
index.
+ merged_filters = list(existing_filters)
+ chosen_binding = existing_binding
+ chosen_subject = existing_subject
+ if temporal_explicit:
+ if subject_authoritative:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+ elif existing_binding is not None:
+ # A range-only update belongs to the saved subject, even when
+ # mapping the partial config proposed the dataset main_dttm.
+ chosen_binding = dict(existing_binding)
+ chosen_subject = existing_subject
+ else:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+
+ if chosen_binding is not None:
+ chosen_binding = dict(chosen_binding)
+ if range_explicit:
+ chosen_binding["comparator"] = (
+ getattr(config, "time_range", None) or NO_TIME_RANGE
+ )
+ elif existing_binding is not None:
+ # Subject-only replacement preserves the saved active or
+ # neutral range instead of resetting it to No filter.
+ chosen_binding["comparator"] = existing_binding.get(
+ "comparator", NO_TIME_RANGE
+ )
+ if existing_binding is not None:
+ binding_index = next(
+ index
+ for index, filter_ in enumerate(merged_filters)
+ if filter_ is existing_binding
+ )
+ if chosen_binding is None:
+ merged_filters.pop(binding_index)
+ else:
+ # A temporal override changes infrastructure in place
instead
+ # of moving it past surrounding native filters.
+ merged_filters[binding_index] = chosen_binding
+ elif chosen_binding is not None:
+ merged_filters.append(chosen_binding)
+ else:
+ # An explicit filter array replaces the saved native sequence. The
mapper
+ # deliberately emits [] for an explicit clear; otherwise retain its
+ # generated temporal binding after the replacement filters.
+ merged_filters = list(incoming_user_filters)
+ if incoming_user_filters or temporal_explicit:
+ if subject_authoritative:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+ elif range_explicit and existing_binding is not None:
+ chosen_binding = dict(existing_binding)
+ chosen_subject = existing_subject
+ else:
+ # A filter-only replacement keeps the saved provenance binding.
+ # The mapper's incoming binding may merely be a dataset
fallback
+ # and must not reset the saved subject or active range.
+ chosen_binding = existing_binding
+ chosen_subject = existing_subject
+ if chosen_binding is not None:
+ chosen_binding = dict(chosen_binding)
+ if range_explicit:
+ chosen_binding["comparator"] = (
+ getattr(config, "time_range", None) or NO_TIME_RANGE
+ )
+ elif subject_authoritative and existing_binding is not None:
+ chosen_binding["comparator"] = existing_binding.get(
+ "comparator", NO_TIME_RANGE
+ )
+ if chosen_binding is not None:
+ _append_or_replace_filter(merged_filters, chosen_binding)
+
+ # Materialize exactly when saved state had the key or the caller made the
+ # controls authoritative. An omitted update must not turn a missing native
+ # filter key into [] merely because its mapper proposed a neutral binding.
+ if filters_explicit or "adhoc_filters" in existing_form_data or
temporal_explicit:
+ new_form_data["adhoc_filters"] = merged_filters
+ else:
+ new_form_data.pop("adhoc_filters", None)
+ if chosen_binding is not None and isinstance(chosen_subject, str):
+ new_form_data[MCP_DASHBOARD_TIME_FILTER_SUBJECT] = chosen_subject
+ else:
+ new_form_data.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+ merge_bullet_form_data(existing_form_data, new_form_data)
+
+
+def _currency_form_value(value: CurrencyFormat | None) -> dict[str, str] |
None:
+ """Return the native value for an explicitly supplied currency control."""
+ return value.to_form_data() if value is not None else None
+
+
+def _column_names(value: Sequence[ColumnRef] | None) -> list[str | None] |
None:
+ """Return a native column-name list while retaining an explicit null."""
+ return [column.name for column in value] if value is not None else None
+
+
+def _table_sort_value(value: Sequence[str | SortByConfig] | None) -> list[str]
| None:
+ """Return the native Table sort control for an explicit typed value."""
+ if value is None:
+ return None
+ return [
+ json.dumps(
+ [entry.column, entry.ascending]
+ if isinstance(entry, SortByConfig)
+ else [entry, False]
+ )
+ for entry in value
+ ]
+
+
+def _table_column_config_value(value: Any) -> dict[str, Any] | None:
+ """Return Table column config without losing an explicit null or empty
map."""
+ if value is None:
+ return None
+ return {
+ label: column.model_dump(by_alias=True, exclude_unset=True)
+ for label, column in value.items()
+ }
+
+
+# Mappers intentionally omit optional controls so fresh charts use the frontend
+# defaults. During a same-viz update, however, an explicitly supplied false,
+# null, or empty value must block preservation of the saved native key. Keep
the
+# typed-to-native relationship declarative so every update path shares it.
+_FormValueConverter = Callable[[Any], Any]
+_FormControlMap = dict[str, tuple[str, _FormValueConverter]]
+
+_COMMON_EXPLICIT_FORM_CONTROLS: _FormControlMap = {
+ "color_scheme": ("color_scheme", lambda value: value),
+ "currency_format": ("currency_format", _currency_form_value),
+ "show_value": ("show_value", lambda value: value),
+}
+
+_CHART_EXPLICIT_FORM_CONTROLS: dict[str, _FormControlMap] = {
+ "table": {
+ "sort_by": ("order_by_cols", _table_sort_value),
+ "column_config": ("column_config", _table_column_config_value),
+ },
+ "xy": {
+ "group_by": ("groupby", _column_names),
+ "series_limit": ("series_limit", lambda value: value),
+ "stacked": ("stack", lambda value: "Stack" if value else None),
+ "orientation": ("orientation", lambda value: value),
+ "legend_orientation": ("legendOrientation", lambda value: value),
+ "x_axis_time_format": ("x_axis_time_format", lambda value: value),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "mixed_timeseries": {
+ "group_by": ("groupby", _column_names),
+ "group_by_secondary": ("groupby_b", _column_names),
+ "currency_format_secondary": (
+ "currency_format_secondary",
+ _currency_form_value,
+ ),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "waterfall": {
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "big_number": {
+ "subheader": ("subheader", lambda value: value),
+ "y_axis_format": ("y_axis_format", lambda value: value),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ "compare_lag": ("compare_lag", lambda value: value),
+ "time_format": ("time_format", lambda value: value),
+ "aggregation": ("aggregation", lambda value: value),
+ },
+ "handlebars": {
+ "style_template": ("styleTemplate", lambda value: value),
+ "columns": ("all_columns", _column_names),
+ "groupby": ("groupby", _column_names),
+ "metrics": ("metrics", _column_names),
+ },
+ "pivot_table": {
+ "date_format": ("date_format", lambda value: value),
+ },
+ "interactive_pivot": {
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ "series_limit": ("series_limit", lambda value: value),
+ "date_format": ("date_format", lambda value: value),
+ "column_sort": ("colOrder", lambda value: value),
+ },
+}
+
+
+def _apply_explicit_form_controls( # noqa: C901
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Apply typed controls whose mapper omission represents a native clear."""
+ explicit_fields = set(getattr(config, "model_fields_set", set()))
+ controls = {
+ **_COMMON_EXPLICIT_FORM_CONTROLS,
+ **_CHART_EXPLICIT_FORM_CONTROLS.get(config.chart_type, {}),
+ }
+ for field_name, (native_key, convert) in controls.items():
+ if field_name in explicit_fields:
+ converted = convert(getattr(config, field_name))
+ is_clear = (
+ converted is None
+ or converted is False
+ or converted == ""
+ or converted in ([], {})
+ )
+ if is_clear:
+ new_form_data[native_key] = converted
+
+ axis_controls = {
+ "xy": (
+ ("x_axis", "x_axis_title", "x_axis_format", None),
+ ("y_axis", "y_axis_title", "y_axis_format", "y_axis_scale"),
+ ),
+ "mixed_timeseries": (
+ ("x_axis", "xAxisTitle", "x_axis_time_format", None),
+ ("y_axis", "yAxisTitle", "y_axis_format", "logAxis"),
+ (
+ "y_axis_secondary",
+ "yAxisTitleSecondary",
+ "y_axis_format_secondary",
+ "logAxisSecondary",
+ ),
+ ),
+ }
+ if config.chart_type in axis_controls:
+ for field_name, title_key, format_key, scale_key in axis_controls[
+ config.chart_type
+ ]:
+ if field_name not in explicit_fields:
+ continue
+ axis = getattr(config, field_name)
+ if axis is None:
+ new_form_data[title_key] = None
+ new_form_data[format_key] = None
+ if scale_key:
+ new_form_data[scale_key] = None
+ continue
+ axis_fields = set(axis.model_fields_set)
+ if "title" in axis_fields:
+ new_form_data[title_key] = axis.title
+ if "format" in axis_fields:
+ new_form_data[format_key] = axis.format
+ if scale_key and "scale" in axis_fields:
+ new_form_data[scale_key] = axis.scale
+
+ if config.chart_type == "xy" and "legend" in explicit_fields:
+ legend = config.legend
+ if legend is None:
+ new_form_data["show_legend"] = None
+ new_form_data["legendOrientation"] = None
+ else:
+ legend_fields = set(legend.model_fields_set)
+ if "show" in legend_fields:
+ new_form_data["show_legend"] = legend.show
+ if "position" in legend_fields:
+ new_form_data["legendOrientation"] = legend.position
+
+ if config.chart_type == "interactive_pivot":
+ if "temporal_column" in explicit_fields and config.temporal_column is
None:
+ new_form_data["granularity_sqla"] = None
+ new_form_data["temporal_columns_lookup"] = None
+ if (
+ "series_limit_metric" in explicit_fields
+ and config.series_limit_metric is None
+ ):
+ new_form_data["series_limit_metric"] = None
+ if "comparison_period" in explicit_fields and config.comparison_period
is None:
+ new_form_data["time_compare"] = None
+ if "comparison_type" in explicit_fields and config.comparison_type is
None:
+ new_form_data["comparison_type"] = None
+
+ # A Waterfall axis replacement cannot inherit a bucket belonging to the old
+ # temporal subject. Grain omission preserves only while the axis is stable;
+ # explicit null is already handled by the declarative control map above.
+ if (
+ config.chart_type == "waterfall"
+ and existing_form_data.get("x_axis") != new_form_data.get("x_axis")
+ and "time_grain" not in explicit_fields
+ ):
+ new_form_data["time_grain_sqla"] = None
+
+
+def merge_same_viz_form_data(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Preserve saved controls that the typed mapper does not represent.
+
+ The typed MCP surface deliberately models a bounded subset of every Explore
+ control panel. For a replacement within the exact same native ``viz_type``,
+ keys absent from the mapper therefore represent omitted controls and retain
+ their saved values. Mapper output and the chart-specific merge helpers run
+ first and remain authoritative, including explicit empty, false, null, and
+ nested values.
+
+ No generic state crosses a visualization boundary. This prevents query-role
+ keys from the previous plugin (for example ``metric`` or ``groupby``) from
+ leaking into a different plugin whose role contract is unrelated.
+ """
+ existing_viz_type = existing_form_data.get("viz_type")
+ if not isinstance(existing_viz_type, str) or existing_viz_type !=
new_form_data.get(
+ "viz_type"
+ ):
+ return
+
+ _apply_explicit_form_controls(existing_form_data, new_form_data, config)
+
+ for key, value in existing_form_data.items():
+ if key == MCP_DASHBOARD_TIME_FILTER_SUBJECT:
+ # merge_update_form_data owns this provenance marker. Its absence
+ # may be an intentional subject clear and must not be undone by the
+ # generic preservation layer.
+ continue
+ if key not in new_form_data:
+ new_form_data[key] = value
+
+
+def validate_merged_bullet_form_data(
+ form_data: Mapping[str, Any],
+ update_config: ChartConfig | None = None,
+) -> BulletChartConfig | None:
+ """Validate final Bullet state without reclassifying preserved filters.
+
+ The typed Bullet surface intentionally creates only SIMPLE WHERE filters,
+ while saved Explore state may legitimately contain SQL WHERE or SIMPLE
+ HAVING filters. When an update omitted ``filters``, those native objects
+ came from the saved state and are validated by the form-data/query layer;
+ removing them only from this schema-validation copy avoids pretending they
+ were newly supplied typed filters. Explicit filter replacements, including
+ ``[]``, still take the strict native-to-typed path.
+ """
+ if form_data.get("viz_type") != "bullet":
+ return None
+ validation_data = dict(form_data)
+ if isinstance(update_config, BulletChartConfig) and (
Review Comment:
Fixed in 703bb6cd. For `update_config is None`, Bullet schema validation now
uses a copy without native `adhoc_filters`, while the original byte-faithful
SQL/HAVING sequence remains in form data for target-dataset Tier-2 validation
and persistence. Direct and public FastMCP dataset-only rebind regressions
assert the native sequence and new datasource survive.
##########
tests/unit_tests/mcp_service/chart/test_bullet_chart.py:
##########
@@ -0,0 +1,3380 @@
+# 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.
+
+"""Product-path coverage for typed ECharts Bullet MCP support."""
+
+import math
+from datetime import date, datetime, time, timedelta, timezone, tzinfo
+from decimal import Decimal
+from enum import Enum, IntEnum, StrEnum
+from types import SimpleNamespace
+from typing import Any
+from unittest.mock import AsyncMock, MagicMock, patch
+from uuid import UUID
+from zoneinfo import ZoneInfo
+
+import numpy as np
+import pandas as pd
+import pytest
+import pytz
+from dateutil import tz as dateutil_tz
+from dateutil.zoneinfo import get_zonefile_instance
+from pydantic import TypeAdapter, ValidationError
+
+from superset.mcp_service.chart.chart_helpers import (
+ build_query_dicts_from_form_data,
+)
+from superset.mcp_service.chart.chart_utils import (
+ analyze_chart_capabilities,
+ map_bullet_config,
+ map_config_to_form_data,
+ MCP_DASHBOARD_TIME_FILTER_SUBJECT,
+ merge_update_form_data,
+ validate_merged_bullet_form_data,
+)
+from superset.mcp_service.chart.compile import _compile_chart
+from superset.mcp_service.chart.preview_utils import (
+ _generate_ascii_preview_from_data,
+ _generate_vega_lite_preview_from_data,
+ _javascript_number_string,
+ BulletOutputError,
+ generate_preview_from_form_data,
+ resolve_bullet_render_model,
+)
+from superset.mcp_service.chart.query_result import (
+ _chart_data_duration_text,
+ _chart_data_temporal_number,
+)
+from superset.mcp_service.chart.schemas import (
+ ASCIIPreview,
+ BulletChartConfig,
+ ChartConfig,
+ ChartError,
+ DataColumn,
+ GenerateChartRequest,
+ GetChartPreviewRequest,
+ UpdateChartPreviewRequest,
+ UpdateChartRequest,
+ VegaLitePreview,
+ XYChartConfig,
+)
+from superset.mcp_service.chart.tool.generate_chart import generate_chart
+from superset.mcp_service.chart.tool.get_chart_data import (
+ _candidates_single_numeric,
+ _VIZ_CATEGORY,
+)
+from superset.mcp_service.chart.tool.get_chart_preview import (
+ ASCIIPreviewStrategy,
+ TablePreviewStrategy,
+ VegaLitePreviewStrategy,
+)
+from superset.mcp_service.chart.tool.get_chart_type_schema import (
+ _get_chart_type_schema_impl,
+ VALID_CHART_TYPES,
+)
+from superset.mcp_service.chart.tool.update_chart import (
+ _build_preview_form_data,
+ _build_update_payload,
+ update_chart,
+)
+from superset.mcp_service.chart.tool.update_chart_preview import
update_chart_preview
+from superset.mcp_service.chart.validation.dataset_validator import
DatasetValidator
+from superset.mcp_service.common.error_schemas import DatasetContext
+from superset.utils.json import json_int_dttm_ser
+
+
+def _reject_scalar_conversion(*_args: object, **_kwargs: object) -> Any:
+ raise AssertionError("hostile query scalar method must not run")
+
+
+class _PathHostileStr(str):
+ __getitem__ = _reject_scalar_conversion
+ __str__ = _reject_scalar_conversion
+
+
+class _PathHostileEnum(str, Enum):
+ FAILED = "warehouse unavailable"
+
+ @property
+ def value(self) -> str:
+ """Reject the public descriptor while preserving Enum's stored
value."""
+ return _reject_scalar_conversion()
+
+ __getitem__ = _reject_scalar_conversion
+ __str__ = _reject_scalar_conversion
+
+
+class _OutputHostileInt(int):
+ __float__ = _reject_scalar_conversion
+ __str__ = _reject_scalar_conversion
+
+
+class _OutputHostileFloat(float):
+ __float__ = _reject_scalar_conversion
+ __str__ = _reject_scalar_conversion
+
+
+class _OutputHostileStr(str):
+ __str__ = _reject_scalar_conversion
+ strip = _reject_scalar_conversion
+
+
+class _OutputHostileDecimal(Decimal):
+ __float__ = _reject_scalar_conversion
+ __str__ = _reject_scalar_conversion
+
+
+class _OutputSafeIntEnum(IntEnum):
+ VALUE = 12
+
+
+class _OutputSafeStrEnum(StrEnum):
+ VALUE = "12.5"
+
+
+_OutputSafeIntEnum.__float__ = _reject_scalar_conversion # type:
ignore[method-assign]
+_OutputSafeIntEnum.__str__ = _reject_scalar_conversion # type:
ignore[method-assign]
+_OutputSafeStrEnum.__float__ = _reject_scalar_conversion # type:
ignore[attr-defined]
+_OutputSafeStrEnum.__str__ = _reject_scalar_conversion # type:
ignore[method-assign]
+
+
+def _simple_metric(name: str = "revenue") -> dict[str, str]:
+ return {"name": name, "aggregate": "SUM"}
+
+
+def _tool_user() -> SimpleNamespace:
+ return SimpleNamespace(id=1, username="admin", roles=[], groups=[])
+
+
+def _orm_dataset() -> SimpleNamespace:
+ def column(
+ name: str, type_: str, *, temporal: bool = False, numeric: bool = False
+ ) -> SimpleNamespace:
+ return SimpleNamespace(
+ column_name=name,
+ type=type_,
+ is_temporal=temporal,
+ is_numeric=numeric,
+ is_dttm=temporal,
+ python_date_format=None,
+ )
+
+ return SimpleNamespace(
+ id=7,
+ table_name="sales",
+ schema=None,
+ main_dttm_col="OrderDate",
+ database=SimpleNamespace(database_name="main", db_engine_spec=None),
+ columns=[
+ column("Revenue", "NUMERIC", numeric=True),
+ column("Region", "VARCHAR"),
+ column("Team", "VARCHAR"),
+ column("Status", "VARCHAR"),
+ column("OrderDate", "TIMESTAMP", temporal=True),
+ column("EventDate", "TIMESTAMP", temporal=True),
+ ],
+ metrics=[
+ SimpleNamespace(
+ metric_name="SavedRevenue",
+ expression="SUM(Revenue)",
+ description=None,
+ )
+ ],
+ )
+
+
+def test_bullet_discriminated_union_uses_exact_tag() -> None:
+ config = TypeAdapter(ChartConfig).validate_python(
+ {"chart_type": "bullet", "metric": _simple_metric()}
+ )
+ assert isinstance(config, BulletChartConfig)
+ with pytest.raises(ValidationError):
+ TypeAdapter(ChartConfig).validate_python(
+ {"chart_type": "bullet_chart", "metric": _simple_metric()}
+ )
+
+
+def test_bullet_equal_dimension_aliases_are_order_independent_and_round_trip()
-> None:
+ for payload in (
+ {
+ "dimensions": [{"name": "Region", "label": "Market"}, "Team"],
+ "groupby": ["Region", {"column_name": "Team"}],
+ },
+ {
+ "groupby": ["Region", {"column": "Team"}],
+ "dimensions": [{"name": "Region", "label": "Market"}, "Team"],
+ },
+ ):
+ config = BulletChartConfig.model_validate(
+ {"metric": _simple_metric(), **payload}
+ )
+ assert [dimension.name for dimension in config.dimensions or []] == [
+ "Region",
+ "Team",
+ ]
+ mapped = map_bullet_config(config)
+ assert mapped["groupby"] == ["Region", "Team"]
+ round_trip = BulletChartConfig.model_validate(mapped)
+ assert [dimension.name for dimension in round_trip.dimensions or []]
== [
+ "Region",
+ "Team",
+ ]
+
+
[email protected](
+ "request_payload",
+ [
+ {"dataset_id": 7},
+ {"identifier": 9},
+ {"dataset_id": 7, "form_data_key": "preview"},
+ ],
+)
[email protected]("reverse", [False, True])
+def test_bullet_request_models_reject_conflicting_dimension_aliases(
+ request_payload: dict[str, object], reverse: bool
+) -> None:
+ aliases = [
+ ("dimensions", [{"name": "Region"}, {"name": "Team"}]),
+ ("groupby", ["Team", "Region"]),
+ ]
+ if reverse:
+ aliases.reverse()
+ config = {"chart_type": "bullet", "metric": _simple_metric(),
**dict(aliases)}
+ payload = {**request_payload, "config": config}
+ request_type = (
+ UpdateChartRequest
+ if "identifier" in request_payload
+ else (
+ UpdateChartPreviewRequest
+ if "form_data_key" in request_payload
+ else GenerateChartRequest
+ )
+ )
+ with pytest.raises(ValidationError, match="Conflicting Bullet dimension
aliases"):
+ request_type.model_validate(payload)
+
+
[email protected](
+ "metric",
+ [
+ {"name": "revenue", "aggregate": "SUM", "label": "Revenue"},
+ {"name": "saved_revenue", "saved_metric": True},
+ {"sql_expression": "SUM(revenue) / COUNT(*)", "label": "Average"},
+ ],
+)
+def test_bullet_accepts_simple_saved_and_sql_metrics(metric: dict[str,
object]) -> None:
+ config = BulletChartConfig(metric=metric)
+ form_data = map_bullet_config(config)
+ assert form_data["viz_type"] == "bullet"
+ assert form_data["metric"]
+
+
+def test_bullet_native_form_data_round_trip_is_semantically_stable() -> None:
+ native = {
+ "viz_type": "bullet",
+ "datasource": "7__table",
+ "metric": {
+ "aggregate": "SUM",
+ "column": {"column_name": "Revenue"},
+ "expressionType": "SIMPLE",
+ "label": "Total Revenue",
+ },
+ "groupby": ["Region", "Team"],
+ "ranges": "100,250,500",
+ "range_labels": "Minimum,Target,Stretch",
+ "markers": "300",
+ "marker_labels": "Plan",
+ "marker_lines": "400",
+ "marker_line_labels": "Forecast",
+ "y_axis_format": "$,.0f",
+ "show_labels": False,
+ "show_legend": True,
+ "row_limit": 250,
+ "orderby": [["Region", True], ["Total Revenue", False]],
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "expressionType": "SIMPLE",
+ "subject": "Status",
+ "operator": "==",
+ "comparator": "Active",
+ }
+ ],
+ }
+ config = BulletChartConfig.model_validate(native)
+ mapped = map_bullet_config(config)
+
+ assert mapped["metric"]["label"] == "Total Revenue"
+ assert mapped["groupby"] == ["Region", "Team"]
+ assert mapped["ranges"] == "100,250,500"
+ assert mapped["range_labels"] == "Minimum,Target,Stretch"
+ assert mapped["markers"] == "300"
+ assert mapped["marker_lines"] == "400"
+ assert mapped["show_labels"] is False
+ assert mapped["show_legend"] is True
+ assert mapped["orderby"][0] == ["Region", True]
+ assert mapped["orderby"][1][0]["label"] == "Total Revenue"
+ assert mapped["adhoc_filters"][0]["subject"] == "Status"
+
+
+def test_bullet_presentation_numbers_use_shortest_round_trip_safe_tokens() ->
None:
+ ranges = [1.2345678901234567, 1.7976931348623157e308]
+ markers = [5e-324, -0.0]
+ marker_lines = [9.876543210987654e-200]
+ config = BulletChartConfig(
+ metric=_simple_metric(),
+ ranges=ranges,
+ markers=markers,
+ marker_lines=marker_lines,
+ show_legend=True,
+ )
+ mapped = map_bullet_config(config)
+
+ for key, expected in (
+ ("ranges", ranges),
+ ("markers", markers),
+ ("marker_lines", marker_lines),
+ ):
+ tokens = mapped[key].split(",")
+ assert [float(token) for token in tokens] == expected
+ assert all(
+ float(token).hex() == value.hex()
+ for token, value in zip(tokens, expected, strict=True)
+ )
+
+ round_trip = BulletChartConfig.model_validate(mapped)
+ assert round_trip.ranges == ranges
+ assert round_trip.markers == markers
+ assert round_trip.marker_lines == marker_lines
+
+ model = resolve_bullet_render_model(
+ [{"SUM(revenue)": 1.0}],
+ mapped,
+ )
+ assert model.ranges == ranges
+ assert model.markers == markers
+ assert model.marker_lines == marker_lines
+ assert (
+ "1.7976931348623157e+308"
+ in _generate_ascii_preview_from_data(
+ [{"SUM(revenue)": 1.0}], mapped
+ ).ascii_content
+ )
+ vega = _generate_vega_lite_preview_from_data([{"SUM(revenue)": 1.0}],
mapped)
+ assert vega.specification["layer"]
+
+
+def test_bullet_native_saved_metric_and_legacy_metric_aliases() -> None:
+ saved = BulletChartConfig.model_validate(
+ {"viz_type": "bullet", "metric": "saved_revenue"}
+ )
+ legacy = BulletChartConfig.model_validate(
+ {"viz_type": "bullet", "metric": "sum__revenue"}
+ )
+ assert saved.metric.saved_metric is True
+ assert saved.metric.name == "saved_revenue"
+ assert legacy.metric.name == "sum__revenue"
+ assert legacy.metric.saved_metric is True
+
+
[email protected]("metric_name", ["sum__num", "sum__SP_POP_TOTL"])
[email protected](
+ ("request_type", "request_fields"),
+ [
+ (GenerateChartRequest, {"dataset_id": 7}),
+ (UpdateChartRequest, {"identifier": 9}),
+ (UpdateChartPreviewRequest, {"dataset_id": 7}),
+ ],
+)
+def
test_bullet_repository_metric_names_round_trip_as_saved_metrics_on_all_requests(
+ metric_name: str,
+ request_type: type[
+ GenerateChartRequest | UpdateChartRequest | UpdateChartPreviewRequest
+ ],
+ request_fields: dict[str, object],
+) -> None:
+ request = request_type.model_validate(
+ {**request_fields, "config": {"chart_type": "bullet", "metric":
metric_name}}
+ )
+ config = request.config
+ assert isinstance(config, BulletChartConfig)
+ assert config.metric.saved_metric is True
+ assert map_bullet_config(config)["metric"] == metric_name
+
+
+def test_bullet_legacy_label_only_saved_metric_adapter_is_strict_and_bounded()
-> None:
+ config = BulletChartConfig.model_validate(
+ {"viz_type": "bullet", "metric": {"label": "sum__num"}}
+ )
+ assert config.metric.saved_metric is True
+ assert map_bullet_config(config)["metric"] == "sum__num"
+
+ with pytest.raises(ValidationError):
+ BulletChartConfig.model_validate(
+ {
+ "viz_type": "bullet",
+ "metric": {"label": "sum__num", "aggregate": "SUM"},
+ }
+ )
+ with pytest.raises(ValidationError, match="at most 255"):
+ BulletChartConfig.model_validate(
+ {"viz_type": "bullet", "metric": {"label": "m" * 256}}
+ )
+
+
[email protected](
+ "metric",
+ [
+ "SavedRevenue",
+ {
+ "aggregate": "SUM",
+ "column": {"column_name": "Revenue"},
+ "expressionType": "SIMPLE",
+ "label": "Simple Revenue",
+ },
+ {
+ "aggregate": None,
+ "column": None,
+ "expressionType": "SQL",
+ "sqlExpression": "SUM(Revenue)",
+ "label": "SQL Revenue",
+ },
+ ],
+)
+def test_bullet_all_metric_shapes_round_trip_full_native_presentation(
+ metric: object,
+) -> None:
+ native = {
+ "viz_type": "bullet",
+ "metric": metric,
+ "groupby": ["Region"],
+ "ranges": "50,100",
+ "range_labels": "Low,High",
+ "markers": "75",
+ "marker_labels": "Plan",
+ "marker_lines": "90",
+ "marker_line_labels": "Forecast",
+ "y_axis_format": "$,.0f",
+ "show_labels": True,
+ "show_legend": True,
+ }
+ mapped = map_bullet_config(BulletChartConfig.model_validate(native))
+ assert validate_merged_bullet_form_data(mapped) is not None
+ assert mapped["groupby"] == ["Region"]
+ assert mapped["ranges"] == "50,100"
+ assert mapped["marker_line_labels"] == "Forecast"
+
+
+def test_bullet_rejects_invalid_roles_and_output_collisions() -> None:
+ with pytest.raises(ValidationError, match="physical dimension"):
+ BulletChartConfig(
+ metric=_simple_metric(),
+ dimensions=[{"name": "region", "aggregate": "COUNT"}],
+ )
+ with pytest.raises(ValidationError, match="Duplicate Bullet dimension"):
+ BulletChartConfig(
+ metric=_simple_metric(),
+ dimensions=[{"name": "Region"}, {"name": "region"}],
+ )
+ with pytest.raises(ValidationError, match="conflicts with a dimension"):
+ BulletChartConfig(
+ metric={"name": "revenue", "aggregate": "SUM", "label": "Region"},
+ dimensions=[{"name": "region", "label": "Region"}],
+ )
+
+
+def test_bullet_rejects_misaligned_labels_and_bad_order_target() -> None:
+ with pytest.raises(ValidationError, match="one label per ranges"):
+ BulletChartConfig(
+ metric=_simple_metric(), ranges=[1, 2], range_labels=["Only one"]
+ )
+ with pytest.raises(ValidationError, match="unknown: not_a_role"):
+ BulletChartConfig(metric=_simple_metric(), order_by=[{"column":
"not_a_role"}])
+
+
+def test_bullet_dimension_labels_are_input_aliases_not_result_aliases() ->
None:
+ config = BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Total"},
+ dimensions=[
+ {"name": "Team", "label": "Region"},
+ {"name": "Region", "label": "Market"},
+ ],
+ # The exact physical Region must win over Team's display label.
+ order_by=[
+ {"column": "Region", "ascending": True},
+ {"column": "Revenue", "ascending": False},
+ ],
+ )
+ form_data = map_bullet_config(config)
+ assert form_data["groupby"] == ["Team", "Region"]
+ assert form_data["orderby"] == [
+ ["Region", True],
+ [form_data["metric"], False],
+ ]
+
+ label_order = map_bullet_config(
+ BulletChartConfig(
+ metric=config.metric,
+ dimensions=config.dimensions,
+ order_by=[{"column": "Market"}],
+ )
+ )
+ assert label_order["orderby"] == [["Region", False]]
+
+ model = resolve_bullet_render_model(
+ [{"Team": "Blue", "Region": "North", "Total": 10}], form_data
+ )
+ assert model.dimensions == ["Team", "Region"]
+ assert [model.rows[0][name] for name in model.dimensions] == ["Blue",
"North"]
+
+
+def test_bullet_rejects_ambiguous_display_alias_for_ordering() -> None:
+ with pytest.raises(ValidationError, match="ambiguous display alias"):
+ BulletChartConfig(
+ metric=_simple_metric(),
+ dimensions=[
+ {"name": "Region", "label": "Area"},
+ {"name": "Team", "label": "area"},
+ ],
+ order_by=[{"column": "AREA"}],
+ )
+
+
[email protected](
+ ("metric", "order_target", "output"),
+ [
+ (
+ {"name": "SavedRevenue", "saved_metric": True, "label":
"Friendly"},
+ "Friendly",
+ "SavedRevenue",
+ ),
+ (
+ {"name": "Revenue", "aggregate": "SUM", "label": "Simple Total"},
+ "Revenue",
+ "Simple Total",
+ ),
+ (
+ {"sql_expression": "SUM(Revenue)", "label": "SQL Total"},
+ "SQL Total",
+ "SQL Total",
+ ),
+ ],
+)
+def test_bullet_metric_shapes_share_physical_dimension_output_contract(
+ metric: dict[str, object], order_target: str, output: str
+) -> None:
+ config = BulletChartConfig(
+ metric=metric,
+ dimensions=[{"name": "Region", "label": "Market"}],
+ order_by=[{"column": order_target}],
+ )
+ form_data = map_bullet_config(config)
+ assert form_data["groupby"] == ["Region"]
+ assert form_data["orderby"] == [[form_data["metric"], False]]
+ model = resolve_bullet_render_model([{"Region": "North", output: 12}],
form_data)
+ assert model.metric_field == output
+ assert model.dimensions == ["Region"]
+
+
+def test_bullet_mapper_preserves_omission_and_honors_explicit_values() -> None:
+ omitted = map_bullet_config(BulletChartConfig(metric=_simple_metric()))
+ explicit = map_bullet_config(
+ BulletChartConfig(
+ metric=_simple_metric(),
+ dimensions=[],
+ filters=[],
+ ranges=[],
+ show_labels=False,
+ show_legend=False,
+ row_limit=42,
+ time_range=None,
+ )
+ )
+ for key in (
+ "groupby",
+ "adhoc_filters",
+ "ranges",
+ "show_labels",
+ "show_legend",
+ "row_limit",
+ "time_range",
+ ):
+ assert key not in omitted
+ assert explicit["groupby"] == []
+ assert explicit["adhoc_filters"] == []
+ assert explicit["ranges"] == ""
+ assert explicit["show_labels"] is False
+ assert explicit["show_legend"] is False
+ assert explicit["row_limit"] == 42
+ assert explicit["time_range"] is None
+
+
+def test_bullet_registry_schema_and_recommendation_metadata() -> None:
+ from superset.mcp_service.app import get_default_instructions
+ from superset.mcp_service.chart.registry import display_name_for_viz_type,
get
+
+ plugin = get("bullet")
+ assert plugin is not None
+ assert plugin.resolve_viz_type(None) == "bullet"
+ assert display_name_for_viz_type("bullet") == "Bullet Chart"
+ assert "bullet" in VALID_CHART_TYPES
+ discovered = _get_chart_type_schema_impl("bullet")
+ assert discovered["chart_type"] == "bullet"
+ assert discovered["examples"][0]["ranges"] == [100000, 250000, 500000]
+ assert _VIZ_CATEGORY["bullet"] == "bullet"
+ candidates = _candidates_single_numeric(
+ DataColumn(
+ name="Revenue",
+ display_name="Revenue",
+ data_type="numeric",
+ sample_values=[1],
+ null_count=0,
+ unique_count=1,
+ ),
+ row_count=1,
+ )
+ assert "bullet chart" in candidates
+ guidance = get_default_instructions()
+ assert 'chart_type="bullet": Bullet Chart' in guidance
+ assert "waterfall, bullet, and interactive_pivot" in guidance
+
+
+def test_bullet_dataset_normalization_canonicalizes_every_reference() -> None:
+ from superset.mcp_service.chart.registry import get
+
+ context = DatasetContext(
+ id=7,
+ table_name="sales",
+ schema=None,
+ database_name="main",
+ available_columns=[
+ {"name": "Revenue", "type": "NUMERIC", "is_numeric": True},
+ {"name": "Region", "type": "VARCHAR"},
+ {"name": "OrderDate", "type": "TIMESTAMP", "is_temporal": True},
+ {"name": "Status", "type": "VARCHAR"},
+ ],
+ available_metrics=[],
+ )
+ config = BulletChartConfig(
+ metric={"name": "revenue", "aggregate": "SUM"},
+ dimensions=[{"name": "region"}],
+ temporal_column="orderdate",
+ filters=[{"column": "status", "op": "=", "value": "active"}],
+ order_by=[{"column": "region", "ascending": True}],
+ )
+ plugin = get("bullet")
+ assert plugin is not None
+ normalized = plugin.normalize_column_refs(config, context)
+ assert normalized.metric.name == "Revenue"
+ assert normalized.dimensions[0].name == "Region"
+ assert normalized.temporal_column == "OrderDate"
+ assert normalized.filters[0].column == "Status"
+ assert normalized.order_by[0].column == "Region"
+ assert normalized.model_fields_set == config.model_fields_set
+
+
+def test_bullet_dataset_normalization_rejects_ambiguous_casefold_candidates()
-> None:
+ from superset.mcp_service.chart.registry import get
+
+ context = DatasetContext(
+ id=7,
+ table_name="sales",
+ schema=None,
+ database_name="main",
+ available_columns=[
+ {"name": "Revenue", "type": "NUMERIC", "is_numeric": True},
+ {"name": "revenue", "type": "NUMERIC", "is_numeric": True},
+ ],
+ available_metrics=[],
+ )
+ plugin = get("bullet")
+ assert plugin is not None
+ config = BulletChartConfig(metric={"name": "REVENUE", "aggregate": "SUM"})
+ with pytest.raises(ValueError, match="Revenue, revenue"):
+ plugin.normalize_column_refs(config, context)
+
+
+def test_bullet_numeric_output_constraint_rejects_text_min() -> None:
+ from superset.mcp_service.chart.registry import get
+
+ context = DatasetContext(
+ id=7,
+ table_name="sales",
+ schema=None,
+ database_name="main",
+ available_columns=[{"name": "status", "type": "VARCHAR"}],
+ available_metrics=[],
+ )
+ plugin = get("bullet")
+ assert plugin is not None
+ config = BulletChartConfig(metric={"name": "status", "aggregate": "MIN"})
+ with patch.object(DatasetValidator, "_get_dataset_context",
return_value=context):
+ error = plugin.post_map_validate(config, {}, dataset_id=7)
+ assert error is not None
+ assert error.error_type == "non_numeric_bullet_metric"
+
+
[email protected]("reverse_metadata", [False, True])
+def test_bullet_exact_case_type_and_role_resolution_is_order_independent(
+ reverse_metadata: bool,
+) -> None:
+ from superset.mcp_service.chart.registry import get
+
+ columns = [
+ {"name": "Revenue", "type": "NUMERIC", "is_numeric": True},
+ {"name": "revenue", "type": "VARCHAR", "is_numeric": False},
+ {"name": "Region", "type": "VARCHAR"},
+ ]
+ if reverse_metadata:
+ columns.reverse()
+ context = DatasetContext(
+ id=7,
+ table_name="sales",
+ schema=None,
+ database_name="main",
+ available_columns=columns,
+ available_metrics=[],
+ )
+ plugin = get("bullet")
+ assert plugin is not None
+
+ numeric = BulletChartConfig(metric={"name": "Revenue", "aggregate": "SUM"})
+ text = BulletChartConfig(metric={"name": "revenue", "aggregate": "MIN"})
+ with patch.object(DatasetValidator, "_get_dataset_context",
return_value=context):
+ assert plugin.post_map_validate(numeric, {}, dataset_id=7) is None
+ error = plugin.post_map_validate(text, {}, dataset_id=7)
+ assert error is not None
+ assert error.error_type == "non_numeric_bullet_metric"
+
+ roles = BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM"},
+ dimensions=[{"name": "revenue"}, {"name": "Region"}],
+ filters=[{"column": "revenue", "op": "=", "value": "retail"}],
+ order_by=[{"column": "revenue", "ascending": True}],
+ )
+ normalized = plugin.normalize_column_refs(roles, context)
+ assert normalized.metric.name == "Revenue"
+ assert [dimension.name for dimension in normalized.dimensions or []] == [
+ "revenue",
+ "Region",
+ ]
+ assert normalized.filters
+ assert normalized.filters[0].column == "revenue"
+ assert normalized.order_by[0].column == "revenue"
+
+ ambiguous = BulletChartConfig(metric={"name": "REVENUE", "aggregate":
"SUM"})
+ with pytest.raises(ValueError, match="Ambiguous"):
+ plugin.normalize_column_refs(ambiguous, context)
+
+
[email protected]("reverse_metadata", [False, True])
+def test_generic_aggregation_validation_uses_exact_case_before_type(
+ reverse_metadata: bool,
+) -> None:
+ from superset.mcp_service.chart.schemas import PieChartConfig
+
+ columns = [
+ {"name": "Amount", "type": "BIGINT", "is_numeric": True},
+ {"name": "amount", "type": "VARCHAR", "is_numeric": False},
+ ]
+ if reverse_metadata:
+ columns.reverse()
+ context = DatasetContext(
+ id=7,
+ table_name="sales",
+ schema=None,
+ database_name="main",
+ available_columns=columns,
+ available_metrics=[],
+ )
+
+ assert (
+ DatasetValidator._validate_aggregations(
+ [BulletChartConfig(metric={"name": "Amount", "aggregate":
"SUM"}).metric],
+ context,
+ )
+ == []
+ )
+ errors = DatasetValidator._validate_aggregations(
+ [BulletChartConfig(metric={"name": "amount", "aggregate":
"SUM"}).metric],
+ context,
+ )
+ assert errors
+ assert errors[0].error_type == "invalid_aggregation"
+
+ ambiguous = DatasetValidator._validate_aggregations(
+ [BulletChartConfig(metric={"name": "AMOUNT", "aggregate":
"SUM"}).metric],
+ context,
+ )
+ assert ambiguous
+ assert ambiguous[0].error_type == "ambiguous_column_reference"
+
+ valid, error = DatasetValidator.validate_against_dataset(
+ PieChartConfig(
+ dimension={"name": "amount"},
+ metric={"name": "Amount", "aggregate": "SUM"},
+ ),
+ 7,
+ dataset_context=context,
+ )
+ assert valid is True
+ assert error is None
+
+
[email protected](
+ ("metric", "field"),
+ [
+ ({"name": "Revenue", "aggregate": "SUM", "label": "Simple"}, "Simple"),
+ ({"name": "SavedRevenue", "saved_metric": True}, "SavedRevenue"),
+ ({"sql_expression": "SUM(Revenue)", "label": "SQL Total"}, "SQL
Total"),
+ ],
+)
+def test_bullet_result_roles_are_exact_for_every_metric_shape(
+ metric: dict[str, object], field: str
+) -> None:
+ form_data = map_bullet_config(BulletChartConfig(metric=metric))
+ model = resolve_bullet_render_model([{field.swapcase(): "12.5"}],
form_data)
+ assert model.metric_field == field.swapcase()
+ assert model.measures == [12.5]
+
+
[email protected](
+ "rows, message",
+ [
+ ([{"other": 123}], "missing"),
+ ([{"Revenue": "not a number"}], "non-numeric text"),
+ ([{"Revenue": math.nan}], "NaN or infinite"),
+ ([{"Revenue": math.inf}], "NaN or infinite"),
+ ([{"Revenue": 1}, {}], "row 1.*missing"),
+ ([{"REVENUE": 1, "revenue": 2}], "ambiguous"),
+ ],
+)
+def test_bullet_result_validation_rejects_malformed_rows(
+ rows: list[dict[str, object]], message: str
+) -> None:
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "amount", "aggregate": "SUM", "label": "Revenue"}
+ )
+ )
+ with pytest.raises(BulletOutputError, match=message):
+ resolve_bullet_render_model(rows, form_data)
+
+
+def test_bullet_result_validation_accepts_null_and_numeric_strings() -> None:
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "amount", "aggregate": "SUM", "label": "Revenue"},
+ dimensions=[{"name": "Region"}],
+ )
+ )
+ model = resolve_bullet_render_model(
+ [
+ {"Region": "North", "Revenue": None},
+ {"Region": "South", "Revenue": " 4.25 "},
+ ],
+ form_data,
+ )
+ assert model.measures == [0.0, 4.25]
+
+
[email protected](
+ ("presentation", "message"),
+ [
+ ({"ranges": "10,nope"}, r"ranges\[1\].*not numeric"),
+ ({"markers": "NaN"}, r"markers\[0\].*NaN or infinite"),
+ (
+ {"ranges": "10,20", "range_labels": "Only one"},
+ "one label per value",
+ ),
+ ],
+)
+def test_bullet_result_validation_rejects_malformed_presentation(
+ presentation: dict[str, object], message: str
+) -> None:
+ form_data = {
+ **map_bullet_config(
+ BulletChartConfig(metric={"name": "amount", "aggregate": "SUM"})
+ ),
+ **presentation,
+ }
+ with pytest.raises(BulletOutputError, match=message):
+ resolve_bullet_render_model([{"SUM(amount)": 1}], form_data)
+
+
+def test_bullet_compile_accepts_empty_ungrouped_result() -> None:
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"}
+ )
+ )
+ factory = MagicMock()
+ factory.create.return_value = MagicMock()
+ command = MagicMock()
+ command.run.return_value = {"queries": [{"data": []}]}
+ with (
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory",
+ return_value=factory,
+ ),
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand",
+ return_value=command,
+ ),
+ ):
+ result = _compile_chart(form_data, 7)
+ assert result.success is True
+ assert result.row_count == 0
+
+
+def test_bullet_compile_inspects_top_level_and_query_error_envelopes() -> None:
+ form_data = map_bullet_config(
+ BulletChartConfig(metric={"name": "Revenue", "aggregate": "SUM"})
+ )
+ factory = MagicMock()
+ factory.create.return_value = MagicMock()
+ command = MagicMock()
+ command.run.return_value = {
+ "status": "success",
+ "queries": [{"status": "failed", "message": "warehouse timeout"}],
+ }
+ with (
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory",
+ return_value=factory,
+ ),
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand",
+ return_value=command,
+ ),
+ ):
+ result = _compile_chart(form_data, 7)
+ assert result.success is False
+ assert "warehouse timeout" in (result.error or "")
+
+
+_MALFORMED_QUERY_ENVELOPES: list[object] = [
+ None,
+ [],
+ {},
+ {"queries": None},
+ {"queries": []},
+ {"queries": [None]},
+ {"queries": [{}]},
+ {"queries": [{"data": None}]},
+ {"queries": [{"data": []}, {"data": "not-an-array"}]},
+ {
+ "queries": [
+ {
+ "data": [{"Revenue": 12}],
+ "colnames": ["Revenue"],
+ "coltypes": [],
+ }
+ ]
+ },
+]
+
+
+def _compile_bullet_with_result(result: object) -> Any:
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"}
+ )
+ )
+ factory = MagicMock()
+ factory.create.return_value = MagicMock()
+ command = MagicMock()
+ command.run.return_value = result
+ with (
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory",
+ return_value=factory,
+ ),
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand",
+ return_value=command,
+ ),
+ ):
+ return _compile_chart(form_data, 7)
+
+
[email protected]("envelope", _MALFORMED_QUERY_ENVELOPES)
+def test_bullet_compile_returns_stable_error_for_malformed_envelopes(
+ envelope: object,
+) -> None:
+ result = _compile_bullet_with_result(envelope)
+ assert result.success is False
+ assert result.error_code == "CHART_COMPILE_FAILED"
+ assert result.error_obj is not None
+ assert result.error_obj.error_type == "compile_error"
+
+
[email protected](
+ ("data", "expected_code", "expected_type"),
+ [
+ ([1], "CHART_COMPILE_FAILED", "compile_error"),
+ (
+ [{"Revenue": 10**10000}],
+ "CHART_COMPILE_FAILED",
+ "compile_error",
+ ),
+ ],
+)
+def test_bullet_compile_returns_malformed_output_for_bad_rows(
+ data: list[object],
+ expected_code: str,
+ expected_type: str,
+) -> None:
+ result = _compile_bullet_with_result({"queries": [{"data": data}]})
+ assert result.success is False
+ assert result.error_code == expected_code
+ assert result.error_obj is not None
+ assert result.error_obj.error_type == expected_type
+
+
+def test_bullet_shared_query_builder_matches_frontend_build_query() -> None:
+ metric = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"},
+ dimensions=[{"name": "Region"}],
+ row_limit=25,
+ order_by=[{"column": "Revenue", "ascending": False}],
+ )
+ )
+ with patch(
+ "superset.mcp_service.chart.chart_helpers.resolve_datasource_engine",
+ return_value="base",
+ ):
+ queries = build_query_dicts_from_form_data(metric, 7, "table")
+ assert len(queries) == 1
+ assert queries[0]["columns"] == ["Region"]
+ assert queries[0]["metrics"] == [metric["metric"]]
+ assert queries[0]["orderby"] == metric["orderby"]
+ assert queries[0]["row_limit"] == 25
+
+
+def test_bullet_compile_path_uses_groupby_metric_orderby_and_usable_result()
-> None:
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"},
+ dimensions=[{"name": "Region"}],
+ order_by=[{"column": "Revenue", "ascending": False}],
+ )
+ )
+ context = MagicMock()
+ factory = MagicMock()
+ factory.create.return_value = context
+ command = MagicMock()
+ command.run.return_value = {
+ "queries": [{"data": [{"Region": "North", "Revenue": "12.5"}]}]
+ }
+ with (
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory",
+ return_value=factory,
+ ),
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand",
+ return_value=command,
+ ),
+ ):
+ result = _compile_chart(form_data, 7)
+ query = factory.create.call_args.kwargs["queries"][0]
+ assert query["columns"] == ["Region"]
+ assert query["metrics"] == [form_data["metric"]]
+ assert query["orderby"] == form_data["orderby"]
+ assert result.success is True
+ assert result.row_count == 1
+
+
+def test_bullet_compile_projects_dataframe_timestamp_before_validation() ->
None:
+ from superset.dataframe import df_to_records
+
+ dublin = dateutil_tz.gettz("Europe/Dublin")
+ new_york = dateutil_tz.gettz("America/New_York")
+ assert dublin is not None
+ assert new_york is not None
+ source_values = [
+ pd.Timestamp("2024-01-02 03:04:05.123456789"),
+ datetime(2024, 10, 27, 1, 30, tzinfo=dublin, fold=1),
+ datetime(2024, 3, 10, 2, 30, tzinfo=new_york),
+ datetime(2040, 7, 1, 12, tzinfo=new_york),
+ ]
+ form_data = map_bullet_config(
+ BulletChartConfig(
+ metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"},
+ dimensions=[{"name": "Category"}],
+ )
+ )
+ rows = df_to_records(
+ pd.DataFrame(
+ {
+ "Category": pd.Series(source_values, dtype=object),
+ "Revenue": range(1, len(source_values) + 1),
+ }
+ ),
+ convert_big_integers=False,
+ )
+ factory = MagicMock()
+ factory.create.return_value = MagicMock()
+ command = MagicMock()
+ command.run.return_value = {"queries": [{"data": rows}]}
+ captured: list[list[dict[str, Any]]] = []
+
+ def validate(data: list[dict[str, Any]], config: dict[str, Any]) -> Any:
+ captured.append(data)
+ return resolve_bullet_render_model(data, config)
+
+ with (
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory",
+ return_value=factory,
+ ),
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand",
+ return_value=command,
+ ),
+ patch(
+
"superset.mcp_service.chart.preview_utils.resolve_bullet_render_model",
+ side_effect=validate,
+ ),
+ ):
+ result = _compile_chart(form_data, 7)
+
+ assert result.success is True
+ assert [row["Category"] for row in captured[0]] == [
+ 1704164645123.456,
+ 1729989000000.0,
+ 1710052200000.0,
+ 2224774800000.0,
+ ]
+
+
+def test_bullet_compile_projects_real_dataframe_durations_to_chart_data_wire()
-> None:
+ from superset.commands.chart.data.get_data_command import ChartDataCommand
+ from superset.common.chart_data import ChartDataResultType
+ from superset.dataframe import df_to_records
+ from superset.utils import json
+
+ source_values = [
+ timedelta(0),
+ timedelta(days=1, seconds=2, microseconds=3),
+ pd.Timedelta(-1, unit="ns"),
+ pd.Timedelta("1 days 00:00:02.000003004"),
+ np.timedelta64(123456789, "ns"),
+ np.timedelta64("NaT"),
+ ]
+ rows = df_to_records(
+ pd.DataFrame(
+ {
+ "Duration": pd.Series(source_values, dtype=object),
+ "Revenue": range(1, len(source_values) + 1),
+ }
+ ),
+ convert_big_integers=False,
+ )
+ expected = [
+ None
+ if row["Duration"] is None
+ else json.loads(json.dumps(row["Duration"],
default=json.json_int_dttm_ser))
+ for row in rows
+ ]
Review Comment:
Reproduced; no product/test crash occurs. `df_to_records(pd.DataFrame(...))`
promotes exact `np.timedelta64(123456789, "ns")` to exact `pd.Timedelta` and
converts `np.timedelta64("NaT")` to `None` before this expected-value
calculation. 703bb6cd adds those producer-boundary assertions immediately
before the existing `json_int_dttm_ser` parity check. The test passes in the
1,451-test focused run and 4,386-test MCP run.
##########
tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.py:
##########
@@ -45,12 +53,1126 @@
_first_query_has_fields,
_no_query_fields_error,
ASCIIPreviewStrategy,
+ get_chart_preview,
PreviewFormatStrategy,
TablePreviewStrategy,
)
from superset.utils import json as utils_json
+def _query_context_stub(form_data: dict[str, Any] | None = None) -> Any:
+ """Return the minimal real-shaped context needed by Jinja form-data
seeding."""
+ return SimpleNamespace(form_data=form_data or {}, queries=[])
+
+
+def _entrypoint_preview(content: str) -> ChartPreview:
+ return ChartPreview(
+ chart_id=1,
+ chart_name="",
+ chart_type="bullet",
+ explore_url="",
+ content=ASCIIPreview(ascii_content=content, width=80, height=20),
+ chart_description="",
+ accessibility=AccessibilityMetadata(
+ color_blind_safe=True,
+ alt_text="",
+ high_contrast_available=False,
+ ),
+ performance=PerformanceMetadata(
+ query_duration_ms=0,
+ cache_status="miss",
+ optimization_suggestions=[],
+ ),
+ )
+
+
[email protected]
[email protected](
+ ("request_payload", "extra", "expected_type"),
+ [
+ ({"id": 1, "format": "ascii"}, 0, ChartPreview),
+ ({"form_data_key": "cached-preview", "format": "ascii"}, 1,
ChartError),
+ ],
+ ids=["identifier-alias-exact-limit", "cached-preview-limit-plus-one"],
+)
+async def
test_get_chart_preview_entrypoint_preflights_complete_exact_wire_response(
+ request_payload: dict[str, object], extra: int, expected_type: type[object]
+) -> None:
+ empty = _entrypoint_preview("")
+ filler = "x" * (
+ MAX_QUERY_RESULT_VALUE_BYTES - len(empty.model_dump_json().encode()) +
extra
+ )
+ candidate = _entrypoint_preview(filler)
+ request = GetChartPreviewRequest.model_validate(request_payload)
+ ctx = MagicMock()
+ ctx.info = AsyncMock()
+ ctx.debug = AsyncMock()
+ ctx.warning = AsyncMock()
+
+ user = MagicMock(id=1, username="admin", roles=[], groups=[])
+ with (
+ patch("superset.mcp_service.auth.get_user_from_request",
return_value=user),
+ patch(
+ "superset.mcp_service.chart.tool.get_chart_preview."
+ "_get_chart_preview_internal",
+ new=AsyncMock(return_value=candidate),
+ ),
+ ):
+ result = await get_chart_preview(request, ctx=ctx)
+
+ assert isinstance(result, expected_type)
+ if extra == 0:
+ assert len(candidate.model_dump_json().encode()) == (
+ MAX_QUERY_RESULT_VALUE_BYTES
+ )
+ assert result is candidate
+ else:
+ assert isinstance(result, ChartError)
+ assert result.error_type == "MalformedQueryResult"
+
+
[email protected]
[email protected]("format_", ["ascii", "vega_lite"])
+async def
test_bullet_numeric_and_temporal_categories_reach_real_mcp_entrypoint(
+ format_: str,
+) -> None:
+ from contextlib import nullcontext
+
+ from fastmcp import Client
+
+ from superset.mcp_service.app import mcp
+
+ preview_module = importlib.import_module(
+ "superset.mcp_service.chart.tool.get_chart_preview"
+ )
+ command_module = importlib.import_module(
+ "superset.commands.chart.data.get_data_command"
+ )
+ form_data = {
+ "viz_type": "bullet",
+ "metric": "Revenue",
+ "groupby": ["Category"],
+ }
+ chart = SimpleNamespace(
+ id=121,
+ slice_name="Number boundaries",
+ viz_type="bullet",
+ datasource_id=1,
+ datasource_type="table",
+ params=utils_json.dumps(form_data),
+ )
+ rows = [
+ {"Category": 9007199254740993, "Revenue": 1},
+ {"Category": Decimal("1.0000000000000001"), "Revenue": 2},
+ {"Category": Decimal("1.7976931348623159e308"), "Revenue": 3},
+ {"Category": date(2026, 9, 2), "Revenue": 4},
+ {
+ "Category": datetime(2026, 9, 2, 3, 4, 5, tzinfo=timezone.utc),
+ "Revenue": 5,
+ },
+ {
+ "Category": datetime(
+ 2023,
+ 11,
+ 5,
+ 1,
+ 30,
+ tzinfo=ZoneInfo("America/New_York"),
+ fold=0,
+ ),
+ "Revenue": 6,
+ },
+ {
+ "Category": datetime(
+ 2023,
+ 11,
+ 5,
+ 1,
+ 30,
+ tzinfo=ZoneInfo("America/New_York"),
+ fold=1,
+ ),
+ "Revenue": 7,
+ },
+ ]
+
+ class _Command:
+ def __init__(self, _query_context: Any) -> None: ...
+
+ def validate(self) -> None: ...
+
+ def run(self) -> dict[str, Any]:
+ return {
+ "queries": [
+ {
+ "data": rows,
+ "colnames": ["Category", "Revenue"],
+ }
+ ]
+ }
+
+ query_context = SimpleNamespace(
+ form_data={},
+ queries=[SimpleNamespace(metrics=["Revenue"], columns=["Category"])],
+ )
+ user = MagicMock(id=1, username="admin", roles=[], groups=[])
+ with (
+ patch("superset.mcp_service.auth.get_user_from_request",
return_value=user),
+ patch("superset.mcp_service.auth.check_tool_permission",
return_value=True),
+ patch.object(preview_module, "find_chart_by_identifier",
return_value=chart),
+ patch.object(preview_module.db.session, "refresh", return_value=None),
+ patch.object(
+ preview_module,
+ "validate_chart_dataset",
+ return_value=SimpleNamespace(is_valid=True, warnings=[],
error=None),
+ ),
+ patch.object(
+ preview_module.event_logger,
+ "log_context",
+ side_effect=lambda **_kwargs: nullcontext(),
+ ),
+ patch.object(
+ preview_module,
+ "build_query_context_from_form_data",
+ return_value=query_context,
+ ),
+ patch.object(preview_module, "set_query_context_form_data",
return_value=None),
+ patch.object(command_module, "ChartDataCommand", _Command),
+ patch.object(
+ preview_module, "get_superset_base_url",
return_value="http://localhost";
+ ),
+ ):
+ async with Client(mcp) as client:
+ result = await client.call_tool(
+ "get_chart_preview",
+ {"request": {"id": 121, "format": format_}},
+ )
+
+ payload = utils_json.loads(result.content[0].text)
+ if format_ == "ascii":
+ content = payload["content"]["ascii_content"]
+ assert "9007199254740992" in content
+ assert "Infinity" in content
+ assert "1788307200000" in content
+ assert "1699162200000" in content
+ assert "1699165800000" in content
+ else:
+ specification = payload["content"]["specification"]
+ bar = next(
+ layer for layer in specification["layer"] if layer["mark"]["type"]
== "bar"
+ )
+ category_field = bar["encoding"]["y"]["field"]
+ assert [row[category_field] for row in
specification["data"]["values"]] == [
+ "9007199254740992",
+ "1",
+ "Infinity",
+ "1788307200000",
+ "1788318245000",
+ "1699162200000",
+ "1699165800000",
+ ]
+ assert [row["Category"] for row in
specification["data"]["values"][3:]] == [
+ 1788307200000.0,
+ 1788318245000.0,
+ 1699162200000.0,
+ 1699165800000.0,
+ ]
+ assert bar["encoding"]["tooltip"][0]["field"] == category_field
+ assert "transform" not in specification
+
+
[email protected]
[email protected]("format_", ["ascii", "vega_lite"])
+async def test_bullet_timestamp_categories_from_dataframe_reach_fastmcp(
+ format_: str,
+) -> None:
+ from contextlib import nullcontext
+
+ from fastmcp import Client
+
+ from superset.dataframe import df_to_records
+ from superset.mcp_service.app import mcp
+
+ preview_module = importlib.import_module(
+ "superset.mcp_service.chart.tool.get_chart_preview"
+ )
+ command_module = importlib.import_module(
+ "superset.commands.chart.data.get_data_command"
+ )
+ zoneinfo_tz = ZoneInfo("America/New_York")
+ pytz_tz = pytz.timezone("America/New_York")
+ dateutil_dublin = dateutil_tz.gettz("Europe/Dublin")
+ dateutil_new_york = dateutil_tz.gettz("America/New_York")
+ assert dateutil_dublin is not None
+ assert dateutil_new_york is not None
+ dublin_fold = datetime(
+ 2024,
+ 10,
+ 27,
+ 1,
+ 30,
+ 0,
+ 123456,
+ tzinfo=dateutil_dublin,
+ fold=1,
+ )
+ new_york_gap = datetime(2024, 3, 10, 2, 30, 0, 123456,
tzinfo=dateutil_new_york)
+ source_values = [
+ pd.Timestamp("2024-01-02 03:04:05.123456789"),
+ pd.Timestamp("2024-01-02 08:34:05.123456789+05:30"),
+ pd.Timestamp(datetime(2024, 11, 3, 1, 30, tzinfo=zoneinfo_tz, fold=0)),
+ pd.Timestamp(datetime(2024, 11, 3, 1, 30, tzinfo=zoneinfo_tz, fold=1)),
+ pd.Timestamp(pytz_tz.localize(datetime(2024, 11, 3, 1, 30),
is_dst=True)),
+ pd.Timestamp(pytz_tz.localize(datetime(2024, 11, 3, 1, 30),
is_dst=False)),
+ pd.Timestamp("1969-12-31 23:59:59.999999999"),
+ date(2024, 1, 2),
+ pd.NaT,
+ dublin_fold,
+ new_york_gap,
+ datetime(2040, 7, 1, 12, 0, 0, 123456, tzinfo=dateutil_new_york),
+ datetime(
+ 2024,
+ 3,
+ 10,
+ 2,
+ 30,
+ 0,
+ 123456,
+ tzinfo=dateutil_tz.tzoffset("EDT", -4 * 3600),
+ ),
+ datetime(2024, 3, 10, 6, 30, 0, 123456, tzinfo=timezone.utc),
+ pd.Timestamp(dublin_fold),
+ pd.Timestamp(new_york_gap),
+ ]
+ rows = df_to_records(
+ pd.DataFrame(
+ {
+ "Category": pd.Series(source_values, dtype=object),
+ "Revenue": range(1, len(source_values) + 1),
+ }
+ ),
+ convert_big_integers=False,
+ )
+ if format_ == "ascii":
+ # ASCII intentionally displays at most ten categories. Keep every
+ # dateutil named-zone edge in this public-format invocation.
+ rows = rows[9:12]
+ expected = [
+ "1704164645123.456",
+ "1704164645123.456",
+ "1730611800000",
+ "1730615400000",
+ "1730611800000",
+ "1730615400000",
+ "-0.0010000000000287557",
+ "1704153600000",
+ "null",
+ "1729989000123.456",
+ "1710052200123.456",
+ "2224774800123.456",
+ "1710052200123.456",
+ "1710052200123.456",
+ "1729989000123.456",
+ "1710052200123.456",
+ ]
+ form_data = {
+ "viz_type": "bullet",
+ "metric": "Revenue",
+ "groupby": ["Category"],
+ }
+ chart = SimpleNamespace(
+ id=122,
+ slice_name="Timestamp categories",
+ viz_type="bullet",
+ datasource_id=1,
+ datasource_type="table",
+ params=utils_json.dumps(form_data),
+ )
+
+ class _Command:
+ def __init__(self, _query_context: Any) -> None: ...
+
+ def validate(self) -> None: ...
+
+ def run(self) -> dict[str, Any]:
+ return {
+ "queries": [
+ {
+ "data": rows,
+ "colnames": ["Category", "Revenue"],
+ }
+ ]
+ }
+
+ query_context = SimpleNamespace(
+ form_data={},
+ queries=[SimpleNamespace(metrics=["Revenue"], columns=["Category"])],
+ )
+ user = MagicMock(id=1, username="admin", roles=[], groups=[])
+ with (
+ patch("superset.mcp_service.auth.get_user_from_request",
return_value=user),
+ patch("superset.mcp_service.auth.check_tool_permission",
return_value=True),
+ patch.object(preview_module, "find_chart_by_identifier",
return_value=chart),
+ patch.object(preview_module.db.session, "refresh", return_value=None),
+ patch.object(
+ preview_module,
+ "validate_chart_dataset",
+ return_value=SimpleNamespace(is_valid=True, warnings=[],
error=None),
+ ),
+ patch.object(
+ preview_module.event_logger,
+ "log_context",
+ side_effect=lambda **_kwargs: nullcontext(),
+ ),
+ patch.object(
+ preview_module,
+ "build_query_context_from_form_data",
+ return_value=query_context,
+ ),
+ patch.object(preview_module, "set_query_context_form_data",
return_value=None),
+ patch.object(command_module, "ChartDataCommand", _Command),
+ patch.object(
+ preview_module, "get_superset_base_url",
return_value="http://localhost";
+ ),
+ ):
+ async with Client(mcp) as client:
+ result = await client.call_tool(
+ "get_chart_preview",
+ {"request": {"id": 122, "format": format_}},
+ )
+
+ payload = utils_json.loads(result.content[0].text)
+ if format_ == "ascii":
+ content = payload["content"]["ascii_content"]
+ for category in set(expected[9:12]):
+ assert category[:20] in content
+ else:
+ specification = payload["content"]["specification"]
+ bar = next(
+ layer for layer in specification["layer"] if layer["mark"]["type"]
== "bar"
+ )
+ category_field = bar["encoding"]["y"]["field"]
+ assert [row[category_field] for row in
specification["data"]["values"]] == (
+ expected
+ )
+ assert bar["encoding"]["tooltip"][0]["field"] == category_field
+ assert [row["Category"] for row in specification["data"]["values"]] ==
[
+ 1704164645123.456,
+ 1704164645123.456,
+ 1730611800000.0,
+ 1730615400000.0,
+ 1730611800000.0,
+ 1730615400000.0,
+ -0.0010000000000287557,
+ 1704153600000.0,
+ None,
+ 1729989000123.456,
+ 1710052200123.456,
+ 2224774800123.456,
+ 1710052200123.456,
+ 1710052200123.456,
+ 1729989000123.456,
+ 1710052200123.456,
+ ]
+
+
[email protected]
+async def test_transitionless_dateutil_dataframe_reaches_fastmcp_preview() ->
None:
+ from contextlib import nullcontext
+
+ from fastmcp import Client
+
+ from superset.commands.chart.data.get_data_command import (
+ ChartDataCommand as ProducerChartDataCommand,
+ )
+ from superset.common.chart_data import ChartDataResultType
+ from superset.dataframe import df_to_records
+ from superset.mcp_service.app import mcp
+ from superset.mcp_service.chart.preview_utils import
_javascript_number_string
+ from superset.utils.json import json_int_dttm_ser
+
+ preview_module = importlib.import_module(
+ "superset.mcp_service.chart.tool.get_chart_preview"
+ )
+ command_module = importlib.import_module(
+ "superset.commands.chart.data.get_data_command"
+ )
+ names = [
+ "UTC",
+ "GMT",
+ "Universal",
+ "Zulu",
+ "EST",
+ "HST",
+ "MST",
+ "Etc/GMT+1",
+ "Etc/GMT-2",
+ ]
+ values = []
+ for getter in (dateutil_tz.gettz, get_zonefile_instance().get):
+ for name in names:
+ timezone_value = getter(name)
+ assert timezone_value is not None
+ values.append(
+ datetime(2040, 7, 1, 12, 34, 56, 123456, tzinfo=timezone_value)
+ )
+ rows = df_to_records(
+ pd.DataFrame(
+ {
+ "Category": pd.Series(values, dtype=object),
+ "Revenue": range(1, len(values) + 1),
+ }
+ ),
+ convert_big_integers=False,
+ )
+
+ class _ProducerContext:
+ result_type = ChartDataResultType.FULL
+
+ def get_payload(self, **_kwargs: Any) -> dict[str, Any]:
+ return {
+ "queries": [
+ {
+ "data": rows,
+ "colnames": ["Category", "Revenue"],
+ "rowcount": len(rows),
+ }
+ ]
+ }
+
+ producer_result = ProducerChartDataCommand(
+ _ProducerContext() # type: ignore[arg-type]
+ ).run()
+ expected_numbers = [json_int_dttm_ser(value) for value in values]
+ expected_categories = [
+ _javascript_number_string(float(value)) for value in expected_numbers
+ ]
+ form_data = {
+ "viz_type": "bullet",
+ "metric": "Revenue",
+ "groupby": ["Category"],
+ }
+ chart = SimpleNamespace(
+ id=123,
+ slice_name="Transitionless timestamps",
+ viz_type="bullet",
+ datasource_id=1,
+ datasource_type="table",
+ params=utils_json.dumps(form_data),
+ )
+
+ class _Command:
+ def __init__(self, _query_context: Any) -> None: ...
+
+ def validate(self) -> None: ...
+
+ def run(self) -> dict[str, Any]:
+ return producer_result
+
+ query_context = SimpleNamespace(
+ form_data={},
+ queries=[SimpleNamespace(metrics=["Revenue"], columns=["Category"])],
+ )
+ user = MagicMock(id=1, username="admin", roles=[], groups=[])
+ with (
+ patch("superset.mcp_service.auth.get_user_from_request",
return_value=user),
+ patch("superset.mcp_service.auth.check_tool_permission",
return_value=True),
+ patch.object(preview_module, "find_chart_by_identifier",
return_value=chart),
+ patch.object(preview_module.db.session, "refresh", return_value=None),
+ patch.object(
+ preview_module,
+ "validate_chart_dataset",
+ return_value=SimpleNamespace(is_valid=True, warnings=[],
error=None),
+ ),
+ patch.object(
+ preview_module.event_logger,
+ "log_context",
+ side_effect=lambda **_kwargs: nullcontext(),
+ ),
+ patch.object(
+ preview_module,
+ "build_query_context_from_form_data",
+ return_value=query_context,
+ ),
+ patch.object(preview_module, "set_query_context_form_data",
return_value=None),
+ patch.object(command_module, "ChartDataCommand", _Command),
+ patch.object(
+ preview_module, "get_superset_base_url",
return_value="http://localhost";
+ ),
+ ):
+ async with Client(mcp) as client:
+ result = await client.call_tool(
+ "get_chart_preview",
+ {"request": {"id": 123, "format": "vega_lite"}},
+ )
+
+ payload = utils_json.loads(result.content[0].text)
+ specification = payload["content"]["specification"]
+ bar = next(
+ layer for layer in specification["layer"] if layer["mark"]["type"] ==
"bar"
+ )
+ category_field = bar["encoding"]["y"]["field"]
+ assert [row["Category"] for row in specification["data"]["values"]] == (
+ expected_numbers
+ )
+ assert [row[category_field] for row in specification["data"]["values"]] ==
(
+ expected_categories
+ )
+ assert bar["encoding"]["tooltip"][0]["field"] == category_field
+
+
[email protected]
[email protected]("format_", ["ascii", "vega_lite"])
+async def test_duration_dataframe_reaches_fastmcp_bullet_preview(
+ format_: str,
+) -> None:
+ from contextlib import nullcontext
+
+ import numpy as np
+ from fastmcp import Client
+
+ from superset.commands.chart.data.get_data_command import (
+ ChartDataCommand as ProducerChartDataCommand,
+ )
+ from superset.common.chart_data import ChartDataResultType
+ from superset.dataframe import df_to_records
+ from superset.mcp_service.app import mcp
+
+ preview_module = importlib.import_module(
+ "superset.mcp_service.chart.tool.get_chart_preview"
+ )
+ command_module = importlib.import_module(
+ "superset.commands.chart.data.get_data_command"
+ )
+ source_values = [
+ timedelta(0),
+ timedelta(days=1, seconds=2, microseconds=3),
+ pd.Timedelta(-1, unit="ns"),
+ np.timedelta64(123456789, "ns"),
+ np.timedelta64("NaT"),
+ ]
+ rows = df_to_records(
+ pd.DataFrame(
+ {
+ "Duration": pd.Series(source_values, dtype=object),
+ "Revenue": [50, 120, 350, 0, 200],
+ }
+ ),
+ convert_big_integers=False,
+ )
+ expected = [
+ "null"
+ if row["Duration"] is None
+ else utils_json.loads(
+ utils_json.dumps(row["Duration"],
default=utils_json.json_int_dttm_ser)
+ )
+ for row in rows
+ ]
Review Comment:
Reproduced; the premise is stale for rows produced by `df_to_records`: exact
NumPy duration becomes exact `pd.Timedelta`, and NumPy NaT becomes `None`.
703bb6cd adds explicit assertions at that boundary in the public FastMCP
preview regression before the serializer-derived expected values. The test
passes locally and in the full MCP suite.
--
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]
