sadpandajoe commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4148800220
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -1441,10 +1420,26 @@ def _write_excel_headers(ws: Any, columns: List[str])
-> None:
ws.cell(row=1, column=idx, value=col)
+def _excel_scalar(value: Any) -> Any:
+ """Project a validated result scalar to values supported by Excel
writers."""
+ if value is None:
+ return ""
+ if type(value) is UUID:
+ return UUID.__str__(value)
+ if type(value) is list or type(value) is dict:
+ return str(value)
+ return value
+
+
def _write_excel_data(ws: Any, data: List[Dict[str, Any]], columns: List[str])
-> None:
"""Write data to Excel worksheet."""
for row_idx, row in enumerate(data, 2):
for col_idx, col in enumerate(columns, 1):
+ ws.cell(
+ row=row_idx,
+ column=col_idx,
+ value=_excel_scalar(row.get(col, "")),
Review Comment:
This new write is immediately overwritten by the pre-existing `ws.cell(...)`
call a few lines below, which recomputes `value` straight from `row.get(col,
"")` without `_excel_scalar`'s UUID/non-finite-float/list/dict handling — so
that handling never actually takes effect. A UUID column in the result makes
`get_chart_data(format="excel")` raise instead of returning a workbook, since
openpyxl can't serialize a raw `UUID`. Should the old write below be removed
now that this one replaces it?
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1373,15 +1406,32 @@ def _bind_dashboard_time_range_filter( # noqa: C901
if _uses_mapper_owned_temporal_binding(form_data, dataset_id):
return
+ explicit_fields = set(getattr(config, "model_fields_set", set()))
+ if _clears_temporal_subject(config, explicit_fields):
Review Comment:
This branch correctly skips generating a new temporal binding when
`temporal_column` is explicitly `null`, but it never clears the existing
`_mcp_dashboard_time_filter_subject` marker either — the key is just absent
from this dict. The overlay merge in `_merge_shared_form_data`
(`{**existing_form_data, **new_form_data}`) then re-inherits the old marker
from the saved chart, so an explicit clear is silently reinterpreted as
omission (or raises once the matching filter has already been dropped
elsewhere). Should this branch explicitly set the marker to `None` so the merge
step can tell "cleared" apart from "omitted"?
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1778,668 @@ 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.
+ """
+ if config.dimensions is None and config.order_by:
+ # An update resolves its saved hierarchy before mapping. Without one,
+ # creation must validate sort targets against an empty hierarchy.
+ BulletChartConfig.model_validate(
+ {**config.model_dump(exclude_unset=True), "dimensions": []}
+ )
+ 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:
Review Comment:
This raises whenever a chart's saved MCP temporal-binding marker
(`_mcp_dashboard_time_filter_subject`) survives an update but its bound filter
doesn't. For example, sending `filters: []` on a chart with an existing
dashboard-time binding clears `adhoc_filters` (the shared merge's
explicit-empty-clears-the-control path) without also clearing this marker, so
this lookup lands here with `found 0`. A plain "remove this chart's filters"
`update_chart`/`update_chart_preview` call now fails outright whenever a
dashboard-time binding exists, instead of applying the clear. Should the marker
be cleared alongside `adhoc_filters` in that case?
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -339,6 +407,532 @@ 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 type(metric) is str:
+ return metric
+ if type(metric) is not dict:
+ return None
+ if label := dict.get(metric, "label"):
+ return label if type(label) is str else None
+ if dict.get(metric, "expressionType") == "SQL":
+ expression = dict.get(metric, "sqlExpression")
+ return expression if type(expression) is str and expression else None
+ column = dict.get(metric, "column")
+ column_name = dict.get(column, "column_name") if type(column) is dict else
column
+ aggregate = dict.get(metric, "aggregate")
+ if type(column_name) is str and type(aggregate) is 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 type(column) is str:
+ return column
+ if type(column) is not dict:
+ return None
+ for key in ("label", "column_name"):
+ if type(value := dict.get(column, key)) is str and value:
+ return value
+ return None
+
+
+def _canonical_result_field(label: str | None, row: Dict[str, Any]) -> str |
None:
+ """Resolve an exact or one unambiguous casefold result-field match."""
+ if label is None:
+ return None
+ if label in dict.keys(row):
+ return label
+ matches = [
+ field
+ for field in dict.keys(row)
+ if type(field) is str and field.casefold() == label.casefold()
+ ]
+ return matches[0] if len(matches) == 1 else None
+
+
+def _require_result_field(label: str | None, row: dict[str, Any], role: str)
-> str:
+ """Resolve a role without falling back to an unrelated result field."""
+ if not label:
+ raise BulletOutputError(f"Bullet {role} has no declared result alias")
+ if label in dict.keys(row):
+ return label
+ matches = sorted(
+ field
+ for field in dict.keys(row)
+ if type(field) is str and field.casefold() == label.casefold()
+ )
+ if len(matches) == 1:
+ return matches[0]
+ if matches:
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is ambiguous; candidates: "
+ f"{', '.join(matches)}"
+ )
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is missing from query output"
+ )
+
+
+def _safe_enum_backing(value: Any) -> Any:
+ """Extract Enum's stored value without public descriptors/conversions."""
+ value_type = type(value)
+ try:
+ mro = type.__getattribute__(value_type, "__mro__")
+ except (AttributeError, TypeError): # pragma: no cover - normal types
have MRO
+ return value
+ if type(mro) is not tuple or not any(base is Enum for base in mro):
+ return value
+ try:
+ backing = object.__getattribute__(value, "_value_")
+ except Exception as ex:
+ raise BulletOutputError("Bullet output contains an unreadable enum")
from ex
+ if not any(type(backing) is allowed for allowed in _ENUM_SCALAR_TYPES):
+ raise BulletOutputError("Bullet output contains an unsupported enum
value")
+ return backing
+
+
+def _decimal_javascript_string(value: Decimal) -> str:
+ """Render an exact binary64 spelling with JavaScript Number thresholds."""
+ sign, digits_tuple, exponent = Decimal.as_tuple(value)
+ if type(exponent) is not int: # finite Decimals always have an integer
exponent
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ if not any(digits_tuple):
+ return "0"
+
+ digits = "".join(str(digit) for digit in digits_tuple)
+ adjusted = len(digits) + exponent - 1
+ prefix = "-" if sign else ""
+ if -6 <= adjusted < 21:
+ point = len(digits) + exponent
+ if point <= 0:
+ text = f"0.{('0' * -point)}{digits}"
+ elif point >= len(digits):
+ text = digits + ("0" * (point - len(digits)))
+ else:
+ text = f"{digits[:point]}.{digits[point:]}"
+ if "." in text:
+ text = text.rstrip("0").rstrip(".")
+ return prefix + text
+
+ fraction = digits[1:].rstrip("0")
+ coefficient = digits[0] + (f".{fraction}" if fraction else "")
+ exponent_text = f"+{adjusted}" if adjusted >= 0 else str(adjusted)
+ return f"{prefix}{coefficient}e{exponent_text}"
+
+
+def _javascript_number_string(value: int | float | Decimal) -> str:
+ """Apply JSON-number -> IEEE-754 Number -> JavaScript String semantics.
+
+ Exact result scalars can retain precision that the frontend cannot: JSON
+ parsing first rounds a numeric token to binary64, and ``String`` then emits
+ the shortest round-tripping decimal with fixed notation for exponents in
+ [-6, 20]. Converting exact builtin scalars to an exact builtin float keeps
+ the path hook-free. Python and JavaScript use the same shortest
+ round-tripping binary64 digits; ``_decimal_javascript_string`` only adjusts
+ the notation thresholds and exponent spelling.
+
+ A finite integer or Decimal outside binary64's range becomes an infinity
+ after JSON parsing, matching JavaScript. Non-finite source values are
+ rejected by the trusted scalar normalizer before this helper is called.
+ """
+ value_type = type(value)
+ if value_type not in {int, float, Decimal}:
+ raise BulletOutputError("Bullet dimension contains an unsupported
number")
+ if value_type is float and not math.isfinite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
number")
+ if isinstance(value, Decimal) and not Decimal.is_finite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ try:
+ number = float(value)
+ except OverflowError:
+ number = -math.inf if value < 0 else math.inf
+
+ if math.isinf(number):
+ return "-Infinity" if number < 0 else "Infinity"
+ if number == 0:
+ # String(-0) is "0" even though JSON.parse preserves negative zero.
+ return "0"
+ return _decimal_javascript_string(Decimal(float.__repr__(number)))
+
+
+def _bullet_category_value( # noqa: C901
+ value: Any, dimension: str, row_index: int
+) -> tuple[Any, str]:
+ """Return a JSON-safe value and bounded frontend ``String(value)`` text.
+
+ The trusted scalar normalizer is type-exact and does not dispatch through
+ application hooks. Vega data retains the normalized Chart Data wire value
+ (including epoch-ms temporal numbers); only the derived category key and
+ ASCII label use the JavaScript-compatible text.
+ """
+ from superset.mcp_service.chart.query_result import (
+ _bounded_utf8_length,
+ _chart_data_duration_text,
+ _chart_data_temporal_number,
+ _is_chart_data_duration_scalar,
+ _is_chart_data_temporal_scalar,
+ _normalize_trusted_scalar,
+ )
+
+ normalized: Any
+ reason: str | None
+ if _is_chart_data_temporal_scalar(value):
+ normalized, reason = _chart_data_temporal_number(value)
+ elif _is_chart_data_duration_scalar(value):
+ normalized, reason = _chart_data_duration_text(value)
+ else:
+ normalized, reason = _normalize_trusted_scalar(
+ value, max_string_bytes=_MAX_BULLET_TEXT_BYTES
+ )
+ if reason is not None:
+ if reason == "contains an unsupported or subclassed value":
+ reason = "has an unsupported value type"
+ elif "oversized string" in reason:
+ reason = "exceeds the size limit"
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} {reason}"
+ )
+
+ value_type = type(normalized)
+ if normalized is None:
+ text = "null"
+ elif value_type is str:
+ text = normalized
+ elif value_type is bool:
+ text = "true" if normalized else "false"
+ elif value_type is int or value_type is float or value_type is Decimal:
+ text = _javascript_number_string(normalized)
+ else:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} has an "
+ "unsupported value type"
+ )
+
+ if _bounded_utf8_length(text, _MAX_BULLET_TEXT_BYTES) is None:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} exceeds the size
limit"
+ )
+ return normalized, text
+
+
+def _bullet_number(value: Any, row_index: int, metric_field: str) -> float:
+ """Apply the frontend's useful ``Number(value ?? 0)`` numeric subset."""
+ value = _safe_enum_backing(value)
+ if value is None:
+ number = 0.0
+ elif type(value) is bool:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} returned a
boolean"
+ )
+ elif type(value) is int or type(value) is float or type(value) is Decimal:
+ try:
+ number = float(value)
+ except (TypeError, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ ) from ex
+ elif type(value) is str:
+ if len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ )
+ stripped = value.strip()
+ if not stripped:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ )
+ try:
+ number = float(Decimal(stripped))
+ except (InvalidOperation, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} returned "
+ f"non-numeric text"
+ ) from ex
+ else:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not numeric"
+ )
+ if not math.isfinite(number):
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is NaN or
infinite"
+ )
+ return number
+
+
+def _bullet_string_tokens(value: Any) -> list[str]:
+ """Parse labels exactly like the frontend's comma tokenizer."""
+ from superset.mcp_service.chart.query_result import _truncate_utf8
+
+ value = _safe_enum_backing(value)
+ if value is None:
+ return []
+ if type(value) is not str or len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError("Bullet labels must be a bounded
comma-separated list")
+ if not value.strip():
+ return []
+ tokens = value.split(",")
+ if len(tokens) > _MAX_BULLET_TOKENS:
+ raise BulletOutputError("Bullet labels exceed the item limit")
+ return [_truncate_utf8(token.strip(), _MAX_BULLET_TEXT_BYTES) for token in
tokens]
+
+
+def _unique_bullet_derived_field(
+ rows: list[dict[str, Any]], base: str, reserved: tuple[str, ...] = ()
+) -> str:
+ """Return one internal key absent from result rows and prior derived
keys."""
+ occupied = {key for row in rows for key in dict.keys(row)}
+ occupied.update(reserved)
+ candidate = base
+ suffix = 0
+ while candidate in occupied:
+ suffix += 1
+ candidate = f"{base}_{suffix}"
+ return candidate
+
+
+def _unique_bullet_category_field(rows: list[dict[str, Any]]) -> str:
+ """Return an internal category key absent from every query-result row."""
+ return _unique_bullet_derived_field(rows, "__mcp_bullet_category")
+
+
+def _validate_bullet_format(format_: Any, values: list[float]) -> str:
+ """Reject a presentation format the backend cannot reproduce."""
+ format_ = _safe_enum_backing(format_)
+ if format_ is None or format_ == "":
+ format_ = "SMART_NUMBER"
+ if type(format_) is not str or len(format_) > 50:
+ raise BulletOutputError(
+ "Bullet number format is unsupported by previews",
+ error_type="UnsupportedFormat",
+ )
+ try:
+ for value in values:
+ _format_bullet_number(format_, value)
+ except (TypeError, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet number format {format_!r} is unsupported by previews",
+ error_type="UnsupportedFormat",
+ ) from ex
+ return format_
+
+
+def _format_bullet_number(format_: str, value: float) -> str:
+ """Format finite Bullet values, including the full binary-float range."""
+ from superset.utils.number_format import format_numeric
+
+ try:
+ return format_numeric(format_, value)
+ except OverflowError:
+ # SMART_NUMBER's significant-digit rounding can overflow a finite float
+ # near DBL_MAX. Scientific repr remains deterministic and informative.
+ if format_ in {"SMART_NUMBER", "SMART_NUMBER_SIGNED"} and
math.isfinite(value):
+ prefix = "+" if format_ == "SMART_NUMBER_SIGNED" and value > 0
else ""
+ return prefix + repr(value)
+ raise
+
+
+def _containing_bullet_range_label(
+ measure: float, ranges: list[float], labels: list[str]
+) -> str | None:
+ """Match the frontend's labelled containing-range tooltip selection."""
+ ascending = sorted(
+ (
+ (value, labels[index] if index < len(labels) else "")
+ for index, value in enumerate(ranges)
+ ),
+ key=lambda entry: entry[0],
+ )
+ for threshold, label in ascending:
+ if measure <= threshold:
+ return label or None
+ if ascending and ascending[-1][1]:
+ return f"> {ascending[-1][1]}"
+ return None
+
+
+def resolve_bullet_render_model( # noqa: C901
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> BulletRenderModel:
+ """Resolve and validate every Bullet row and presentation control."""
+ if type(data) is not list:
+ raise BulletOutputError("Bullet query output must be an array of
objects")
+ for row_index in range(list.__len__(data)):
+ row = list.__getitem__(data, row_index)
+ if type(row) is not dict:
+ raise BulletOutputError("Bullet query output must be an array of
objects")
+ if dict.__len__(row) > _MAX_BULLET_FIELDS:
+ raise BulletOutputError("Bullet query row exceeds the field limit")
+ for key in dict.keys(row):
+ if type(key) is not str:
+ raise BulletOutputError("Bullet query row keys must be
strings")
+ if len(key) > _MAX_BULLET_FIELD_BYTES:
+ raise BulletOutputError("Bullet query row key exceeds the size
limit")
+
+ if type(form_data) is not dict:
+ raise BulletOutputError("Bullet form data must be an object")
+
+ metric_label = _form_metric_label(dict.get(form_data, "metric"))
+ if not metric_label:
+ raise BulletOutputError("Bullet metric has no declared result alias")
+ raw_groupby = dict.get(form_data, "groupby")
+ if raw_groupby is None:
+ raw_groupby = []
+ if type(raw_groupby) is not list:
+ raise BulletOutputError("Bullet dimensions must be an array")
+ dimension_labels = [
+ _form_column_label(list.__getitem__(raw_groupby, index))
+ for index in range(list.__len__(raw_groupby))
+ ]
+ if any(not label for label in dimension_labels):
+ raise BulletOutputError("Bullet dimension has no declared result
alias")
+
+ if data:
+ first_row = list.__getitem__(data, 0)
+ metric_field = _require_result_field(metric_label, first_row, "metric")
+ dimensions = [
+ _require_result_field(label, first_row, "dimension")
+ for label in dimension_labels
+ ]
+ else:
+ # The frontend accepts empty results. Ungrouped charts retain one
+ # zero-valued measure; grouped charts retain the declared roles but no
+ # categories or rows are fabricated.
+ metric_field = metric_label
+ dimensions = [label for label in dimension_labels if label is not None]
+
+ measures: list[float] = []
+ copied_rows: list[dict[str, Any]] = []
+ for index in range(list.__len__(data)):
+ row = list.__getitem__(data, index)
+ row_metric_field = _require_result_field(
+ metric_label, row, f"metric row {index}"
+ )
+ measure = _bullet_number(
+ dict.__getitem__(row, row_metric_field), index, metric_field
+ )
+ # Reserve every exact output key so the internal Vega category alias
+ # cannot collide with an unselected result field. Unselected values are
+ # deliberately replaced with None rather than converted or serialized.
+ copied: dict[str, Any] = dict.fromkeys(dict.keys(row))
+ copied[metric_field] = measure
+ for label, dimension in zip(dimension_labels, dimensions, strict=True):
+ row_dimension = _require_result_field(label, row, f"dimension row
{index}")
+ dimension_value, _ = _bullet_category_value(
+ dict.__getitem__(row, row_dimension), dimension, index
+ )
+ copied[dimension] = dimension_value
+ copied_rows.append(copied)
+ measures.append(measure)
+
+ # The frontend validates/coerces the whole result array but renders only
+ # the first row for an ungrouped aggregate.
+ if not dimensions:
+ copied_rows = copied_rows[:1]
+ measures = measures[:1]
+ if not copied_rows:
+ copied_rows = [{metric_field: 0.0}]
+ measures = [0.0]
+
+ ranges = _strict_bullet_numeric_tokens(dict.get(form_data, "ranges"),
"ranges")
Review Comment:
This raises `BulletOutputError` for any non-numeric token in `ranges`, but
the ECharts frontend already tolerates and silently drops non-numeric tokens
here when rendering a saved chart. A previously-saved Bullet chart with a stray
non-numeric token in `ranges`/`markers`/`marker_lines` (which still renders
fine in Explore) now fails every `get_chart_preview`/`get_chart_data` call
through this path, and the native `List[float]` schema field rejects it on any
unrelated `update_chart` too — the chart becomes permanently unreadable via
MCP. Should this drop non-numeric tokens instead of raising, to match the
frontend?
##########
superset/mcp_service/chart/validation/dataset_validator.py:
##########
@@ -524,6 +543,25 @@ def get_canonical_column_name(
The canonical column name from the dataset, or the original name
if no match is found.
"""
+ names = [col["name"] for col in dataset_context.available_columns]
+ names.extend(metric["name"] for metric in
dataset_context.available_metrics)
Review Comment:
This builds one combined name list from `available_columns` and
`available_metrics` before the case-fold check, so a physical column and a
same-name-different-case saved metric now collide: with physical column
`Revenue` and saved metric `REVENUE`, requesting `revenue` raises
`AmbiguousDatasetReferenceError` instead of resolving to the physical column.
Should physical columns keep precedence over same-cased metrics here, the way
`resolve_dataset_column` orders them?
##########
superset/mcp_service/chart/plugins/bullet.py:
##########
@@ -0,0 +1,444 @@
+# 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 Callable, 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,
+ ChartError,
+ ColumnRef,
+ resolve_bullet_order_target,
+ VegaLitePreview,
+)
+from superset.mcp_service.chart.validation.dataset_validator import (
+ AmbiguousDatasetReferenceError,
+ DatasetValidator,
+ is_numeric_column,
+ resolve_dataset_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:
+ # AmbiguousDatasetReferenceError subclasses ValueError, so existing
+ # ValueError handlers keep working, while the validation pipeline
+ # re-raises it instead of downgrading it to a warning and proceeding
+ # with the unresolved reference.
+ raise AmbiguousDatasetReferenceError(name, matches, f"Bullet {role}")
+ return matches[0] if matches else name
+
+
+def _render_model_error(rows: Any, form_data: Mapping[str, Any]) -> ChartError
| None:
+ """Return why ``rows`` cannot build the strict Bullet render model."""
+ from superset.mcp_service.chart.preview_utils import (
+ BulletOutputError,
+ resolve_bullet_render_model,
+ )
+ from superset.mcp_service.chart.query_result import safe_exception_message
+
+ try:
+ resolve_bullet_render_model(rows, dict(form_data))
+ except BulletOutputError as ex:
+ return ChartError(error=safe_exception_message(ex),
error_type=ex.error_type)
+ return None
+
+
+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",
+ }
+ # Bullet/transformProps.ts converts temporal values with Number().
+ temporal_json_numbers = True
+ # The frontend renders an empty result (a zero measure when ungrouped).
+ allows_empty_result = True
+ allows_empty_data_result = True
+ # Updates merge filter provenance plus Bullet's bounded native controls;
+ # no other saved control is carried into the typed Bullet state.
+ owns_update_merge = True
+ binds_time_range_to_temporal_filter = True
+ table_preview_unsupported_reason: ClassVar[str | None] = (
+ "Table previews cannot represent Bullet ranges, markers, "
+ "labels, and legend semantics"
+ )
+ invalid_result_error_code = "MALFORMED_BULLET_OUTPUT"
+ invalid_result_message = (
+ "Bullet query output does not contain a usable sizing measure."
+ )
+
+ 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( # noqa: C901
+ 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 in dataset
metadata"
+ ),
+ 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
+ try:
+ column = resolve_dataset_column(metric.name or "", dataset_context)
+ except ValueError as ex:
+ return ChartGenerationError(
+ error_type="ambiguous_bullet_reference",
+ message=(
+ f"Bullet metric column {metric.name!r} is ambiguous in "
+ "dataset metadata"
+ ),
+ details=str(ex),
+ suggestions=["Use get_dataset_info and copy the exact-case
field name"],
+ error_code="AMBIGUOUS_BULLET_REFERENCE",
+ )
+ if column is None or is_numeric_column(column):
+ return None
+ return ChartGenerationError(
+ error_type="non_numeric_bullet_metric",
+ message=(
+ f"Bullet metric {metric.name!r} must produce a number; dataset
"
+ f"type is {column.get('type', 'UNKNOWN')}."
+ ),
+ details=(
+ "Bullet/transformProps.ts converts the metric result with
Number(). "
+ "A non-numeric MIN/MAX or default SUM would render an invalid
bar."
+ ),
+ suggestions=[
+ "Use COUNT or COUNT_DISTINCT for a text column",
+ "Choose a numeric dataset column",
+ "Use a saved or SQL metric that returns a numeric value",
+ ],
+ error_code="NON_NUMERIC_BULLET_METRIC",
+ )
+
+ def normalize_column_refs(
+ self, config: Any, dataset_context: DatasetContext
+ ) -> Any:
+ if not isinstance(config, BulletChartConfig):
+ return config
+ explicit_fields = set(config.model_fields_set)
+ config_dict = config.model_dump(exclude_unset=True)
+ columns = [column["name"] for column in
dataset_context.available_columns]
+ metrics = [metric["name"] for metric in
dataset_context.available_metrics]
+
+ metric = config_dict["metric"]
+ if not metric.get("sql_expression"):
+ metric["name"] = _canonical_reference(
+ metric["name"],
+ metrics if metric.get("saved_metric") else columns,
+ "saved metric" if metric.get("saved_metric") else "metric
column",
+ )
+ for dimension in config_dict.get("dimensions") or []:
+ dimension["name"] = _canonical_reference(
+ dimension["name"], columns, "dimension"
+ )
+ if temporal := config_dict.get("temporal_column"):
+ config_dict["temporal_column"] = _canonical_reference(
+ temporal, columns, "temporal column"
+ )
+ for filter_ in config_dict.get("filters") or []:
+ filter_["column"] = _canonical_reference(
+ filter_["column"], columns, "filter column"
+ )
+
+ # Sort targets may use ergonomic role names or labels. Canonicalize
+ # physical-name targets and leave explicit display labels untouched.
+ for order in config_dict.get("order_by") or []:
+ role, index = resolve_bullet_order_target(
+ order["column"], config.dimensions or [], config.metric
+ )
+ if role == "dimension" and index is not None:
+ order["column"] = config_dict["dimensions"][index]["name"]
+ elif not metric.get("sql_expression") and not metric.get("label"):
+ order["column"] = metric["name"]
+
+ normalized = BulletChartConfig.model_validate(config_dict)
+ normalized.model_fields_set.clear()
+ normalized.model_fields_set.update(explicit_fields)
+ return normalized
+
+ def generate_name(self, config: Any, dataset_name: str | None = None) ->
str:
+ metric = config.metric.label or config.metric.name or "Metric"
+ what = f"{metric} bullet"
+ if config.dimensions:
+ what += " by " + ", ".join(
+ dimension.label or dimension.name or "dimension"
+ for dimension in config.dimensions
+ )
+ return self._with_context(what, _summarize_filters(config.filters))
+
+ def resolve_viz_type(self, config: Any) -> str:
+ return "bullet"
+
+ def schema_error_hint(self) -> ChartGenerationError | None:
+ return ChartGenerationError(
+ error_type="bullet_validation_error",
+ message="Bullet chart configuration validation failed",
+ details=(
+ "Bullet requires one numeric metric and optional unique
physical "
+ "dimensions. Threshold/marker labels must align with their
values."
Review Comment:
This hint is shown whenever Bullet schema validation fails for any reason,
but "Threshold/marker labels must align with their values" isn't actually
required — missing/short label arrays fall back to formatted numbers per index
and extra labels are ignored. A caller whose config fails validation for an
unrelated reason can be misled into "fixing" a label-count mismatch that was
never a problem. Should this line be dropped or reworded?
##########
tests/unit_tests/mcp_service/chart/test_bullet_chart.py:
##########
@@ -0,0 +1,3786 @@
+# 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.plugins.bullet import BulletChartPlugin
+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,
+ ChartInfo,
+ 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 (
+ AmbiguousDatasetReferenceError,
+ DatasetValidator,
+)
+from superset.mcp_service.chart.validation.pipeline import ValidationPipeline
+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",
+ ]
+
+
+def test_bullet_dimension_aliases_require_exact_physical_identity() -> None:
+ """Schema-only alias resolution must not merge quoted case variants."""
+ with pytest.raises(ValidationError, match="Conflicting Bullet dimension
aliases"):
+ BulletChartConfig.model_validate(
+ {
+ "metric": _simple_metric(),
+ "dimensions": ["Region"],
+ "groupby": ["region"],
+ }
+ )
+
+
[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
+
+
[email protected](
+ ("native_operator", "typed_operator", "round_trip_operator"),
+ [
+ ("EQUALS", "=", "=="),
+ ("NOT_EQUALS", "!=", "!="),
+ ("LESS_THAN", "<", "<"),
+ ("LESS_THAN_OR_EQUAL", "<=", "<="),
+ ("GREATER_THAN", ">", ">"),
+ ("GREATER_THAN_OR_EQUAL", ">=", ">="),
+ ("IN", "IN", "IN"),
+ ("NOT_IN", "NOT IN", "NOT IN"),
+ ("LIKE", "LIKE", "LIKE"),
+ ("ILIKE", "ILIKE", "ILIKE"),
+ ("IS_NULL", "IS NULL", "IS NULL"),
+ ("IS_NOT_NULL", "IS NOT NULL", "IS NOT NULL"),
+ ],
+)
+def test_bullet_native_filter_operator_names_round_trip(
+ native_operator: str, typed_operator: str, round_trip_operator: str
+) -> None:
+ comparator: object
+ if native_operator.startswith("IS_"):
+ comparator = None
+ elif native_operator in {"IN", "NOT_IN"}:
+ comparator = ["North"]
+ else:
+ comparator = "North"
+ config = BulletChartConfig.model_validate(
+ {
+ "viz_type": "bullet",
+ "metric": "SavedRevenue",
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "expressionType": "SIMPLE",
+ "subject": "Region",
+ "operator": native_operator,
+ "comparator": comparator,
+ }
+ ],
+ }
+ )
+
+ assert config.filters is not None
+ assert config.filters[0].op == typed_operator
+ mapped = map_bullet_config(config)
+ assert mapped["adhoc_filters"][0]["operator"] == round_trip_operator
+
+
+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": "Friendly"}],
+ )
+
+
+def test_bullet_accepts_short_labels_and_rejects_bad_order_target() -> None:
+ config = BulletChartConfig(
+ metric=_simple_metric(), ranges=[1, 2], range_labels=["Only one"]
+ )
+ assert config.range_labels == ["Only one"]
+ with pytest.raises(ValidationError, match="unknown: not_a_role"):
+ BulletChartConfig(
+ metric=_simple_metric(), dimensions=[], 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, gantt, bullet, bubble_v2, 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(
+ AmbiguousDatasetReferenceError, match="Bullet metric column reference"
+ ):
+ 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"),
+ ],
+)
+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:
Review Comment:
This test is named as a frontend/backend query-parity check, but it only
calls the Python query builder and asserts against fields from the same config
object — it never touches the frontend's `buildQuery`. If the frontend's
`buildQuery` stopped forwarding `order_by`/`row_limit`, this test would still
pass, so Explore and an MCP preview could silently query different rows or
ordering for the same saved chart. Could this assert against the frontend
`buildQuery` output (or a shared fixture) instead of just the Python builder's
own config?
--
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]
