aminghadersohi commented on code in PR #43771:
URL: https://github.com/apache/superset/pull/43771#discussion_r4190029086
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -853,55 +1206,28 @@ def build_query_dicts_from_form_data(
if plugin_queries is not None:
return plugin_queries
- metrics, groupby = resolve_metrics_and_groupby(form_data, chart)
-
# Deck.gl charts use spatial column configs rather than the standard
# metrics / groupby fields. Extract columns from the spatial controls.
if viz_type.startswith("deck_"):
- deck_columns = resolve_deck_gl_columns(form_data)
- deck_metrics = _resolve_deck_gl_metrics(form_data, viz_type)
- qd = build_single_query_dict(
- form_data,
- deck_columns,
- deck_metrics,
- row_limit=row_limit,
- order_desc=order_desc,
- )
- if deck_metrics:
- # Mirror BaseDeckGLViz.query_obj(): order by first metric
descending
- qd["orderby"] = [(deck_metrics[0], not form_data.get("order_desc",
True))]
- if viz_type in _DECK_TIMESERIES_VIZ_TYPES and (
- time_grain := form_data.get("time_grain_sqla")
- ):
- qd["is_timeseries"] = True
- qd["granularity"] = form_data.get("granularity_sqla")
- qd.setdefault("extras", {})["time_grain_sqla"] = time_grain
- if form_data.get("filter_nulls", True):
- null_filters = _deck_gl_null_filters(form_data)
- if null_filters:
- qd["filters"] = [*(qd.get("filters") or []), *null_filters]
- return [qd]
-
- if viz_type.startswith("echarts_timeseries"):
- groupby = with_x_axis_column(form_data, groupby)
-
- return [
- build_single_query_dict(
+ from superset.common.form_data_query_context import
normalize_time_column
+
+ qd = build_query_objects_from_form_data(
form_data,
- groupby,
- metrics,
+ viz_type=viz_type,
row_limit=row_limit,
order_desc=order_desc,
- )
- ]
-
-
-def with_x_axis_column(form_data: dict[str, Any], groupby: list[Any]) ->
list[Any]:
- """Prepend a time-series chart's x-axis column to its query columns."""
- x_axis_col = extract_x_axis_col(form_data)
- if x_axis_col and x_axis_col not in groupby:
- return [x_axis_col, *groupby]
- return groupby
+ filters_prepared=True,
+ )[0]
+ qd = _deck_query_adapter(form_data, qd, viz_type)
+ return [normalize_time_column(form_data, qd)]
+ return build_query_objects_from_form_data(
Review Comment:
Fixed in 0d1196e052. `build_query_objects_from_form_data` now has a
World/Country Map adapter that mirrors both plugins' `buildQuery`: `entity` is
the only grouping column, Country Map selects `metric`, and World Map adds a
distinct `secondary_metric` and orders by the metric only when `sort_by_metric`
is set. A rebuild without `query_context` returns per-entity rows again.
Regression test: `test_build_query_dicts_groups_entity_maps_by_entity`.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1694,6 +1773,741 @@ def map_bubble_config(config: BubbleChartConfig) ->
Dict[str, Any]:
return form_data
+def map_sunburst_config(config: SunburstChartConfig) -> Dict[str, Any]:
+ """Map typed Sunburst config to the ECharts ``sunburst_v2`` form_data.
+
+ The frontend control panel stores hierarchy levels under ``columns`` and
+ metrics under singular ``metric`` / ``secondary_metric`` keys. Its
+ buildQuery adds primary-metric descending ordering when ``sort_by_metric``
+ is enabled; server-side query builders mirror that transform separately.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "sunburst_v2",
+ "columns": [dimension.name for dimension in config.hierarchy],
+ "metric": create_metric_object(config.metric),
+ "sort_by_metric": config.sort_by_metric,
+ "row_limit": config.row_limit,
+ "show_labels": config.show_labels,
+ "show_labels_threshold": config.show_labels_threshold,
+ "show_total": config.show_total,
+ "show_null_values": config.show_null_values,
+ "label_type": config.label_type,
+ "number_format": config.number_format,
+ "date_format": config.date_format,
+ }
+ if config.secondary_metric is not None:
+ form_data["secondary_metric"] =
create_metric_object(config.secondary_metric)
+ if config.color_scheme is not None:
+ form_data["color_scheme"] = config.color_scheme
+ if config.linear_color_scheme is not None:
+ form_data["linear_color_scheme"] = config.linear_color_scheme
+ if config.time_range is not None:
+ form_data["time_range"] = config.time_range
+ if config.temporal_column is not None:
+ form_data["granularity_sqla"] = config.temporal_column
+ if config.time_grain is not None:
+ form_data["time_grain_sqla"] = config.time_grain
+
+ _copy_sunburst_native_envelope(form_data, config)
+
+ add_currency_format(form_data, config.currency_format)
+ _add_adhoc_filters(form_data, config.filters)
+ return form_data
+
+
+# Sunburst fields with explicit omission/clear semantics. Mapper defaults must
+# not overwrite same-viz state when the typed field was omitted, while explicit
+# clears must also beat the shared preservation registry on cross-viz updates.
+# Required query roles (hierarchy and metric) are deliberately absent: a full
+# replacement always updates them.
+_SUNBURST_UPDATE_FIELD_KEYS: dict[str, str] = {
+ "time_range": "time_range",
+ "time_grain": "time_grain_sqla",
+ "temporal_column": "granularity_sqla",
+ "sort_by_metric": "sort_by_metric",
+ "row_limit": "row_limit",
+ "color_scheme": "color_scheme",
+ "linear_color_scheme": "linear_color_scheme",
+ "show_labels": "show_labels",
+ "show_labels_threshold": "show_labels_threshold",
+ "show_total": "show_total",
+ "show_null_values": "show_null_values",
+ "label_type": "label_type",
+ "number_format": "number_format",
+ "date_format": "date_format",
+ "currency_format": "currency_format",
+ "extra_form_data": "extra_form_data",
+ "url_params": "url_params",
+ "standardized_form_data": "standardizedFormData",
+}
+
+
+# Presentation controls emitted sparsely by chart mappers need three-way update
+# semantics: omitted preserves saved native state, an explicit value replaces
+# it, and explicit ``None``/``False`` clears a truthy saved value when the
mapper
+# has no canonical false/null representation. Query roles are intentionally
+# absent: a replacement config always owns those through the plugin contract.
+# Paths below also cover nested axis/legend models so an omitted nested
property
+# is not mistaken for an explicit clear of the whole control.
+_MODELED_UPDATE_CONTROL_PATHS: dict[str, dict[str, tuple[tuple[str, ...],
...]]] = {
+ "GaugeChartConfig": {
+ key: ((key,),)
+ for key in (
+ "sort_by_metric",
+ "row_limit",
+ "min_val",
+ "max_val",
+ "color_scheme",
+ "font_size",
+ "number_format",
+ "currency_format",
+ "value_formatter",
+ "start_angle",
+ "end_angle",
+ "show_pointer",
+ "animation",
+ "show_axis_tick",
+ "show_split_line",
+ "split_number",
+ "show_progress",
+ "overlap",
+ "round_cap",
+ "intervals",
+ "interval_color_indices",
+ "time_range",
+ "granularity_sqla",
+ )
+ },
+ "PieChartConfig": {
+ "color_scheme": (("color_scheme",),),
+ "show_labels": (("show_labels",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "label_type": (("label_type",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ "sort_by_metric": (("sort_by_metric",),),
+ "row_limit": (("row_limit",),),
+ "donut": (("donut",),),
+ "show_total": (("show_total",),),
+ "labels_outside": (("labels_outside",),),
+ "outerRadius": (("outer_radius",),),
+ "innerRadius": (("inner_radius",),),
+ "currency_format": (("currency_format",),),
+ },
+ "TableChartConfig": {
+ "row_limit": (("row_limit",),),
+ "color_scheme": (("color_scheme",),),
+ "column_config": (("column_config",),),
+ },
+ "XYChartConfig": {
+ "row_limit": (("row_limit",),),
+ "series_limit": (("series_limit",),),
+ "stack": (("stacked",),),
+ "orientation": (("orientation",),),
+ "x_axis_title": (("x_axis", "title"),),
+ "x_axis_format": (("x_axis", "format"),),
+ "y_axis_title": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "y_axis_scale": (("y_axis", "scale"),),
+ "show_legend": (("legend", "show"),),
+ "legendOrientation": (("legend", "position"), ("legend_orientation",)),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "show_value": (("show_value",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ },
+ "HistogramChartConfig": {
+ "bins": (("bins",),),
+ "normalize": (("normalize",),),
+ "cumulative": (("cumulative",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BoxPlotChartConfig": {
+ "whiskerOptions": (
+ ("whisker_type",),
+ ("percentile_low",),
+ ("percentile_high",),
+ ),
+ "row_limit": (("row_limit",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ },
+ "GanttChartConfig": {
+ "tooltip_columns": (("tooltip_columns",),),
+ "tooltip_metrics": (("tooltip_metrics",),),
+ "order_by_cols": (("order_by",),),
+ "row_limit": (("row_limit",),),
+ },
+ "WaterfallChartConfig": {
+ "show_total": (("show_total",),),
+ "show_legend": (("show_legend",),),
+ "increase_label": (("increase_label",),),
+ "decrease_label": (("decrease_label",),),
+ "total_label": (("total_label",),),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "y_axis_format": (("y_axis_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BigNumberChartConfig": {
+ "subheader": (("subheader",),),
+ "y_axis_format": (("y_axis_format",),),
+ "time_format": (("time_format",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ "start_y_axis_at_zero": (("start_y_axis_at_zero",),),
+ "compare_lag": (("compare_lag",),),
+ "aggregation": (("aggregation",),),
+ },
+ "HandlebarsChartConfig": {
+ "row_limit": (("row_limit",),),
+ "order_desc": (("order_desc",),),
+ "styleTemplate": (("style_template",),),
+ },
+ "PivotTableChartConfig": {
+ "aggregateFunction": (("aggregate_function",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "transposePivot": (("transpose",),),
+ "combineMetric": (("combine_metric",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "InteractivePivotChartConfig": {
+ "order_desc": (("sort_descending",),),
+ "row_limit": (("row_limit",),),
+ "rowGroupCounts": (("show_row_group_counts",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "colSubTotals": (("show_column_subtotals",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "colOrder": (("column_sort",),),
+ "allow_render_html": (("allow_render_html",),),
+ "expand_pivot_groups": (("expand_pivot_groups",),),
+ "time_compare": (("comparison_period",),),
+ "comparison_type": (("comparison_type",),),
+ },
+ "MixedTimeseriesChartConfig": {
+ "seriesType": (("primary_kind",),),
+ "area": (("primary_kind",),),
+ "seriesTypeB": (("secondary_kind",),),
+ "areaB": (("secondary_kind",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "show_value": (("show_value",),),
+ "color_scheme": (("color_scheme",),),
+ "currency_format": (("currency_format",),),
+ "currency_format_secondary": (("currency_format_secondary",),),
+ "xAxisTitle": (("x_axis", "title"),),
+ "x_axis_time_format": (("x_axis", "format"),),
+ "yAxisTitle": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "logAxis": (("y_axis", "scale"),),
+ "yAxisTitleSecondary": (("y_axis_secondary", "title"),),
+ "y_axis_format_secondary": (("y_axis_secondary", "format"),),
+ "logAxisSecondary": (("y_axis_secondary", "scale"),),
+ "row_limit": (("row_limit",),),
+ },
+}
+
+
+def _model_path_was_set(config: Any, path: tuple[str, ...]) -> bool:
+ """Return whether every component of a Pydantic model path was supplied."""
+ current = config
+ for field_name in path:
+ if field_name not in getattr(current, "model_fields_set", set()):
+ return False
+ current = getattr(current, field_name, None)
+ if current is None:
+ # An explicit null parent clears all of its mapped descendants.
+ return True
+ return True
+
+
+def _apply_modeled_update_semantics(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+) -> set[str]:
+ """Preserve truly omitted modeled controls and return explicit clears."""
+ explicit_clears: set[str] = set()
+ controls = _MODELED_UPDATE_CONTROL_PATHS.get(type(config).__name__, {})
+ for form_key, paths in controls.items():
+ if any(_model_path_was_set(config, path) for path in paths):
+ if form_key not in new_form_data:
+ explicit_clears.add(form_key)
+ continue
+ if form_key in existing_form_data:
+ new_form_data[form_key] = existing_form_data[form_key]
+ else:
+ new_form_data.pop(form_key, None)
+ return explicit_clears
+
+
+_TEMPORAL_FORM_DATA_KEYS = frozenset(
+ {
+ "granularity",
+ "granularity_sqla",
+ "since",
+ "time_grain",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+)
+
+
+def _is_temporal_filter(filter_: Any) -> bool:
+ """Return whether a native, adhoc, or legacy filter carries a time
range."""
+ return isinstance(filter_, dict) and (
+ filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("op") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("col") in {"__time_col", "__time_grain", "__time_range"}
+ )
+
+
+def _without_temporal_filters(value: Any) -> Any:
+ """Copy a filter list without temporal predicates, preserving other
shapes."""
+ if not isinstance(value, list):
+ return value
+ return [filter_ for filter_ in value if not _is_temporal_filter(filter_)]
+
+
+def _scrub_temporal_form_data(form_data: Mapping[str, Any]) -> Dict[str, Any]:
+ """Remove every source capable of reconstructing explicitly cleared time
state."""
+ scrubbed = dict(form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ scrubbed.pop(key, None)
+ scrubbed.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in scrubbed:
+ scrubbed[key] = _without_temporal_filters(scrubbed[key])
+
+ extra_form_data = scrubbed.get("extra_form_data")
+ if isinstance(extra_form_data, dict):
+ cleaned_extra = dict(extra_form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ cleaned_extra.pop(key, None)
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in cleaned_extra:
+ cleaned_extra[key] =
_without_temporal_filters(cleaned_extra[key])
+ scrubbed["extra_form_data"] = cleaned_extra
+ elif extra_form_data is None:
+ scrubbed.pop("extra_form_data", None)
+ return scrubbed
+
+
+# One bounded registry owns state that may survive a form-data replacement.
+# Query roles and plugin-specific controls are deliberately absent. This keeps
+# cross-viz transitions preview/save-safe without chart-by-chart allowlists
that
+# can drift as new plugins are registered.
+FORM_DATA_UPDATE_PRESERVE_KEYS: dict[str, frozenset[str]] = {
+ "envelope": frozenset(
+ {
+ "dashboardId",
+ "dashboards",
+ "datasource",
+ "extra_form_data",
+ "slice_id",
+ "slice_name",
+ "standardizedFormData",
+ "url_params",
+ }
+ ),
+ "presentation": frozenset(
+ {
+ "color_scheme",
+ "currency_format",
+ "date_format",
+ "legendOrientation",
+ "linear_color_scheme",
+ "number_format",
+ "show_legend",
+ }
+ ),
+ "filters": frozenset({"adhoc_filters", "extra_filters", "filters"}),
+ "time": frozenset(
+ {
+ "granularity_sqla",
+ "since",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+ ),
+}
+_FORM_DATA_UPDATE_PRESERVE_KEYS = frozenset().union(
+ *FORM_DATA_UPDATE_PRESERVE_KEYS.values()
+)
+
+
+def _merge_preserved_adhoc_filters(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ *,
+ drop_existing_temporal: bool,
+) -> list[Any] | None:
+ """Merge omitted structured filters while removing stale time bindings."""
+ previous = existing_form_data.get("adhoc_filters")
+ generated = new_form_data.get("adhoc_filters")
+ if not isinstance(previous, list):
+ return list(generated) if isinstance(generated, list) else None
+
+ previous_binding =
existing_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ new_binding = new_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ merged: list[Any] = []
+ for filter_ in previous:
+ is_temporal = (
+ isinstance(filter_, dict)
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ )
+ stale_generated_binding = (
+ is_temporal
+ and previous_binding
+ and previous_binding != new_binding
+ and filter_.get("subject") == previous_binding
+ and filter_.get("comparator") == NO_TIME_RANGE
+ )
+ if (drop_existing_temporal and is_temporal) or stale_generated_binding:
+ continue
+ merged.append(filter_)
+
+ for filter_ in generated if isinstance(generated, list) else []:
+ if isinstance(filter_, dict):
+ same_filter = any(
+ isinstance(previous_filter, dict)
+ and previous_filter.get("clause") == filter_.get("clause")
+ and previous_filter.get("expressionType")
+ == filter_.get("expressionType")
+ and previous_filter.get("subject") == filter_.get("subject")
+ and previous_filter.get("operator") == filter_.get("operator")
+ for previous_filter in merged
+ )
+ if same_filter:
+ continue
+ elif filter_ in merged:
+ continue
+ merged.append(filter_)
+ return merged
+
+
+def _merge_allowlisted_form_data(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+) -> Dict[str, Any]:
+ """Start from mapped target state and add only registry-approved
omissions."""
+ merged = dict(new_form_data)
+ for key in _FORM_DATA_UPDATE_PRESERVE_KEYS:
+ if key not in merged and key in existing_form_data:
+ merged[key] = existing_form_data[key]
+ return merged
+
+
+def merge_form_data_for_update(
+ existing_form_data: Dict[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool = False,
+) -> Dict[str, Any]:
+ """Merge mapped updates without leaking query roles across visualizations.
+
+ Same-viz updates retain native controls outside the simplified MCP schema
by
+ starting from saved form data. Cross-viz updates remain bounded by the
+ shared preservation registry. Explicit clears are applied last.
+
+ A dataset rebind prunes every dataset-bound role from the saved state and
+ then merges as a same-dataset update, unless the owning plugin declares a
+ strict rebind contract (``strict_dataset_rebind``), in which case its
+ ``merge_update_form_data`` hook receives ``dataset_rebind=True``. Plugins
+ that declare ``owns_update_merge`` merge same-viz updates themselves;
+ every other update takes the shared overlay and then the plugin's
+ ``finalize_update_form_data`` hook.
+ """
+ from superset.mcp_service.chart.registry import plugin_for_viz_type
+
+ plugin = plugin_for_viz_type(new_form_data.get("viz_type"))
+ if dataset_rebind and not (plugin is not None and
plugin.strict_dataset_rebind):
+ existing_form_data = scrub_dataset_bound_form_data(existing_form_data)
+ dataset_rebind = False
+
+ same_viz = existing_form_data.get("viz_type") ==
new_form_data.get("viz_type")
+ if same_viz and plugin is not None and (dataset_rebind or
plugin.owns_update_merge):
+ plugin_merged = plugin.merge_update_form_data(
+ existing_form_data,
+ new_form_data,
+ config,
+ dataset_rebind=dataset_rebind,
+ )
+ if plugin_merged is not None:
+ return plugin_merged
+ if dataset_rebind:
+ # A strict rebind never inherits saved state the plugin did not merge.
+ return dict(new_form_data)
+
+ merged = overlay_update_form_data(existing_form_data, new_form_data,
config)
+ if plugin is not None:
+ merged = plugin.finalize_update_form_data(
+ existing_form_data, new_form_data, merged, config
+ )
+ return merged
+
+
+def overlay_update_form_data(
+ existing_form_data: Dict[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+) -> Dict[str, Any]:
+ """Overlay a same-dataset update on the saved state with shared
semantics."""
+ same_viz = existing_form_data.get("viz_type") ==
new_form_data.get("viz_type")
+ explicit_control_clears = (
+ _apply_modeled_update_semantics(existing_form_data, new_form_data,
config)
+ if same_viz
+ else set()
+ )
+ if same_viz:
+ from superset.mcp_service.chart.registry import (
+ query_role_keys_for_viz_type,
+ )
+
+ # Strip every target-owned query role first, then overlay the mapper's
+ # complete replacement. This removes mutually exclusive aliases (for
+ # example Pie ``metrics`` vs ``metric`` and raw vs aggregate table
+ # roles) without dropping unmodeled native presentation controls.
+ query_role_keys = query_role_keys_for_viz_type(
+ str(new_form_data.get("viz_type"))
+ )
+ merged = {
+ key: value
+ for key, value in existing_form_data.items()
+ if key not in query_role_keys
+ }
+ merged.update(new_form_data)
+ else:
+ merged = _merge_allowlisted_form_data(existing_form_data,
new_form_data)
+
+ for key in explicit_control_clears:
+ merged.pop(key, None)
+
+ fields_set: set[str] = getattr(config, "model_fields_set", set())
+ if getattr(config, "filters", None) == []:
+ merged.pop("adhoc_filters", None)
+ elif getattr(config, "filters", None) is None:
+ filters = _merge_preserved_adhoc_filters(
+ existing_form_data,
+ new_form_data,
+ drop_existing_temporal=bool(
+ {"temporal_column", "time_grain", "time_range"} & fields_set
+ ),
+ )
+ if filters is not None:
+ merged["adhoc_filters"] = filters
+ return merged
+
+
+def retain_mixed_timeseries_secondary_update_state(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+) -> Dict[str, Any]:
+ """Return saved query-B controls a same-viz Mixed Timeseries update keeps.
+
+ Query B inherits unsuffixed controls only when the suffixed key is absent.
+ Preserve explicit native clears for controls the typed mapper did not
+ replace, so []/None never turns into accidental inheritance from query A.
+ Valid comparison state is also retained; malformed/stale dataset roles
+ remain fail-closed and are dropped.
+ """
+ from superset.common.form_data_query_context import (
+ MIXED_TIMESERIES_SECONDARY_QUERY_KEYS,
+ )
+
+ retained: Dict[str, Any] = {}
+ for key in MIXED_TIMESERIES_SECONDARY_QUERY_KEYS:
+ if key in new_form_data or key not in existing_form_data:
+ continue
+ value = existing_form_data[key]
+ is_explicit_clear = value is None or value in ([], {}, "")
+ is_valid_comparison = key == "comparison_type_b" and value in {
+ "values",
+ "difference",
+ "percentage",
+ "ratio",
+ }
+ is_valid_list_state = key in {
+ "adhoc_filters_b",
+ "annotation_layers_b",
+ "time_compare_b",
+ } and isinstance(value, list)
+ if is_explicit_clear or is_valid_comparison or is_valid_list_state:
+ retained[key] = value
+ return retained
+
+
+def finalize_sunburst_update_form_data( # noqa: C901
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ merged: Dict[str, Any],
+ config: SunburstChartConfig,
+) -> Dict[str, Any]:
+ """Apply Sunburst omission, explicit-clear and temporal-pair semantics."""
+ same_viz = existing_form_data.get("viz_type") ==
new_form_data.get("viz_type")
+ fields_set: set[str] = getattr(config, "model_fields_set", set())
+ temporal_fields = {"time_grain", "temporal_column"}
+ for field_name, form_key in _SUNBURST_UPDATE_FIELD_KEYS.items():
+ if field_name in temporal_fields:
+ continue
+ if field_name not in fields_set:
+ if same_viz:
+ if form_key in existing_form_data:
+ merged[form_key] = existing_form_data[form_key]
+ else:
+ merged.pop(form_key, None)
+ continue
+
+ value = getattr(config, field_name)
+ if value is None:
+ merged.pop(form_key, None)
+ if field_name == "time_range":
+ # The generic query-context mapper reconstructs time_range from
+ # these legacy keys. A clear must remove all three sources.
+ merged.pop("since", None)
+ merged.pop("until", None)
+
+ # A null temporal control is an atomic clear. Apply it after every merge so
+ # cached/native aliases and extra-form-data overrides cannot recreate time
+ # state in QueryContextFactory.
+ explicit_temporal_clear = any(
+ field_name in fields_set and getattr(config, field_name) is None
+ for field_name in ("temporal_column", "time_grain", "time_range")
+ )
+ if explicit_temporal_clear:
+ merged = _scrub_temporal_form_data(merged)
+
+ # Merge the temporal subject and grain as one final-state control pair.
+ temporal_set = "temporal_column" in fields_set
+ grain_set = "time_grain" in fields_set
+ if temporal_set and config.temporal_column is not None:
+ merged["granularity_sqla"] = config.temporal_column
+
+ if grain_set and config.time_grain is not None:
+ merged["time_grain_sqla"] = config.time_grain
+
+ if "time_range" in fields_set and config.time_range is not None:
+ merged["time_range"] = config.time_range
+
+ for key in ("extra_form_data", "standardizedFormData", "url_params"):
+ if merged.get(key) is None:
+ merged.pop(key, None)
+
+ # Do not discard an orphan grain here. The final-form-data validator must
+ # see and reject it consistently across immediate, preview-first, and
+ # cached update paths instead of letting the query builder silently ignore
+ # a request that cannot be represented by the frontend contract.
+ return merged
+
+
+_DATASET_BOUND_FORM_DATA_KEYS = frozenset(
Review Comment:
Fixed in 6fe5fcbb8b. `where` and `having` are now in
`_DATASET_BOUND_FORM_DATA_KEYS`, so a dataset rebind scrubs them along with
`adhoc_filters`/`filters`. `prepare_form_data_for_query` can no longer rebuild
them into the new dataset's query. Regression test:
`test_cached_table_rebind_drops_legacy_sql_predicates`.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1694,6 +1773,741 @@ def map_bubble_config(config: BubbleChartConfig) ->
Dict[str, Any]:
return form_data
+def map_sunburst_config(config: SunburstChartConfig) -> Dict[str, Any]:
+ """Map typed Sunburst config to the ECharts ``sunburst_v2`` form_data.
+
+ The frontend control panel stores hierarchy levels under ``columns`` and
+ metrics under singular ``metric`` / ``secondary_metric`` keys. Its
+ buildQuery adds primary-metric descending ordering when ``sort_by_metric``
+ is enabled; server-side query builders mirror that transform separately.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "sunburst_v2",
+ "columns": [dimension.name for dimension in config.hierarchy],
+ "metric": create_metric_object(config.metric),
+ "sort_by_metric": config.sort_by_metric,
+ "row_limit": config.row_limit,
+ "show_labels": config.show_labels,
+ "show_labels_threshold": config.show_labels_threshold,
+ "show_total": config.show_total,
+ "show_null_values": config.show_null_values,
+ "label_type": config.label_type,
+ "number_format": config.number_format,
+ "date_format": config.date_format,
+ }
+ if config.secondary_metric is not None:
+ form_data["secondary_metric"] =
create_metric_object(config.secondary_metric)
+ if config.color_scheme is not None:
+ form_data["color_scheme"] = config.color_scheme
+ if config.linear_color_scheme is not None:
+ form_data["linear_color_scheme"] = config.linear_color_scheme
+ if config.time_range is not None:
+ form_data["time_range"] = config.time_range
+ if config.temporal_column is not None:
+ form_data["granularity_sqla"] = config.temporal_column
+ if config.time_grain is not None:
+ form_data["time_grain_sqla"] = config.time_grain
+
+ _copy_sunburst_native_envelope(form_data, config)
+
+ add_currency_format(form_data, config.currency_format)
+ _add_adhoc_filters(form_data, config.filters)
+ return form_data
+
+
+# Sunburst fields with explicit omission/clear semantics. Mapper defaults must
+# not overwrite same-viz state when the typed field was omitted, while explicit
+# clears must also beat the shared preservation registry on cross-viz updates.
+# Required query roles (hierarchy and metric) are deliberately absent: a full
+# replacement always updates them.
+_SUNBURST_UPDATE_FIELD_KEYS: dict[str, str] = {
+ "time_range": "time_range",
+ "time_grain": "time_grain_sqla",
+ "temporal_column": "granularity_sqla",
+ "sort_by_metric": "sort_by_metric",
+ "row_limit": "row_limit",
+ "color_scheme": "color_scheme",
+ "linear_color_scheme": "linear_color_scheme",
+ "show_labels": "show_labels",
+ "show_labels_threshold": "show_labels_threshold",
+ "show_total": "show_total",
+ "show_null_values": "show_null_values",
+ "label_type": "label_type",
+ "number_format": "number_format",
+ "date_format": "date_format",
+ "currency_format": "currency_format",
+ "extra_form_data": "extra_form_data",
+ "url_params": "url_params",
+ "standardized_form_data": "standardizedFormData",
+}
+
+
+# Presentation controls emitted sparsely by chart mappers need three-way update
+# semantics: omitted preserves saved native state, an explicit value replaces
+# it, and explicit ``None``/``False`` clears a truthy saved value when the
mapper
+# has no canonical false/null representation. Query roles are intentionally
+# absent: a replacement config always owns those through the plugin contract.
+# Paths below also cover nested axis/legend models so an omitted nested
property
+# is not mistaken for an explicit clear of the whole control.
+_MODELED_UPDATE_CONTROL_PATHS: dict[str, dict[str, tuple[tuple[str, ...],
...]]] = {
+ "GaugeChartConfig": {
+ key: ((key,),)
+ for key in (
+ "sort_by_metric",
+ "row_limit",
+ "min_val",
+ "max_val",
+ "color_scheme",
+ "font_size",
+ "number_format",
+ "currency_format",
+ "value_formatter",
+ "start_angle",
+ "end_angle",
+ "show_pointer",
+ "animation",
+ "show_axis_tick",
+ "show_split_line",
+ "split_number",
+ "show_progress",
+ "overlap",
+ "round_cap",
+ "intervals",
+ "interval_color_indices",
+ "time_range",
+ "granularity_sqla",
+ )
+ },
+ "PieChartConfig": {
+ "color_scheme": (("color_scheme",),),
+ "show_labels": (("show_labels",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "label_type": (("label_type",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ "sort_by_metric": (("sort_by_metric",),),
+ "row_limit": (("row_limit",),),
+ "donut": (("donut",),),
+ "show_total": (("show_total",),),
+ "labels_outside": (("labels_outside",),),
+ "outerRadius": (("outer_radius",),),
+ "innerRadius": (("inner_radius",),),
+ "currency_format": (("currency_format",),),
+ },
+ "TableChartConfig": {
+ "row_limit": (("row_limit",),),
+ "color_scheme": (("color_scheme",),),
+ "column_config": (("column_config",),),
+ },
+ "XYChartConfig": {
+ "row_limit": (("row_limit",),),
+ "series_limit": (("series_limit",),),
+ "stack": (("stacked",),),
+ "orientation": (("orientation",),),
+ "x_axis_title": (("x_axis", "title"),),
+ "x_axis_format": (("x_axis", "format"),),
+ "y_axis_title": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "y_axis_scale": (("y_axis", "scale"),),
+ "show_legend": (("legend", "show"),),
+ "legendOrientation": (("legend", "position"), ("legend_orientation",)),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "show_value": (("show_value",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ },
+ "HistogramChartConfig": {
+ "bins": (("bins",),),
+ "normalize": (("normalize",),),
+ "cumulative": (("cumulative",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BoxPlotChartConfig": {
+ "whiskerOptions": (
+ ("whisker_type",),
+ ("percentile_low",),
+ ("percentile_high",),
+ ),
+ "row_limit": (("row_limit",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ },
+ "GanttChartConfig": {
+ "tooltip_columns": (("tooltip_columns",),),
+ "tooltip_metrics": (("tooltip_metrics",),),
+ "order_by_cols": (("order_by",),),
+ "row_limit": (("row_limit",),),
+ },
+ "WaterfallChartConfig": {
+ "show_total": (("show_total",),),
+ "show_legend": (("show_legend",),),
+ "increase_label": (("increase_label",),),
+ "decrease_label": (("decrease_label",),),
+ "total_label": (("total_label",),),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "y_axis_format": (("y_axis_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BigNumberChartConfig": {
+ "subheader": (("subheader",),),
+ "y_axis_format": (("y_axis_format",),),
+ "time_format": (("time_format",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ "start_y_axis_at_zero": (("start_y_axis_at_zero",),),
+ "compare_lag": (("compare_lag",),),
+ "aggregation": (("aggregation",),),
+ },
+ "HandlebarsChartConfig": {
+ "row_limit": (("row_limit",),),
+ "order_desc": (("order_desc",),),
+ "styleTemplate": (("style_template",),),
+ },
+ "PivotTableChartConfig": {
+ "aggregateFunction": (("aggregate_function",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "transposePivot": (("transpose",),),
+ "combineMetric": (("combine_metric",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "InteractivePivotChartConfig": {
+ "order_desc": (("sort_descending",),),
+ "row_limit": (("row_limit",),),
+ "rowGroupCounts": (("show_row_group_counts",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "colSubTotals": (("show_column_subtotals",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "colOrder": (("column_sort",),),
+ "allow_render_html": (("allow_render_html",),),
+ "expand_pivot_groups": (("expand_pivot_groups",),),
+ "time_compare": (("comparison_period",),),
+ "comparison_type": (("comparison_type",),),
+ },
+ "MixedTimeseriesChartConfig": {
+ "seriesType": (("primary_kind",),),
+ "area": (("primary_kind",),),
+ "seriesTypeB": (("secondary_kind",),),
+ "areaB": (("secondary_kind",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "show_value": (("show_value",),),
+ "color_scheme": (("color_scheme",),),
+ "currency_format": (("currency_format",),),
+ "currency_format_secondary": (("currency_format_secondary",),),
+ "xAxisTitle": (("x_axis", "title"),),
+ "x_axis_time_format": (("x_axis", "format"),),
+ "yAxisTitle": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "logAxis": (("y_axis", "scale"),),
+ "yAxisTitleSecondary": (("y_axis_secondary", "title"),),
+ "y_axis_format_secondary": (("y_axis_secondary", "format"),),
+ "logAxisSecondary": (("y_axis_secondary", "scale"),),
+ "row_limit": (("row_limit",),),
+ },
+}
+
+
+def _model_path_was_set(config: Any, path: tuple[str, ...]) -> bool:
+ """Return whether every component of a Pydantic model path was supplied."""
+ current = config
+ for field_name in path:
+ if field_name not in getattr(current, "model_fields_set", set()):
+ return False
+ current = getattr(current, field_name, None)
+ if current is None:
+ # An explicit null parent clears all of its mapped descendants.
+ return True
+ return True
+
+
+def _apply_modeled_update_semantics(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+) -> set[str]:
+ """Preserve truly omitted modeled controls and return explicit clears."""
+ explicit_clears: set[str] = set()
+ controls = _MODELED_UPDATE_CONTROL_PATHS.get(type(config).__name__, {})
+ for form_key, paths in controls.items():
+ if any(_model_path_was_set(config, path) for path in paths):
+ if form_key not in new_form_data:
+ explicit_clears.add(form_key)
+ continue
+ if form_key in existing_form_data:
+ new_form_data[form_key] = existing_form_data[form_key]
+ else:
+ new_form_data.pop(form_key, None)
+ return explicit_clears
+
+
+_TEMPORAL_FORM_DATA_KEYS = frozenset(
+ {
+ "granularity",
+ "granularity_sqla",
+ "since",
+ "time_grain",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+)
+
+
+def _is_temporal_filter(filter_: Any) -> bool:
+ """Return whether a native, adhoc, or legacy filter carries a time
range."""
+ return isinstance(filter_, dict) and (
+ filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("op") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("col") in {"__time_col", "__time_grain", "__time_range"}
+ )
+
+
+def _without_temporal_filters(value: Any) -> Any:
+ """Copy a filter list without temporal predicates, preserving other
shapes."""
+ if not isinstance(value, list):
+ return value
+ return [filter_ for filter_ in value if not _is_temporal_filter(filter_)]
+
+
+def _scrub_temporal_form_data(form_data: Mapping[str, Any]) -> Dict[str, Any]:
+ """Remove every source capable of reconstructing explicitly cleared time
state."""
+ scrubbed = dict(form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ scrubbed.pop(key, None)
+ scrubbed.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in scrubbed:
+ scrubbed[key] = _without_temporal_filters(scrubbed[key])
+
+ extra_form_data = scrubbed.get("extra_form_data")
+ if isinstance(extra_form_data, dict):
+ cleaned_extra = dict(extra_form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ cleaned_extra.pop(key, None)
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in cleaned_extra:
+ cleaned_extra[key] =
_without_temporal_filters(cleaned_extra[key])
+ scrubbed["extra_form_data"] = cleaned_extra
+ elif extra_form_data is None:
+ scrubbed.pop("extra_form_data", None)
+ return scrubbed
+
+
+# One bounded registry owns state that may survive a form-data replacement.
+# Query roles and plugin-specific controls are deliberately absent. This keeps
+# cross-viz transitions preview/save-safe without chart-by-chart allowlists
that
+# can drift as new plugins are registered.
+FORM_DATA_UPDATE_PRESERVE_KEYS: dict[str, frozenset[str]] = {
+ "envelope": frozenset(
+ {
+ "dashboardId",
+ "dashboards",
+ "datasource",
+ "extra_form_data",
+ "slice_id",
+ "slice_name",
+ "standardizedFormData",
+ "url_params",
+ }
+ ),
+ "presentation": frozenset(
+ {
+ "color_scheme",
+ "currency_format",
+ "date_format",
+ "legendOrientation",
+ "linear_color_scheme",
+ "number_format",
+ "show_legend",
+ }
+ ),
+ "filters": frozenset({"adhoc_filters", "extra_filters", "filters"}),
+ "time": frozenset(
+ {
+ "granularity_sqla",
+ "since",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+ ),
+}
+_FORM_DATA_UPDATE_PRESERVE_KEYS = frozenset().union(
+ *FORM_DATA_UPDATE_PRESERVE_KEYS.values()
+)
+
+
+def _merge_preserved_adhoc_filters(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ *,
+ drop_existing_temporal: bool,
+) -> list[Any] | None:
+ """Merge omitted structured filters while removing stale time bindings."""
+ previous = existing_form_data.get("adhoc_filters")
+ generated = new_form_data.get("adhoc_filters")
+ if not isinstance(previous, list):
+ return list(generated) if isinstance(generated, list) else None
+
+ previous_binding =
existing_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ new_binding = new_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ merged: list[Any] = []
+ for filter_ in previous:
+ is_temporal = (
+ isinstance(filter_, dict)
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ )
+ stale_generated_binding = (
+ is_temporal
+ and previous_binding
+ and previous_binding != new_binding
+ and filter_.get("subject") == previous_binding
+ and filter_.get("comparator") == NO_TIME_RANGE
+ )
+ if (drop_existing_temporal and is_temporal) or stale_generated_binding:
+ continue
+ merged.append(filter_)
+
+ for filter_ in generated if isinstance(generated, list) else []:
+ if isinstance(filter_, dict):
+ same_filter = any(
+ isinstance(previous_filter, dict)
+ and previous_filter.get("clause") == filter_.get("clause")
+ and previous_filter.get("expressionType")
+ == filter_.get("expressionType")
+ and previous_filter.get("subject") == filter_.get("subject")
+ and previous_filter.get("operator") == filter_.get("operator")
+ for previous_filter in merged
+ )
+ if same_filter:
+ continue
+ elif filter_ in merged:
+ continue
+ merged.append(filter_)
+ return merged
+
+
+def _merge_allowlisted_form_data(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+) -> Dict[str, Any]:
+ """Start from mapped target state and add only registry-approved
omissions."""
+ merged = dict(new_form_data)
+ for key in _FORM_DATA_UPDATE_PRESERVE_KEYS:
+ if key not in merged and key in existing_form_data:
+ merged[key] = existing_form_data[key]
+ return merged
+
+
+def merge_form_data_for_update(
+ existing_form_data: Dict[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool = False,
+) -> Dict[str, Any]:
+ """Merge mapped updates without leaking query roles across visualizations.
+
+ Same-viz updates retain native controls outside the simplified MCP schema
by
+ starting from saved form data. Cross-viz updates remain bounded by the
+ shared preservation registry. Explicit clears are applied last.
+
+ A dataset rebind prunes every dataset-bound role from the saved state and
+ then merges as a same-dataset update, unless the owning plugin declares a
+ strict rebind contract (``strict_dataset_rebind``), in which case its
+ ``merge_update_form_data`` hook receives ``dataset_rebind=True``. Plugins
+ that declare ``owns_update_merge`` merge same-viz updates themselves;
+ every other update takes the shared overlay and then the plugin's
+ ``finalize_update_form_data`` hook.
+ """
+ from superset.mcp_service.chart.registry import plugin_for_viz_type
+
+ plugin = plugin_for_viz_type(new_form_data.get("viz_type"))
Review Comment:
Fixed in 6fe5fcbb8b. `scrub_dataset_bound_form_data` takes a
`target_viz_type`. When it differs from the source viz, the replacement starts
from the mapped target config and keeps only the shared preservation-registry
keys that are not dataset-bound. No source role contract is needed, because the
cross-viz merge never inherits source query roles. Both
`merge_form_data_for_update` and the cached `update_chart_preview` path pass
it. A dataset-only rebind of an unregistered viz still fails closed. Tests:
`test_cross_viz_rebind_from_unregistered_viz_uses_mapped_target`,
`test_cached_cross_viz_rebind_from_unregistered_viz_uses_target_config`.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1694,6 +1773,741 @@ def map_bubble_config(config: BubbleChartConfig) ->
Dict[str, Any]:
return form_data
+def map_sunburst_config(config: SunburstChartConfig) -> Dict[str, Any]:
+ """Map typed Sunburst config to the ECharts ``sunburst_v2`` form_data.
+
+ The frontend control panel stores hierarchy levels under ``columns`` and
+ metrics under singular ``metric`` / ``secondary_metric`` keys. Its
+ buildQuery adds primary-metric descending ordering when ``sort_by_metric``
+ is enabled; server-side query builders mirror that transform separately.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "sunburst_v2",
+ "columns": [dimension.name for dimension in config.hierarchy],
+ "metric": create_metric_object(config.metric),
+ "sort_by_metric": config.sort_by_metric,
+ "row_limit": config.row_limit,
+ "show_labels": config.show_labels,
+ "show_labels_threshold": config.show_labels_threshold,
+ "show_total": config.show_total,
+ "show_null_values": config.show_null_values,
+ "label_type": config.label_type,
+ "number_format": config.number_format,
+ "date_format": config.date_format,
+ }
+ if config.secondary_metric is not None:
+ form_data["secondary_metric"] =
create_metric_object(config.secondary_metric)
+ if config.color_scheme is not None:
+ form_data["color_scheme"] = config.color_scheme
+ if config.linear_color_scheme is not None:
+ form_data["linear_color_scheme"] = config.linear_color_scheme
+ if config.time_range is not None:
+ form_data["time_range"] = config.time_range
+ if config.temporal_column is not None:
+ form_data["granularity_sqla"] = config.temporal_column
+ if config.time_grain is not None:
+ form_data["time_grain_sqla"] = config.time_grain
+
+ _copy_sunburst_native_envelope(form_data, config)
+
+ add_currency_format(form_data, config.currency_format)
+ _add_adhoc_filters(form_data, config.filters)
+ return form_data
+
+
+# Sunburst fields with explicit omission/clear semantics. Mapper defaults must
+# not overwrite same-viz state when the typed field was omitted, while explicit
+# clears must also beat the shared preservation registry on cross-viz updates.
+# Required query roles (hierarchy and metric) are deliberately absent: a full
+# replacement always updates them.
+_SUNBURST_UPDATE_FIELD_KEYS: dict[str, str] = {
+ "time_range": "time_range",
+ "time_grain": "time_grain_sqla",
+ "temporal_column": "granularity_sqla",
+ "sort_by_metric": "sort_by_metric",
+ "row_limit": "row_limit",
+ "color_scheme": "color_scheme",
+ "linear_color_scheme": "linear_color_scheme",
+ "show_labels": "show_labels",
+ "show_labels_threshold": "show_labels_threshold",
+ "show_total": "show_total",
+ "show_null_values": "show_null_values",
+ "label_type": "label_type",
+ "number_format": "number_format",
+ "date_format": "date_format",
+ "currency_format": "currency_format",
+ "extra_form_data": "extra_form_data",
+ "url_params": "url_params",
+ "standardized_form_data": "standardizedFormData",
+}
+
+
+# Presentation controls emitted sparsely by chart mappers need three-way update
+# semantics: omitted preserves saved native state, an explicit value replaces
+# it, and explicit ``None``/``False`` clears a truthy saved value when the
mapper
+# has no canonical false/null representation. Query roles are intentionally
+# absent: a replacement config always owns those through the plugin contract.
+# Paths below also cover nested axis/legend models so an omitted nested
property
+# is not mistaken for an explicit clear of the whole control.
+_MODELED_UPDATE_CONTROL_PATHS: dict[str, dict[str, tuple[tuple[str, ...],
...]]] = {
+ "GaugeChartConfig": {
+ key: ((key,),)
+ for key in (
+ "sort_by_metric",
+ "row_limit",
+ "min_val",
+ "max_val",
+ "color_scheme",
+ "font_size",
+ "number_format",
+ "currency_format",
+ "value_formatter",
+ "start_angle",
+ "end_angle",
+ "show_pointer",
+ "animation",
+ "show_axis_tick",
+ "show_split_line",
+ "split_number",
+ "show_progress",
+ "overlap",
+ "round_cap",
+ "intervals",
+ "interval_color_indices",
+ "time_range",
+ "granularity_sqla",
+ )
+ },
+ "PieChartConfig": {
+ "color_scheme": (("color_scheme",),),
+ "show_labels": (("show_labels",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "label_type": (("label_type",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ "sort_by_metric": (("sort_by_metric",),),
+ "row_limit": (("row_limit",),),
+ "donut": (("donut",),),
+ "show_total": (("show_total",),),
+ "labels_outside": (("labels_outside",),),
+ "outerRadius": (("outer_radius",),),
+ "innerRadius": (("inner_radius",),),
+ "currency_format": (("currency_format",),),
+ },
+ "TableChartConfig": {
+ "row_limit": (("row_limit",),),
+ "color_scheme": (("color_scheme",),),
+ "column_config": (("column_config",),),
+ },
+ "XYChartConfig": {
+ "row_limit": (("row_limit",),),
+ "series_limit": (("series_limit",),),
+ "stack": (("stacked",),),
+ "orientation": (("orientation",),),
+ "x_axis_title": (("x_axis", "title"),),
+ "x_axis_format": (("x_axis", "format"),),
+ "y_axis_title": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "y_axis_scale": (("y_axis", "scale"),),
+ "show_legend": (("legend", "show"),),
+ "legendOrientation": (("legend", "position"), ("legend_orientation",)),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "show_value": (("show_value",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ },
+ "HistogramChartConfig": {
+ "bins": (("bins",),),
+ "normalize": (("normalize",),),
+ "cumulative": (("cumulative",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BoxPlotChartConfig": {
+ "whiskerOptions": (
+ ("whisker_type",),
+ ("percentile_low",),
+ ("percentile_high",),
+ ),
+ "row_limit": (("row_limit",),),
+ "number_format": (("number_format",),),
+ "date_format": (("date_format",),),
+ },
+ "GanttChartConfig": {
+ "tooltip_columns": (("tooltip_columns",),),
+ "tooltip_metrics": (("tooltip_metrics",),),
+ "order_by_cols": (("order_by",),),
+ "row_limit": (("row_limit",),),
+ },
+ "WaterfallChartConfig": {
+ "show_total": (("show_total",),),
+ "show_legend": (("show_legend",),),
+ "increase_label": (("increase_label",),),
+ "decrease_label": (("decrease_label",),),
+ "total_label": (("total_label",),),
+ "x_axis_time_format": (("x_axis_time_format",),),
+ "y_axis_format": (("y_axis_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "BigNumberChartConfig": {
+ "subheader": (("subheader",),),
+ "y_axis_format": (("y_axis_format",),),
+ "time_format": (("time_format",),),
+ "currency_format": (("currency_format",),),
+ "color_scheme": (("color_scheme",),),
+ "start_y_axis_at_zero": (("start_y_axis_at_zero",),),
+ "compare_lag": (("compare_lag",),),
+ "aggregation": (("aggregation",),),
+ },
+ "HandlebarsChartConfig": {
+ "row_limit": (("row_limit",),),
+ "order_desc": (("order_desc",),),
+ "styleTemplate": (("style_template",),),
+ },
+ "PivotTableChartConfig": {
+ "aggregateFunction": (("aggregate_function",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "transposePivot": (("transpose",),),
+ "combineMetric": (("combine_metric",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "row_limit": (("row_limit",),),
+ },
+ "InteractivePivotChartConfig": {
+ "order_desc": (("sort_descending",),),
+ "row_limit": (("row_limit",),),
+ "rowGroupCounts": (("show_row_group_counts",),),
+ "rowTotals": (("show_row_totals",),),
+ "colTotals": (("show_column_totals",),),
+ "colSubTotals": (("show_column_subtotals",),),
+ "valueFormat": (("value_format",),),
+ "date_format": (("date_format",),),
+ "currency_format": (("currency_format",),),
+ "colOrder": (("column_sort",),),
+ "allow_render_html": (("allow_render_html",),),
+ "expand_pivot_groups": (("expand_pivot_groups",),),
+ "time_compare": (("comparison_period",),),
+ "comparison_type": (("comparison_type",),),
+ },
+ "MixedTimeseriesChartConfig": {
+ "seriesType": (("primary_kind",),),
+ "area": (("primary_kind",),),
+ "seriesTypeB": (("secondary_kind",),),
+ "areaB": (("secondary_kind",),),
+ "show_legend": (("show_legend",),),
+ "legendOrientation": (("legend_orientation",),),
+ "show_value": (("show_value",),),
+ "color_scheme": (("color_scheme",),),
+ "currency_format": (("currency_format",),),
+ "currency_format_secondary": (("currency_format_secondary",),),
+ "xAxisTitle": (("x_axis", "title"),),
+ "x_axis_time_format": (("x_axis", "format"),),
+ "yAxisTitle": (("y_axis", "title"),),
+ "y_axis_format": (("y_axis", "format"),),
+ "logAxis": (("y_axis", "scale"),),
+ "yAxisTitleSecondary": (("y_axis_secondary", "title"),),
+ "y_axis_format_secondary": (("y_axis_secondary", "format"),),
+ "logAxisSecondary": (("y_axis_secondary", "scale"),),
+ "row_limit": (("row_limit",),),
+ },
+}
+
+
+def _model_path_was_set(config: Any, path: tuple[str, ...]) -> bool:
+ """Return whether every component of a Pydantic model path was supplied."""
+ current = config
+ for field_name in path:
+ if field_name not in getattr(current, "model_fields_set", set()):
+ return False
+ current = getattr(current, field_name, None)
+ if current is None:
+ # An explicit null parent clears all of its mapped descendants.
+ return True
+ return True
+
+
+def _apply_modeled_update_semantics(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+) -> set[str]:
+ """Preserve truly omitted modeled controls and return explicit clears."""
+ explicit_clears: set[str] = set()
+ controls = _MODELED_UPDATE_CONTROL_PATHS.get(type(config).__name__, {})
+ for form_key, paths in controls.items():
+ if any(_model_path_was_set(config, path) for path in paths):
+ if form_key not in new_form_data:
+ explicit_clears.add(form_key)
+ continue
+ if form_key in existing_form_data:
+ new_form_data[form_key] = existing_form_data[form_key]
+ else:
+ new_form_data.pop(form_key, None)
+ return explicit_clears
+
+
+_TEMPORAL_FORM_DATA_KEYS = frozenset(
+ {
+ "granularity",
+ "granularity_sqla",
+ "since",
+ "time_grain",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+)
+
+
+def _is_temporal_filter(filter_: Any) -> bool:
+ """Return whether a native, adhoc, or legacy filter carries a time
range."""
+ return isinstance(filter_, dict) and (
+ filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("op") == FilterOperator.TEMPORAL_RANGE.value
+ or filter_.get("col") in {"__time_col", "__time_grain", "__time_range"}
+ )
+
+
+def _without_temporal_filters(value: Any) -> Any:
+ """Copy a filter list without temporal predicates, preserving other
shapes."""
+ if not isinstance(value, list):
+ return value
+ return [filter_ for filter_ in value if not _is_temporal_filter(filter_)]
+
+
+def _scrub_temporal_form_data(form_data: Mapping[str, Any]) -> Dict[str, Any]:
+ """Remove every source capable of reconstructing explicitly cleared time
state."""
+ scrubbed = dict(form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ scrubbed.pop(key, None)
+ scrubbed.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in scrubbed:
+ scrubbed[key] = _without_temporal_filters(scrubbed[key])
+
+ extra_form_data = scrubbed.get("extra_form_data")
+ if isinstance(extra_form_data, dict):
+ cleaned_extra = dict(extra_form_data)
+ for key in _TEMPORAL_FORM_DATA_KEYS:
+ cleaned_extra.pop(key, None)
+ for key in ("adhoc_filters", "extra_filters", "filters"):
+ if key in cleaned_extra:
+ cleaned_extra[key] =
_without_temporal_filters(cleaned_extra[key])
+ scrubbed["extra_form_data"] = cleaned_extra
+ elif extra_form_data is None:
+ scrubbed.pop("extra_form_data", None)
+ return scrubbed
+
+
+# One bounded registry owns state that may survive a form-data replacement.
+# Query roles and plugin-specific controls are deliberately absent. This keeps
+# cross-viz transitions preview/save-safe without chart-by-chart allowlists
that
+# can drift as new plugins are registered.
+FORM_DATA_UPDATE_PRESERVE_KEYS: dict[str, frozenset[str]] = {
+ "envelope": frozenset(
+ {
+ "dashboardId",
+ "dashboards",
+ "datasource",
+ "extra_form_data",
+ "slice_id",
+ "slice_name",
+ "standardizedFormData",
+ "url_params",
+ }
+ ),
+ "presentation": frozenset(
+ {
+ "color_scheme",
+ "currency_format",
+ "date_format",
+ "legendOrientation",
+ "linear_color_scheme",
+ "number_format",
+ "show_legend",
+ }
+ ),
+ "filters": frozenset({"adhoc_filters", "extra_filters", "filters"}),
+ "time": frozenset(
+ {
+ "granularity_sqla",
+ "since",
+ "time_grain_sqla",
+ "time_range",
+ "until",
+ }
+ ),
+}
+_FORM_DATA_UPDATE_PRESERVE_KEYS = frozenset().union(
+ *FORM_DATA_UPDATE_PRESERVE_KEYS.values()
+)
+
+
+def _merge_preserved_adhoc_filters(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+ *,
+ drop_existing_temporal: bool,
+) -> list[Any] | None:
+ """Merge omitted structured filters while removing stale time bindings."""
+ previous = existing_form_data.get("adhoc_filters")
+ generated = new_form_data.get("adhoc_filters")
+ if not isinstance(previous, list):
+ return list(generated) if isinstance(generated, list) else None
+
+ previous_binding =
existing_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ new_binding = new_form_data.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT)
+ merged: list[Any] = []
+ for filter_ in previous:
+ is_temporal = (
+ isinstance(filter_, dict)
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ )
+ stale_generated_binding = (
+ is_temporal
+ and previous_binding
+ and previous_binding != new_binding
+ and filter_.get("subject") == previous_binding
+ and filter_.get("comparator") == NO_TIME_RANGE
+ )
+ if (drop_existing_temporal and is_temporal) or stale_generated_binding:
+ continue
+ merged.append(filter_)
+
+ for filter_ in generated if isinstance(generated, list) else []:
+ if isinstance(filter_, dict):
+ same_filter = any(
+ isinstance(previous_filter, dict)
+ and previous_filter.get("clause") == filter_.get("clause")
+ and previous_filter.get("expressionType")
+ == filter_.get("expressionType")
+ and previous_filter.get("subject") == filter_.get("subject")
+ and previous_filter.get("operator") == filter_.get("operator")
+ for previous_filter in merged
+ )
+ if same_filter:
+ continue
+ elif filter_ in merged:
+ continue
+ merged.append(filter_)
+ return merged
+
+
+def _merge_allowlisted_form_data(
+ existing_form_data: Mapping[str, Any],
+ new_form_data: Mapping[str, Any],
+) -> Dict[str, Any]:
+ """Start from mapped target state and add only registry-approved
omissions."""
+ merged = dict(new_form_data)
+ for key in _FORM_DATA_UPDATE_PRESERVE_KEYS:
+ if key not in merged and key in existing_form_data:
+ merged[key] = existing_form_data[key]
+ return merged
+
+
+def merge_form_data_for_update(
+ existing_form_data: Dict[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool = False,
+) -> Dict[str, Any]:
+ """Merge mapped updates without leaking query roles across visualizations.
+
+ Same-viz updates retain native controls outside the simplified MCP schema
by
+ starting from saved form data. Cross-viz updates remain bounded by the
+ shared preservation registry. Explicit clears are applied last.
+
+ A dataset rebind prunes every dataset-bound role from the saved state and
+ then merges as a same-dataset update, unless the owning plugin declares a
+ strict rebind contract (``strict_dataset_rebind``), in which case its
+ ``merge_update_form_data`` hook receives ``dataset_rebind=True``. Plugins
+ that declare ``owns_update_merge`` merge same-viz updates themselves;
+ every other update takes the shared overlay and then the plugin's
+ ``finalize_update_form_data`` hook.
+ """
+ from superset.mcp_service.chart.registry import plugin_for_viz_type
+
+ plugin = plugin_for_viz_type(new_form_data.get("viz_type"))
+ if dataset_rebind and not (plugin is not None and
plugin.strict_dataset_rebind):
+ existing_form_data = scrub_dataset_bound_form_data(existing_form_data)
+ dataset_rebind = False
+
+ same_viz = existing_form_data.get("viz_type") ==
new_form_data.get("viz_type")
+ if same_viz and plugin is not None and (dataset_rebind or
plugin.owns_update_merge):
+ plugin_merged = plugin.merge_update_form_data(
+ existing_form_data,
+ new_form_data,
+ config,
+ dataset_rebind=dataset_rebind,
+ )
+ if plugin_merged is not None:
+ return plugin_merged
+ if dataset_rebind:
+ # A strict rebind never inherits saved state the plugin did not merge.
+ return dict(new_form_data)
+
+ merged = overlay_update_form_data(existing_form_data, new_form_data,
config)
+ if plugin is not None:
+ merged = plugin.finalize_update_form_data(
+ existing_form_data, new_form_data, merged, config
+ )
+ return merged
+
+
+def overlay_update_form_data(
+ existing_form_data: Dict[str, Any],
+ new_form_data: Dict[str, Any],
+ config: Any,
+) -> Dict[str, Any]:
+ """Overlay a same-dataset update on the saved state with shared
semantics."""
+ same_viz = existing_form_data.get("viz_type") ==
new_form_data.get("viz_type")
+ explicit_control_clears = (
+ _apply_modeled_update_semantics(existing_form_data, new_form_data,
config)
+ if same_viz
+ else set()
+ )
+ if same_viz:
+ from superset.mcp_service.chart.registry import (
+ query_role_keys_for_viz_type,
+ )
+
+ # Strip every target-owned query role first, then overlay the mapper's
+ # complete replacement. This removes mutually exclusive aliases (for
+ # example Pie ``metrics`` vs ``metric`` and raw vs aggregate table
+ # roles) without dropping unmodeled native presentation controls.
+ query_role_keys = query_role_keys_for_viz_type(
+ str(new_form_data.get("viz_type"))
+ )
+ merged = {
+ key: value
+ for key, value in existing_form_data.items()
+ if key not in query_role_keys
+ }
+ merged.update(new_form_data)
+ else:
+ merged = _merge_allowlisted_form_data(existing_form_data,
new_form_data)
+
+ for key in explicit_control_clears:
+ merged.pop(key, None)
+
+ fields_set: set[str] = getattr(config, "model_fields_set", set())
Review Comment:
Fixed in 6fe5fcbb8b. An explicit `filters=[]` now pops every saved predicate
source: `adhoc_filters`, `filters`, `extra_filters`, `where` and `having`. The
legacy keys can no longer survive the allowlisted merge and be rebuilt at query
time. Regression test:
`test_explicit_empty_filters_clear_legacy_predicates_on_cross_viz_update`,
which also builds the query and asserts no EMEA predicate.
--
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]
