aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4183053647
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1758,770 @@ 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 _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:
+ """Drop saved sorts that no longer target a final exact query output."""
+ saved = existing_form_data.get("orderby")
+ if not isinstance(saved, list):
+ return saved
+ outputs = {
+ name for name in new_form_data.get("groupby") or [] if
isinstance(name, str)
+ }
+ metrics = new_form_data.get("metrics") or []
+ if not isinstance(metrics, (list, tuple)):
+ metrics = [metrics]
+ for metric in [new_form_data.get("metric"), *metrics]:
+ label = metric.get("label") if isinstance(metric, Mapping) else metric
+ if isinstance(label, str):
+ outputs.add(label)
+ retained = []
+ for entry in saved:
+ if isinstance(entry, (list, tuple)) and entry:
+ target = entry[0]
+ if isinstance(target, Mapping):
+ target = target.get("label") or target.get("metric_name")
+ if isinstance(target, str) and target not in outputs:
+ continue
+ retained.append(entry)
+ return retained
+
+
+def _filter_identity(filter_: Any) -> tuple[Any, ...] | None:
+ """Return the native identity used when one filter replaces another."""
+ if not isinstance(filter_, Mapping):
+ return None
+ return (
+ filter_.get("clause"),
+ filter_.get("expressionType"),
+ filter_.get("subject"),
+ filter_.get("operator"),
+ )
+
+
+def _temporal_binding_filter(filters: list[Any], subject: Any) -> dict[str,
Any] | None:
+ """Find the unique filter owned by a recorded MCP temporal marker."""
+ if subject is None:
+ return None
+ if not isinstance(subject, str) or not subject:
+ raise ValueError(
+ "MCP temporal binding provenance subject must be a non-empty
string"
+ )
+ matches = [
+ filter_
+ for filter_ in filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == subject
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ ]
+ if len(matches) != 1:
+ raise ValueError(
+ "MCP temporal binding provenance must match exactly one "
+ f"TEMPORAL_RANGE filter for subject {subject!r}; found
{len(matches)}"
+ )
+ return matches[0]
+
+
+def _append_or_replace_filter(filters: list[Any], filter_: Any) -> None:
+ """Append a filter, replacing the same native role when identifiable."""
+ identity = _filter_identity(filter_)
+ if identity is None:
+ if filter_ not in filters:
+ filters.append(filter_)
+ return
+ filters[:] = [item for item in filters if _filter_identity(item) !=
identity]
+ filters.append(filter_)
+
+
+_NATIVE_TEMPORAL_ROLE_FIELDS: dict[str, frozenset[str]] = {
+ # Typed ``x`` is persisted as native x_axis/granularity_sqla for XY and
+ # Mixed Timeseries. Waterfall exposes the typed field as ``x_axis``.
+ "x_axis": frozenset({"x", "x_axis"}),
+ "granularity_sqla": frozenset({"x", "x_axis", "temporal_column"}),
+ # Chart plugins may designate a chart-specific query role as the implicit
+ # dashboard-time subject.
+ "start_time": frozenset({"start_time"}),
+}
+
+
+def _native_temporal_subject_changed(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ explicit_fields: set[str],
+) -> bool:
+ """Return whether an authoritative native temporal role was replaced.
+
+ Mapping a partial update can propose a dataset fallback binding even when
+ the caller only changed filters. That proposal is not authoritative. A
+ changed x/granularity/chart-specific role is authoritative only when its
+ corresponding typed field was actually supplied.
+ """
+ for native_key, typed_fields in _NATIVE_TEMPORAL_ROLE_FIELDS.items():
+ if explicit_fields.isdisjoint(typed_fields):
+ continue
+ existing_value = existing_form_data.get(native_key)
+ incoming_value = new_form_data.get(native_key)
+ if existing_value != incoming_value:
+ return True
+ return False
+
+
+def _native_temporal_binding(
+ form_data: Mapping[str, Any], filters: list[Any]
+) -> tuple[str | None, dict[str, Any] | None]:
+ """Resolve one binding for a trusted native temporal role, if present."""
+ for native_key in _NATIVE_TEMPORAL_ROLE_FIELDS:
+ subject = form_data.get(native_key)
+ if not isinstance(subject, str) or not subject:
+ continue
+ matches = [
+ filter_
+ for filter_ in filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == subject
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ ]
+ if len(matches) > 1:
+ raise ValueError(
+ "An authoritative native temporal subject must match at most
one "
+ f"TEMPORAL_RANGE filter for subject {subject!r}; found "
+ f"{len(matches)}"
+ )
+ if matches:
+ return subject, matches[0]
+ return None, None
+
+
+def merge_update_form_data( # noqa: C901
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Apply the shared omission/provenance contract for chart updates.
+
+ Mapper-generated neutral temporal bindings are infrastructure, not evidence
+ that the caller supplied ``filters`` or changed a saved time-range binding.
+ This helper is used by immediate saves, preview-first saved updates, and
+ cached-preview updates so omission, clear, replacement, and temporal
+ overrides have identical behavior.
+
+ State never crosses a visualization boundary: a viz-type change starts from
+ the mapper's output, so the previous chart's predicates are not restored.
+ """
+ existing_viz_type = existing_form_data.get("viz_type")
+ if isinstance(existing_viz_type, str) and existing_viz_type !=
new_form_data.get(
+ "viz_type"
+ ):
+ return
+ existing_filters = list(existing_form_data.get("adhoc_filters") or [])
+ incoming_filters = list(new_form_data.get("adhoc_filters") or [])
+ existing_subject =
existing_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ incoming_subject = new_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ existing_binding = _temporal_binding_filter(existing_filters,
existing_subject)
+ incoming_binding = _temporal_binding_filter(incoming_filters,
incoming_subject)
+
+ explicit_fields = set(getattr(config, "model_fields_set", set()))
+ filters_explicit = "filters" in explicit_fields
+ range_explicit = "time_range" in explicit_fields
+ subject_explicit = "temporal_column" in explicit_fields
+ native_subject_changed = _native_temporal_subject_changed(
+ existing_form_data, new_form_data, explicit_fields
+ )
+ subject_authoritative = subject_explicit or native_subject_changed
+ if incoming_binding is None:
+ native_subject, native_binding = _native_temporal_binding(
+ new_form_data, incoming_filters
+ )
+ if native_binding is not None:
+ incoming_subject = native_subject
+ incoming_binding = native_binding
+ incoming_user_filters = [
+ filter_ for filter_ in incoming_filters if filter_ is not
incoming_binding
+ ]
+ temporal_explicit = range_explicit or subject_authoritative
+ if (
+ existing_binding is None
+ and incoming_binding is not None
+ and isinstance(incoming_subject, str)
+ and "filters" not in explicit_fields
+ and temporal_explicit
+ ):
+ # A saved temporal filter that Explore wrote has no MCP provenance
+ # marker. When it is the only native filter for the incoming subject,
+ # the update replaces it in place instead of appending a duplicate.
+ native_matches = [
+ filter_
+ for filter_ in existing_filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == incoming_subject
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ ]
+ if len(native_matches) == 1:
+ existing_binding = native_matches[0]
+ existing_subject = incoming_subject
+
+ chosen_binding: dict[str, Any] | None = None
+ chosen_subject: Any = None
+ if not filters_explicit:
+ # Omission is byte-faithful: keep the native sequence in its exact
order,
+ # including SQL/HAVING objects and a provenance-owned binding at any
index.
+ merged_filters = list(existing_filters)
+ chosen_binding = existing_binding
+ chosen_subject = existing_subject
+ if temporal_explicit:
+ if subject_authoritative:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+ elif existing_binding is not None:
+ # A range-only update belongs to the saved subject, even when
+ # mapping the partial config proposed the dataset main_dttm.
+ chosen_binding = dict(existing_binding)
+ chosen_subject = existing_subject
+ else:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+
+ if chosen_binding is not None:
+ chosen_binding = dict(chosen_binding)
+ if range_explicit:
+ chosen_binding["comparator"] = (
+ getattr(config, "time_range", None) or NO_TIME_RANGE
+ )
+ elif existing_binding is not None:
+ # Subject-only replacement preserves the saved active or
+ # neutral range instead of resetting it to No filter.
+ chosen_binding["comparator"] = existing_binding.get(
+ "comparator", NO_TIME_RANGE
+ )
+ if existing_binding is not None:
+ binding_index = next(
+ index
+ for index, filter_ in enumerate(merged_filters)
+ if filter_ is existing_binding
+ )
+ if chosen_binding is None:
+ merged_filters.pop(binding_index)
+ else:
+ # A temporal override changes infrastructure in place
instead
+ # of moving it past surrounding native filters.
+ merged_filters[binding_index] = chosen_binding
+ elif chosen_binding is not None:
+ merged_filters.append(chosen_binding)
+ else:
+ # An explicit filter array replaces the saved native sequence. The
mapper
+ # deliberately emits [] for an explicit clear; otherwise retain its
+ # generated temporal binding after the replacement filters.
+ merged_filters = list(incoming_user_filters)
+ if incoming_user_filters or temporal_explicit:
+ if subject_authoritative:
+ chosen_binding = incoming_binding
+ chosen_subject = incoming_subject
+ elif range_explicit and existing_binding is not None:
+ chosen_binding = dict(existing_binding)
+ chosen_subject = existing_subject
+ else:
+ # A filter-only replacement keeps the saved provenance binding.
+ # The mapper's incoming binding may merely be a dataset
fallback
+ # and must not reset the saved subject or active range.
+ chosen_binding = existing_binding
+ chosen_subject = existing_subject
+ if chosen_binding is not None:
+ chosen_binding = dict(chosen_binding)
+ if range_explicit:
+ chosen_binding["comparator"] = (
+ getattr(config, "time_range", None) or NO_TIME_RANGE
+ )
+ elif subject_authoritative and existing_binding is not None:
+ chosen_binding["comparator"] = existing_binding.get(
+ "comparator", NO_TIME_RANGE
+ )
+ if chosen_binding is not None:
+ _append_or_replace_filter(merged_filters, chosen_binding)
+
+ # Materialize exactly when saved state had the key or the caller made the
+ # controls authoritative. An omitted update must not turn a missing native
+ # filter key into [] merely because its mapper proposed a neutral binding.
+ if filters_explicit or "adhoc_filters" in existing_form_data or
temporal_explicit:
+ new_form_data["adhoc_filters"] = merged_filters
+ else:
+ new_form_data.pop("adhoc_filters", None)
+ if chosen_binding is not None and isinstance(chosen_subject, str):
+ new_form_data[MCP_DASHBOARD_TIME_FILTER_SUBJECT] = chosen_subject
+ else:
+ new_form_data.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+
+def _currency_form_value(value: CurrencyFormat | None) -> dict[str, str] |
None:
+ """Return the native value for an explicitly supplied currency control."""
+ return value.to_form_data() if value is not None else None
+
+
+def _column_names(value: Sequence[ColumnRef] | None) -> list[str | None] |
None:
+ """Return a native column-name list while retaining an explicit null."""
+ return [column.name for column in value] if value is not None else None
+
+
+def _table_sort_value(value: Sequence[str | SortByConfig] | None) -> list[str]
| None:
+ """Return the native Table sort control for an explicit typed value."""
+ if value is None:
+ return None
+ return [
+ json.dumps(
+ [entry.column, entry.ascending]
+ if isinstance(entry, SortByConfig)
+ else [entry, False]
+ )
+ for entry in value
+ ]
+
+
+def _table_column_config_value(value: Any) -> dict[str, Any] | None:
+ """Return Table column config without losing an explicit null or empty
map."""
+ if value is None:
+ return None
+ return {
+ label: column.model_dump(by_alias=True, exclude_unset=True)
+ for label, column in value.items()
+ }
+
+
+# Mappers intentionally omit optional controls so fresh charts use the frontend
+# defaults. During a same-viz update, however, an explicitly supplied false,
+# null, or empty value must block preservation of the saved native key. Keep
the
+# typed-to-native relationship declarative so every update path shares it.
+_FormValueConverter = Callable[[Any], Any]
+_FormControlMap = dict[str, tuple[str, _FormValueConverter]]
+
+_COMMON_EXPLICIT_FORM_CONTROLS: _FormControlMap = {
+ "color_scheme": ("color_scheme", lambda value: value),
+ "currency_format": ("currency_format", _currency_form_value),
+ "show_value": ("show_value", lambda value: value),
+}
+
+_CHART_EXPLICIT_FORM_CONTROLS: dict[str, _FormControlMap] = {
+ "table": {
+ "sort_by": ("order_by_cols", _table_sort_value),
+ "column_config": ("column_config", _table_column_config_value),
+ },
+ "xy": {
+ "group_by": ("groupby", _column_names),
+ "series_limit": ("series_limit", lambda value: value),
+ "stacked": ("stack", lambda value: "Stack" if value else None),
+ "orientation": ("orientation", lambda value: value),
+ "legend_orientation": ("legendOrientation", lambda value: value),
+ "x_axis_time_format": ("x_axis_time_format", lambda value: value),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "mixed_timeseries": {
+ "group_by": ("groupby", _column_names),
+ "group_by_secondary": ("groupby_b", _column_names),
+ "currency_format_secondary": (
+ "currency_format_secondary",
+ _currency_form_value,
+ ),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "waterfall": {
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ },
+ "big_number": {
+ "subheader": ("subheader", lambda value: value),
+ "y_axis_format": ("y_axis_format", lambda value: value),
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ "compare_lag": ("compare_lag", lambda value: value),
+ "time_format": ("time_format", lambda value: value),
+ "aggregation": ("aggregation", lambda value: value),
+ },
+ "handlebars": {
+ "style_template": ("styleTemplate", lambda value: value),
+ "columns": ("all_columns", _column_names),
+ "groupby": ("groupby", _column_names),
+ "metrics": ("metrics", _column_names),
+ },
+ "pivot_table": {
+ "date_format": ("date_format", lambda value: value),
+ },
+ "interactive_pivot": {
+ "time_grain": ("time_grain_sqla", lambda value: value),
+ "series_limit": ("series_limit", lambda value: value),
+ "date_format": ("date_format", lambda value: value),
+ "column_sort": ("colOrder", lambda value: value),
+ },
+}
+
+
+def _apply_explicit_form_controls( # noqa: C901
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Apply typed controls whose mapper omission represents a native clear."""
+ explicit_fields = set(getattr(config, "model_fields_set", set()))
+ controls = {
+ **_COMMON_EXPLICIT_FORM_CONTROLS,
+ **_CHART_EXPLICIT_FORM_CONTROLS.get(config.chart_type, {}),
+ }
+ for field_name, (native_key, convert) in controls.items():
+ if field_name in explicit_fields:
+ converted = convert(getattr(config, field_name))
+ is_clear = (
+ converted is None
+ or converted is False
+ or converted == ""
+ or converted in ([], {})
+ )
+ if is_clear:
+ new_form_data[native_key] = converted
+
+ axis_controls = {
+ "xy": (
+ ("x_axis", "x_axis_title", "x_axis_format", None),
+ ("y_axis", "y_axis_title", "y_axis_format", "logAxis"),
+ ),
+ "mixed_timeseries": (
+ ("x_axis", "xAxisTitle", "x_axis_time_format", None),
+ ("y_axis", "yAxisTitle", "y_axis_format", "logAxis"),
+ (
+ "y_axis_secondary",
+ "yAxisTitleSecondary",
+ "y_axis_format_secondary",
+ "logAxisSecondary",
+ ),
+ ),
+ }
+ if config.chart_type in axis_controls:
+ for field_name, title_key, format_key, scale_key in axis_controls[
+ config.chart_type
+ ]:
+ if field_name not in explicit_fields:
+ continue
+ axis = getattr(config, field_name)
+ if axis is None:
+ new_form_data[title_key] = None
+ new_form_data[format_key] = None
+ if scale_key:
+ new_form_data[scale_key] = None
+ continue
+ axis_fields = set(axis.model_fields_set)
+ if "title" in axis_fields:
+ new_form_data[title_key] = axis.title
+ if "format" in axis_fields:
+ new_form_data[format_key] = axis.format
+ if scale_key and "scale" in axis_fields:
+ new_form_data[scale_key] = (
+ None if axis.scale is None else axis.scale == "log"
+ )
+
+ if config.chart_type == "xy" and "legend" in explicit_fields:
+ legend = config.legend
+ if legend is None:
+ new_form_data["show_legend"] = None
+ new_form_data["legendOrientation"] = None
+ else:
+ legend_fields = set(legend.model_fields_set)
+ if "show" in legend_fields:
+ new_form_data["show_legend"] = legend.show
+ if "position" in legend_fields:
+ new_form_data["legendOrientation"] = legend.position
+
+ if config.chart_type == "interactive_pivot":
+ if "temporal_column" in explicit_fields and config.temporal_column is
None:
+ new_form_data["granularity_sqla"] = None
+ new_form_data["temporal_columns_lookup"] = None
+ if (
+ "series_limit_metric" in explicit_fields
+ and config.series_limit_metric is None
+ ):
+ new_form_data["series_limit_metric"] = None
+ if "comparison_period" in explicit_fields and config.comparison_period
is None:
+ new_form_data["time_compare"] = None
+ if "comparison_type" in explicit_fields and config.comparison_type is
None:
+ new_form_data["comparison_type"] = None
+
+ # A Waterfall axis replacement cannot inherit a bucket belonging to the old
+ # temporal subject. Grain omission preserves only while the axis is stable;
+ # explicit null is already handled by the declarative control map above.
+ if (
+ config.chart_type == "waterfall"
+ and existing_form_data.get("x_axis") != new_form_data.get("x_axis")
+ and "time_grain" not in explicit_fields
+ ):
+ new_form_data["time_grain_sqla"] = None
+
+
+def merge_same_viz_form_data(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: ChartConfig,
+) -> None:
+ """Preserve saved controls that the typed mapper does not represent.
+
+ The typed MCP surface deliberately models a bounded subset of every Explore
+ control panel. For a replacement within the exact same native ``viz_type``,
+ keys absent from the mapper therefore represent omitted controls and retain
+ their saved values. Mapper output and the chart-specific merge helpers run
+ first and remain authoritative, including explicit empty, false, null, and
+ nested values.
+
+ No generic state crosses a visualization boundary. This prevents query-role
+ keys from the previous plugin (for example ``metric`` or ``groupby``) from
+ leaking into a different plugin whose role contract is unrelated.
+ """
+ existing_viz_type = existing_form_data.get("viz_type")
+ if not isinstance(existing_viz_type, str) or existing_viz_type !=
new_form_data.get(
+ "viz_type"
+ ):
+ return
+
+ _apply_explicit_form_controls(existing_form_data, new_form_data, config)
+
+ for key, value in existing_form_data.items():
+ if key == MCP_DASHBOARD_TIME_FILTER_SUBJECT:
+ # merge_update_form_data owns this provenance marker. Its absence
+ # may be an intentional subject clear and must not be undone by the
+ # generic preservation layer.
+ continue
+ if key not in new_form_data:
+ new_form_data[key] = value
+
+
+def validate_merged_bullet_form_data(
+ form_data: Mapping[str, Any],
+ update_config: ChartConfig | None = None,
+) -> BulletChartConfig | None:
+ """Validate final Bullet state without reclassifying preserved filters.
+
+ The typed Bullet surface intentionally creates only SIMPLE WHERE filters,
+ while saved Explore state may legitimately contain SQL WHERE or SIMPLE
+ HAVING filters. When an update omitted ``filters``, those native objects
+ came from the saved state and are validated by the form-data/query layer;
+ removing them only from this schema-validation copy avoids pretending they
+ were newly supplied typed filters. Explicit filter replacements, including
+ ``[]``, still take the strict native-to-typed path.
+ """
+ if form_data.get("viz_type") != "bullet":
+ return None
+ validation_data = dict(form_data)
+ preserves_native_filters = update_config is None or (
+ isinstance(update_config, BulletChartConfig)
+ and "filters" not in update_config.model_fields_set
+ )
+ if preserves_native_filters:
+ validation_data.pop("adhoc_filters", None)
+ validation_data.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+ return BulletChartConfig.model_validate(validation_data)
Review Comment:
Fixed in `74cad83129d2fde30b08dac585523da0087198bb`: `url_params` is
excluded only from typed validation and retained for compilation and
persistence.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2713,6 +2820,464 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class BulletChartConfig(BaseChartConfig):
+ """Config for bullet charts (viz_type ``bullet``)."""
+
+ # Semantic field names are exposed to MCP clients; validation aliases and
the
+ # native adapter accept saved Explore ``form_data`` without weakening the
+ # unknown-field checks that catch misspelled controls.
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["bullet"] = "bullet"
+ metric: ColumnRef = Field(
+ ...,
+ description="Numeric measure shown by each bullet bar",
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ validation_alias=AliasChoices("dimensions", "groupby"),
+ description=(
+ "Category hierarchy; one bullet row per unique combination. Omit
to "
+ "keep the saved hierarchy on update; [] clears it."
+ ),
+ max_length=20,
+ )
+ filters: List[FilterConfig] | None = Field(None, max_length=100)
+ time_range: str | None = Field(
+ None,
+ min_length=1,
+ max_length=1000,
+ description=(
+ "Superset time range, e.g. 'Last 30 days' or '2025-01-01 :
2025-12-31'"
+ ),
+ )
+ row_limit: int = Field(
+ 10000,
+ ge=1,
+ le=50000,
+ description="Maximum bullet rows",
+ )
+ order_by: List[SortByConfig] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("order_by", "orderby", "order_by_cols"),
+ max_length=20,
+ description="Row order by a dimension name or the metric output
label/name",
+ )
+
+ # Presentation fields map one-for-one onto Bullet/transformProps.ts
controls.
+ ranges: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Qualitative range thresholds shaded behind the measure",
+ )
+ range_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("range_labels", "rangeLabels"),
+ max_length=100,
+ )
+ markers: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Target values drawn as point markers",
+ )
+ marker_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_labels", "markerLabels"),
+ max_length=100,
+ )
+ marker_lines: List[float] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_lines", "markerLines"),
+ max_length=100,
+ description="Reference values drawn as vertical lines",
+ )
+ marker_line_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_line_labels",
"markerLineLabels"),
+ max_length=100,
+ )
+ y_axis_format: str = Field(
+ "SMART_NUMBER",
+ validation_alias=AliasChoices("y_axis_format", "yAxisFormat"),
+ max_length=100,
+ )
+ show_labels: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_labels", "showLabels"),
+ )
+ show_legend: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_legend", "showLegend"),
+ )
+
+ @staticmethod
+ def _adapt_native_metric(value: Any) -> Any:
+ """Translate QueryFormMetric shapes into the shared ColumnRef
contract."""
+ if isinstance(value, str):
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict):
+ return value
+ if "expressionType" not in value:
+ # QueryObject's documented legacy saved-metric representation is a
+ # label-only object. Keep this adapter deliberately narrow: objects
+ # carrying ad-hoc fields must declare expressionType explicitly,
and
+ # semantic ColumnRef objects continue through normal validation.
+ if set(value) == {"label"}:
+ label = value["label"]
+ if not isinstance(label, str) or not label or len(label) > 255:
+ raise ValueError(
+ "legacy saved metric label must be a non-empty string
of "
+ "at most 255 characters"
+ )
+ return {"name": label, "saved_metric": True}
+ return value
+ expression_type = value.get("expressionType")
+ if expression_type == "SQL":
+ return {
+ "sql_expression": value.get("sqlExpression"),
+ "label": value.get("label") or value.get("sqlExpression"),
+ }
+ if expression_type != "SIMPLE":
+ raise ValueError("metric.expressionType must be 'SIMPLE' or 'SQL'")
+ column = value.get("column")
+ if isinstance(column, dict):
+ name = column.get("column_name")
+ else:
+ name = column
+ return {
+ "name": name,
+ "aggregate": value.get("aggregate"),
+ "label": value.get("label"),
+ }
+
+ @staticmethod
+ def _canonical_dimension_alias(value: Any, field_name: str) -> list[str]:
+ """Canonicalize semantic/native dimension aliases for conflict
checks."""
+ if not isinstance(value, list):
+ raise ValueError(f"{field_name} must be an array")
+ canonical: list[str] = []
+ for index, item in enumerate(value):
+ name: str | None
+ if isinstance(item, str):
+ name = item
+ elif isinstance(item, ColumnRef):
+ name = item.name
+ elif isinstance(item, dict):
+ name = next(
+ (
+ item[key]
+ for key in ("name", "column_name", "column")
+ if isinstance(item.get(key), str)
+ ),
+ None,
+ )
+ else:
+ name = None
+ if not name:
+ raise ValueError(
+ f"{field_name}[{index}] must identify a physical column"
+ )
+ canonical.append(name)
+ return canonical
+
+ @staticmethod
+ def _adapt_native_order_by(value: Any) -> Any: # noqa: C901
+ if value is None:
+ return []
+ if not isinstance(value, list):
+ raise ValueError("order_by must be an array")
+ result: list[Any] = []
+ for index, entry in enumerate(value):
+ if isinstance(entry, str):
+ if len(entry) > 2000:
+ raise ValueError(f"order_by[{index}] is too long")
+ try:
+ entry = json.loads(entry)
+ except json.JSONDecodeError:
+ # A bare output/column name is the ergonomic typed form.
+ result.append({"column": entry, "ascending": False})
+ continue
+ if isinstance(entry, dict):
+ result.append(entry)
+ continue
+ if not isinstance(entry, (list, tuple)) or len(entry) != 2:
+ raise ValueError(
+ f"order_by[{index}] must be [column, ascending_boolean]"
+ )
+ target, ascending = entry
+ if isinstance(target, dict):
+ target = target.get("label") or target.get("metric_name")
+ if not isinstance(target, str) or not target:
+ raise ValueError(f"order_by[{index}] needs a column or metric
label")
+ if not isinstance(ascending, bool):
+ raise ValueError(f"order_by[{index}] ascending value must be
boolean")
+ result.append({"column": target, "ascending": ascending})
+ return result
+
+ @staticmethod
+ def _adapt_native_filters(data: dict[str, Any]) -> None: # noqa: C901
+ if "adhoc_filters" not in data:
+ return
+ if "filters" in data:
+ raise ValueError("Use either filters or native adhoc_filters, not
both")
+ raw_filters = data.pop("adhoc_filters")
+ if not isinstance(raw_filters, list):
+ raise ValueError("adhoc_filters must be an array")
+ filters: list[dict[str, Any]] = []
+ temporal_pairs: list[tuple[str, str]] = []
+ inert_subjects: set[str] = set()
+ for index, raw_filter in enumerate(raw_filters):
+ if not isinstance(raw_filter, dict):
+ raise ValueError(f"adhoc_filters[{index}] must be an object")
+ if raw_filter.get("expressionType") != "SIMPLE":
+ raise ValueError(
+ f"adhoc_filters[{index}] must use expressionType='SIMPLE'"
+ )
+ if raw_filter.get("clause") not in (None, "WHERE"):
+ raise ValueError(f"adhoc_filters[{index}] must use
clause='WHERE'")
+ subject = raw_filter.get("subject")
+ operator = raw_filter.get("operator")
+ comparator = raw_filter.get("comparator")
+ if operator == "TEMPORAL_RANGE":
+ if not isinstance(subject, str) or not subject:
+ raise ValueError(
+ f"adhoc_filters[{index}] temporal filter needs subject"
+ )
+ if not isinstance(comparator, str) or not comparator:
+ raise ValueError(
+ f"adhoc_filters[{index}] temporal filter needs a range"
+ )
+ if comparator.casefold() == "no filter":
+ inert_subjects.add(subject)
+ else:
+ temporal_pairs.append((subject, comparator))
+ continue
+ if not isinstance(operator, str):
+ raise ValueError(f"adhoc_filters[{index}] needs an operator")
+ operator_map = {
+ "==": "=",
+ "EQUALS": "=",
+ "NOT_EQUALS": "!=",
+ "LESS_THAN": "<",
+ "LESS_THAN_OR_EQUAL": "<=",
+ "GREATER_THAN": ">",
+ "GREATER_THAN_OR_EQUAL": ">=",
+ "NOT_IN": "NOT IN",
+ "IS_NULL": "IS NULL",
+ "IS_NOT_NULL": "IS NOT NULL",
+ }
+ operator = operator_map.get(operator, operator)
+ filters.append({"column": subject, "op": operator, "value":
comparator})
+ if len(temporal_pairs) > 1:
+ raise ValueError(
+ "Multiple active native temporal filters cannot be represented
by "
+ "a single temporal_column/time_range pair"
+ )
+ if temporal_pairs:
+ subject, comparator = temporal_pairs[0]
+ if data.get("temporal_column") not in (None, subject) or data.get(
+ "time_range"
+ ) not in (None, "No filter", comparator):
+ raise ValueError(
+ "Native temporal filter conflicts with
temporal_column/time_range"
+ )
+ data["temporal_column"] = subject
+ data["time_range"] = comparator
+ elif len(inert_subjects) == 1:
+ data.setdefault("temporal_column", next(iter(inert_subjects)))
+ data["filters"] = filters
+
+ @model_validator(mode="before")
+ @classmethod
+ def adapt_native_form_data(cls, raw: Any) -> Any: # noqa: C901
+ """Accept recognized saved Bullet form_data and reject ambiguous
state."""
+ if not isinstance(raw, dict):
+ return raw
+ data = dict(raw)
+ # Null means no hierarchy was supplied, not a conflicting alias or a
+ # request to clear saved dimensions. Only an explicit [] clears them.
+ for key in ("dimensions", "groupby"):
+ if data.get(key) is None:
+ data.pop(key, None)
+ if "dimensions" in data and "groupby" in data:
+ dimensions = cls._canonical_dimension_alias(
+ data["dimensions"], "dimensions"
+ )
+ groupby = cls._canonical_dimension_alias(data["groupby"],
"groupby")
+ if dimensions != groupby:
+ raise ValueError(
+ "Conflicting Bullet dimension aliases: 'dimensions' and "
+ "native 'groupby' must identify the same physical columns
in "
+ "the same order; provide only one or make them equivalent"
+ )
+ # Avoid relying on AliasChoices precedence or JSON key order.
+ data.pop("groupby")
+ if data.get("viz_type") == "bullet":
+ from superset.mcp_service.chart.preview_utils import (
+ _bullet_numeric_control_tokens,
+ )
+
+ data.setdefault("chart_type", "bullet")
+ data.pop("viz_type", None)
+ # Saved Explore controls are lenient; newly authored typed lists
+ # still undergo the strict List[float] validation below.
+ for control in ("ranges", "markers", "marker_lines"):
+ if control in data:
+ data[control] = _bullet_numeric_control_tokens(
+ data[control], control
+ )
+ for key in (
+ "annotation_layers",
+ "dashboards",
+ "datasource",
+ "datasource_id",
+ "datasource_type",
+ "extra_form_data",
+ "slice_id",
+ "slice_name",
+ ):
+ data.pop(key, None)
+
+ if (marker_key := "_mcp_dashboard_time_filter_subject") in data:
+ marker = data.pop(marker_key)
+ if not isinstance(marker, str) or not marker:
+ raise ValueError(f"{marker_key} must be a physical column
name")
+ raw_filters = data.get("adhoc_filters")
+ if not isinstance(raw_filters, list):
+ raise ValueError(
+ f"{marker_key} requires an adhoc_filters array containing
its "
+ "generated binding"
+ )
+ provenance_matches = [
+ filter_
+ for filter_ in raw_filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == marker
+ and filter_.get("operator") == "TEMPORAL_RANGE"
+ ]
+ if len(provenance_matches) != 1:
+ raise ValueError(
+ f"{marker_key} must match exactly one TEMPORAL_RANGE
filter "
+ f"for subject {marker!r}; found {len(provenance_matches)}"
+ )
+ data.setdefault("temporal_column", marker)
+
+ if "metric" in data:
+ data["metric"] = cls._adapt_native_metric(data["metric"])
+ for key in ("groupby", "dimensions"):
+ if key in data:
+ if not isinstance(data[key], list):
+ raise ValueError(f"{key} must be an array")
+ data[key] = [
Review Comment:
Fixed in `74cad83129d2fde30b08dac585523da0087198bb`: omitted inherited
Bullet dimensions use the native query contract, preserving SQL hierarchies and
their sorts.
##########
superset/mcp_service/chart/plugins/bullet.py:
##########
@@ -0,0 +1,462 @@
+# 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 are optional; missing
marker "
+ "labels fall back to formatted values."
+ ),
+ suggestions=[
+ "Use metric with aggregate, saved_metric, or sql_expression +
label",
+ "Use dimensions (alias: groupby) for row hierarchy",
+ "Use ranges, markers, and marker_lines for comparison targets",
+ ],
+ error_code="BULLET_VALIDATION_ERROR",
+ )
+
+ def normalize_query_result(self, result: Any, form_data: Mapping[str,
Any]) -> Any:
+ """Reject results that cannot size a Bullet chart without guessing."""
+ from superset.mcp_service.chart.query_result import query_result_data
+
+ data, failure = query_result_data(result, temporal_json_numbers=True)
+ if failure is not None:
+ return failure
+ rows = data[0] if data else []
+ if (error := _render_model_error(rows, form_data)) is not None:
+ return error
+ return result
+
+ def sanitize_data_rows(
+ self, data: list[Any], form_data: Mapping[str, Any]
+ ) -> tuple[list[Any], ChartError | None]:
+ """Expose rows through the same strict model the renderers use."""
+ from superset.mcp_service.chart.preview_utils import (
+ _safe_enum_backing,
+ BulletOutputError,
+ resolve_bullet_render_model,
+ )
+ from superset.mcp_service.chart.query_result import
safe_exception_message
+
+ try:
+ model = resolve_bullet_render_model(data, dict(form_data))
Review Comment:
Fixed in `74cad83129d2fde30b08dac585523da0087198bb`: raw row validation
skips preview-formatter checks, so `DURATION` and `MEMORY_BINARY` work for
JSON, CSV, and Excel.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1758,770 @@ 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 _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:
+ """Drop saved sorts that no longer target a final exact query output."""
+ saved = existing_form_data.get("orderby")
+ if not isinstance(saved, list):
+ return saved
+ outputs = {
+ name for name in new_form_data.get("groupby") or [] if
isinstance(name, str)
+ }
+ metrics = new_form_data.get("metrics") or []
+ if not isinstance(metrics, (list, tuple)):
+ metrics = [metrics]
+ for metric in [new_form_data.get("metric"), *metrics]:
+ label = metric.get("label") if isinstance(metric, Mapping) else metric
+ if isinstance(label, str):
+ outputs.add(label)
+ retained = []
+ for entry in saved:
+ if isinstance(entry, (list, tuple)) and entry:
+ target = entry[0]
+ if isinstance(target, Mapping):
+ target = target.get("label") or target.get("metric_name")
+ if isinstance(target, str) and target not in outputs:
+ continue
+ retained.append(entry)
Review Comment:
Fixed in `74cad83129d2fde30b08dac585523da0087198bb`: inherited
expression-based metric sorts are rebound to the final metric object when the
output label is unchanged.
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -809,10 +1739,557 @@ def build_mixed_timeseries_secondary(
return qd
-# Deck.gl viz types that conditionally set is_timeseries from time_grain_sqla
-_DECK_TIMESERIES_VIZ_TYPES: frozenset[str] = frozenset(
- {"deck_arc", "deck_path", "deck_polygon", "deck_scatter",
"deck_screengrid"}
-)
+def build_histogram_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Histogram buildQuery, including its histogram post-processing."""
+ column = form_data.get("column")
+ histogram_groupby = _as_list(form_data.get("groupby"))
+ query = build_single_query_dict(
+ form_data,
+ [*histogram_groupby, column] if column is not None else
histogram_groupby,
+ [],
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ having_filter = bool(form_data.get("having")) or any(
+ isinstance(filter_, dict) and filter_.get("clause") == "HAVING"
+ for filter_ in form_data.get("adhoc_filters") or []
+ )
+ if having_filter:
+ query["metrics"] = [
+ {
+ "expressionType": "SQL",
+ "sqlExpression": "COUNT(*)",
+ "label": "COUNT(*)",
+ }
+ ]
+ bins = form_data.get("bins", 5)
+ try:
+ parsed_bins = float(bins)
+ parsed_bins = int(parsed_bins) if parsed_bins.is_integer() else
parsed_bins
+ except (TypeError, ValueError):
+ parsed_bins = 5
+ query["post_processing"] = [
+ {
+ "operation": "histogram",
+ "options": {
+ "column": _column_label(column),
+ "groupby": [
+ label
+ for item in histogram_groupby
+ if (label := _column_label(item))
+ ],
+ "bins": parsed_bins,
+ "cumulative": bool(form_data.get("cumulative")),
+ "normalize": bool(form_data.get("normalize")),
+ },
+ }
+ ]
+ return [query]
+
+
+def build_box_plot_query_dicts( # noqa: C901
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Box Plot buildQuery, including its boxplot post-processing."""
+ distribute = _as_list(form_data.get("columns"))
+ if not distribute and form_data.get("granularity_sqla"):
+ distribute = [form_data["granularity_sqla"]]
+ box_groupby = _as_list(form_data.get("groupby"))
+ query = build_single_query_dict(
+ form_data,
+ [
+ *(_temporal_column(column, form_data) for column in distribute),
+ *box_groupby,
+ ],
+ list(form_data.get("metrics") or []),
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ query["series_columns"] = box_groupby
+ if whisker := form_data.get("whiskerOptions"):
+ whisker_type = "tukey"
+ percentiles: list[int] | None = None
+ if whisker == "Min/max (no outliers)":
+ whisker_type = "min/max"
+ elif match := re.fullmatch(r"(\d{1,3})/(\d{1,3}) percentiles",
str(whisker)):
+ whisker_type = "percentile"
+ percentiles = [int(match.group(1)), int(match.group(2))]
+ elif whisker != "Tukey":
+ raise ValueError(f"Unsupported whisker type: {whisker}")
+ query["post_processing"] = [
+ {
+ "operation": "boxplot",
+ "options": {
+ "whisker_type": whisker_type,
+ "percentiles": percentiles,
+ "groupby": [
+ label
+ for column in box_groupby
+ if (label := _column_label(column))
+ ],
+ "metrics": [
+ label
+ for metric in query["metrics"]
+ if (label := _metric_label(metric))
+ ],
+ },
+ }
+ ]
+ return [query]
+
+
+def build_pivot_table_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Pivot Table buildQuery, including subtotal grouping sets."""
+ rows = _as_list(form_data.get("groupbyRows"))
+ pivot_columns = _as_list(form_data.get("groupbyColumns"))
+ if form_data.get("transposePivot"):
+ rows, pivot_columns = pivot_columns, rows
+ columns = _dedupe_query_fields([*rows, *pivot_columns], _column_label)
+ query = build_single_query_dict(
+ form_data,
+ [_temporal_column(column, form_data) for column in columns],
+ list(form_data.get("metrics") or []),
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ sort_metric = query.get("series_limit_metric")
+ if sort_metric is None and query["metrics"]:
+ sort_metric = query["metrics"][0]
+ if sort_metric is not None:
+ query["orderby"] = [[sort_metric, not query.get("order_desc", True)]]
+ if grouping_sets := _pivot_grouping_sets(form_data, rows, pivot_columns):
+ query["grouping_sets"] = grouping_sets
+ return [query]
+
+
+def build_pie_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ contribution: bool,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Pie/Sunburst buildQuery; Pie adds a contribution operator."""
+ metric = form_data.get("metric")
+ query = build_single_query_dict(
+ form_data,
+ _as_list(form_data.get("groupby")),
+ [metric] if metric is not None else [],
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=form_data.get("orderby"),
+ )
+ if form_data.get("sort_by_metric") and metric is not None:
+ query["orderby"] = [[metric, False]]
+ if contribution and (label := _metric_label(metric)):
+ query["post_processing"] = [
+ {
+ "operation": "contribution",
+ "options": {
+ "columns": [label],
+ "rename_columns": [f"{label}__contribution"],
+ },
+ }
+ ]
+ return [query]
+
+
+def _positive_int(value: Any) -> int:
+ """Coerce a stored limit (int, numeric string, or empty) to a positive int
or 0."""
+ try:
+ coerced = int(value)
+ except (TypeError, ValueError):
+ return 0
+ return coerced if coerced > 0 else 0
+
+
+def build_table_query_dicts( # noqa: C901
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Table buildQuery: percent metrics, comparisons, totals,
paging."""
+ raw_mode = form_data.get("query_mode") == "raw" or (
+ form_data.get("query_mode") not in {"raw", "aggregate"}
+ and bool(form_data.get("all_columns"))
+ )
+ # Native extractQueryFields excludes empty-string column references.
+ table_columns = [
+ column
+ for column in _as_list(
+ form_data.get("all_columns") if raw_mode else
form_data.get("groupby")
+ )
+ if column != ""
+ ]
+ table_metrics = [] if raw_mode else _as_list(form_data.get("metrics"))
+ percent_metrics = [] if raw_mode else
_as_list(form_data.get("percent_metrics"))
+ table_metrics = _dedupe_query_fields(
+ [*table_metrics, *percent_metrics], _metric_label
+ )
+ table_orderby = _parse_orderby(form_data.get("order_by_cols"))
+ if not raw_mode:
+ sort_metrics = _as_list(form_data.get("timeseries_limit_metric"))
+ if sort_metrics:
+ table_orderby = [[sort_metrics[0], not form_data.get("order_desc",
False)]]
+ elif table_metrics:
+ table_orderby = [[table_metrics[0], False]]
+ query = build_single_query_dict(
+ form_data,
+ table_columns,
+ table_metrics,
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=table_orderby,
+ )
+ if not raw_mode:
+ # Table selects one temporal axis and places it before the other roles.
+ for index, column in enumerate(table_columns):
+ temporal_column = _temporal_column(column, form_data)
+ if temporal_column is not column:
+ query["columns"] = [
+ temporal_column,
+ *table_columns[:index],
+ *table_columns[index + 1 :],
+ ]
+ break
+ offsets = _table_time_offsets(form_data, query)
+ query["time_offsets"] = offsets
+ post_processing: list[dict[str, Any]] = []
+ contribution: dict[str, Any] | None = None
+ if percent_metrics:
+ labels: list[str] = []
+ for metric in percent_metrics:
+ if label := _metric_label(metric):
+ candidates = [label]
+ if offsets:
+ candidates.extend(f"{label}__{offset}" for offset in
offsets)
+ for candidate in candidates:
+ if candidate not in labels:
+ labels.append(candidate)
+ contribution = {
+ "operation": "contribution",
+ "options": {
+ "columns": labels,
+ "rename_columns": [f"%{label}" for label in labels],
+ },
+ }
+ post_processing.append(contribution)
+ if offsets and form_data.get("comparison_type") != "values":
+ source: list[str] = []
+ shifted: list[str] = []
+ for metric in table_metrics:
Review Comment:
Fixed in `74cad83129d2fde30b08dac585523da0087198bb`: Table comparisons use
ordinary metrics only, retaining distinct percentage-only metric and
contribution columns.
--
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]
