aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4189472695
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1758,795 @@ 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._inherited_groupby 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.order_dimensions
+ 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")
+ dimension = dimensions[index]
+ order_target = (
+ dimension.name if isinstance(dimension, ColumnRef) else
dimension
+ )
+ 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 _normalize_bullet_query_aliases(form_data: Mapping[str, Any]) -> Dict[str,
Any]:
+ """Fold inherited native predicates and ordering into canonical
controls."""
+ from superset.mcp_service.chart.chart_helpers import _parse_orderby
+ from superset.utils.core import form_data_to_adhoc, simple_filter_to_adhoc
+
+ normalized = dict(form_data)
+ legacy_filters = [
+ form_data_to_adhoc(normalized, clause)
+ for clause in ("having", "where")
+ if normalized.get(clause)
+ ]
+ legacy_filters.extend(
+ simple_filter_to_adhoc(filter_, "where")
+ for filter_ in normalized.get("filters") or []
+ if filter_ is not None
+ )
+ if legacy_filters:
+ normalized["adhoc_filters"] = [
+ *legacy_filters,
+ *(normalized.get("adhoc_filters") or []),
+ ]
+ for key in ("where", "having", "filters"):
+ normalized.pop(key, None)
+ if "order_by_cols" in normalized:
+ # Native extractQueryFields concatenates both aliases in key order.
+ ordering: list[Any] = []
+ for key, value in normalized.items():
+ if key == "order_by_cols":
+ ordering.extend(_parse_orderby(value))
+ elif key == "orderby":
+ ordering.extend(value or [])
+ normalized["orderby"] = ordering
+ normalized.pop("order_by_cols", None)
+ return normalized
+
+
+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
+ existing_form_data = _normalize_bullet_query_aliases(existing_form_data)
+ 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",
+ "url_params",
+ 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] = ""
+
+ preserve_orderby = (
+ "orderby" not in new_form_data and "orderby" in existing_form_data
+ )
+ 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]
+ if preserve_orderby:
+ new_form_data["orderby"] = _orderby_for_final_output_roles(
+ existing_form_data, new_form_data
+ )
+
+
+def _orderby_for_final_output_roles(
+ existing_form_data: Mapping[str, Any], new_form_data: Mapping[str, Any]
+) -> Any:
+ """Retain final output sorts and rebind inherited metric expressions."""
+ from superset.mcp_service.chart.chart_helpers import _column_label,
_metric_label
+
+ saved = existing_form_data.get("orderby")
+ if not isinstance(saved, list):
+ return saved
+ outputs = {
+ label
+ for column in new_form_data.get("groupby") or []
+ if (label := _column_label(column)) is not None
+ }
+ metrics = new_form_data.get("metrics") or []
+ if not isinstance(metrics, (list, tuple)):
+ metrics = [metrics]
+ metric_outputs = {
+ label: metric
+ for metric in [new_form_data.get("metric"), *metrics]
+ if (label := _metric_label(metric)) is not None
+ }
+ outputs.update(metric_outputs)
+ retained = []
+ for entry in saved:
+ if isinstance(entry, (list, tuple)) and entry:
+ target = entry[0]
+ label = (
+ _metric_label(target)
+ or _column_label(target)
+ or target.get("metric_name")
+ if isinstance(target, Mapping)
+ else target
+ )
+ if isinstance(label, str) and label not in outputs:
Review Comment:
Agreed: native ordering can rank by an undisplayed saved metric
(metrics_by_name in get_sqla_query). Fixed in edbce563ed
(https://github.com/apache/superset/pull/43770/commits/edbce563ed):
_orderby_for_final_output_roles only drops a sort whose target was a previous
Bullet output role that the final state no longer has; independent sorters such
as ["profit", false] are kept. Existing tests for removed dimensions/metrics
still pass. Regression test:
test_saved_bullet_labels_update_keeps_independent_ranking_metric (row_limit 1).
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -154,6 +168,17 @@ def _compute_effective_force(request: GetChartDataRequest)
-> bool:
return request.force_refresh or not request.use_cache
+def _download_form_data(form_data: dict[str, Any], request: Any) -> dict[str,
Any]:
+ """Return form_data without UI page sizing for CSV/Excel downloads.
+
+ Server-paginated tables size a query to the UI page; a download keeps the
+ caller's limit instead of shrinking to that page.
+ """
+ if request.format in {"csv", "excel"} and
form_data.get("server_pagination"):
Review Comment:
Agreed, the bypass was unguarded. Added in edbce563ed
(https://github.com/apache/superset/pull/43770/commits/edbce563ed):
test_server_paginated_table_export_queries_full_limit runs get_chart_data
through the real build_query_dicts_from_form_data on the saved-fallback,
saved+form_data_key and form_data_key-only paths for CSV and Excel, with a
command that returns as many rows as the main query LIMIT. It asserts the main
query row_limit is 1000 and that 1000 (> 25) rows are exported; with
_download_form_data reduced to a no-op all six cases fail.
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -339,6 +414,610 @@ 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 type(value) in {list, tuple, dict}:
+ from superset.mcp_service.chart.query_result import query_result_data
+
+ data, failure = query_result_data(
+ {"queries": [{"data": [{"value": value}]}]},
temporal_json_numbers=True
+ )
+ if failure is not None or data is None:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} "
+ "has an invalid or unbounded container value"
+ )
+ normalized = data[0][0]["value"]
+ reason = None
+ elif _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 value_type in {list, dict}:
+ text = _bullet_container_category_text(normalized, dimension,
row_index)
+ elif 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_container_category_text(value: Any, dimension: str, row_index:
int) -> str:
+ """Stringify validated containers like JavaScript with a bounded text
budget."""
+ from superset.mcp_service.chart.query_result import _bounded_utf8_length
+
+ if type(value) is dict:
+ return "[object Object]"
+ parts: list[str] = []
+ size = 0
+ for index, item in enumerate(value):
+ if type(item) in {list, dict}:
+ text = _bullet_container_category_text(item, dimension, row_index)
+ else:
+ text = (
+ ""
+ if item is None
+ else _bullet_category_value(item, dimension, row_index)[1]
+ )
+ text_size = _bounded_utf8_length(text, _MAX_BULLET_TEXT_BYTES)
+ size += (text_size if text_size is not None else
_MAX_BULLET_TEXT_BYTES + 1) + (
+ index > 0
+ )
+ if size > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} exceeds the
size limit"
+ )
+ parts.append(text)
+ return ",".join(parts)
+
+
+def _javascript_numeric_string(value: str) -> float:
+ """Parse a nonempty trimmed string using JavaScript Number's grammar."""
+ if re.fullmatch(r"0[xX][0-9a-fA-F]+|0[bB][01]+|0[oO][0-7]+", value):
+ return float(int(value, 0))
+ if re.fullmatch(
+
r"[+-]?(?:Infinity|(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?)",
+ value,
+ ):
+ return float(value)
+ raise ValueError("Invalid JavaScript number spelling")
+
+
+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(_JAVASCRIPT_WHITESPACE)
+ if not stripped:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ )
+ try:
+ number = _javascript_numeric_string(stripped)
+ except (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",
+ )
+ from superset.utils.number_format import D3_FORMAT_RE
+
+ # Specifier length does not bound its requested output precision. Check the
+ # parsed precision before the formatter can allocate or round any value.
+ match = D3_FORMAT_RE.match(format_)
+ if match and match.group(8) is not None and int(match.group(8)) > 20:
+ raise BulletOutputError(
+ "Bullet number format precision must not exceed 20",
+ 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:
+ # Binary64 has at most 309 integer digits. Leave room for the bounded
+ # fractional precision, percent scaling, and a rounding carry without
+ # changing the caller's Decimal context.
+ with localcontext() as context:
+ context.prec = max(context.prec, 334)
+ 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],
+ *,
+ validate_format: bool = True,
+) -> BulletRenderModel:
+ """Resolve Bullet rows; check preview formatter support only when
rendering."""
+ 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:
Review Comment:
Agreed: Bullet/transformProps.ts reads
ensureIsArray(groupby).map(getColumnLabel), so a scalar groupby is a one-level
hierarchy and an unlabeled SQL dimension is keyed by its sqlExpression. Fixed
in edbce563ed
(https://github.com/apache/superset/pull/43770/commits/edbce563ed):
resolve_bullet_render_model wraps a scalar/object groupby and
_form_column_label falls back to sqlExpression (shared by data reads, exports
and both previews); the update path normalizes the saved hierarchy the same way
(bullet_groupby_list) so a sort on a scalar "Region" resolves instead of
iterating characters. Regression tests:
test_bullet_saved_groupby_follows_native_ensure_is_array_and_label and
test_saved_bullet_scalar_groupby_resolves_sort_update.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1758,795 @@ 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._inherited_groupby 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.order_dimensions
+ 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")
+ dimension = dimensions[index]
+ order_target = (
+ dimension.name if isinstance(dimension, ColumnRef) else
dimension
+ )
+ 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 _normalize_bullet_query_aliases(form_data: Mapping[str, Any]) -> Dict[str,
Any]:
+ """Fold inherited native predicates and ordering into canonical
controls."""
+ from superset.mcp_service.chart.chart_helpers import _parse_orderby
+ from superset.utils.core import form_data_to_adhoc, simple_filter_to_adhoc
+
+ normalized = dict(form_data)
+ legacy_filters = [
+ form_data_to_adhoc(normalized, clause)
+ for clause in ("having", "where")
+ if normalized.get(clause)
+ ]
+ legacy_filters.extend(
+ simple_filter_to_adhoc(filter_, "where")
+ for filter_ in normalized.get("filters") or []
+ if filter_ is not None
+ )
+ if legacy_filters:
+ normalized["adhoc_filters"] = [
+ *legacy_filters,
+ *(normalized.get("adhoc_filters") or []),
+ ]
+ for key in ("where", "having", "filters"):
+ normalized.pop(key, None)
+ if "order_by_cols" in normalized:
+ # Native extractQueryFields concatenates both aliases in key order.
+ ordering: list[Any] = []
+ for key, value in normalized.items():
+ if key == "order_by_cols":
+ ordering.extend(_parse_orderby(value))
+ elif key == "orderby":
+ ordering.extend(value or [])
+ normalized["orderby"] = ordering
+ normalized.pop("order_by_cols", None)
+ return normalized
+
+
+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
+ existing_form_data = _normalize_bullet_query_aliases(existing_form_data)
+ preserved_keys = {
Review Comment:
Agreed: other plugins keep these through merge_same_viz_form_data, but the
Bullet allowlist dropped them. Fixed in edbce563ed
(https://github.com/apache/superset/pull/43770/commits/edbce563ed):
merge_bullet_form_data preserves extra_form_data and extra_filters, and
validate_merged_bullet_form_data excludes them from the typed validation copy
(as it does url_params) while the compiled/persisted state keeps them.
Regression tests: test_bullet_labels_update_preserves_native_query_context
(preview merge + saved preview) and
test_saved_bullet_labels_update_persists_native_query_context.
--
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]
