aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4224693206
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1398,6 +2413,112 @@ def fallback_vega_lite_preview(
return None
+def generate_xy_pivot_vega_lite_preview(
+ data: list[dict[str, Any]], form_data: dict[str, Any], *, mark: str
+) -> VegaLitePreview | None:
+ """Render flattened timeseries pivot columns without dropping grouped
series.
+
+ Folding escaped field paths resolves literal output keys without splitting
+ category values that contain escaped commas. The legend retains each
+ complete metric/category label.
+ Long-form results continue through the generic renderer.
+ """
+ from superset.mcp_service.chart.chart_helpers import _as_list
+ from superset.utils.pandas_postprocessing.utils import (
+ escape_separator,
+ FLAT_COLUMN_SEPARATOR,
+ )
+
+ if not data:
+ return None
+ dimensions = [
+ label
+ for column in _as_list(form_data.get("groupby"))
+ if (label := _form_column_label(column))
+ ]
+ if not dimensions or any(label in data[0] for label in dimensions):
+ return None
+ x_axis = _form_column_label(form_data.get("x_axis")) or "__timestamp"
+ if x_axis not in data[0]:
+ return None
+ metric_labels = [
+ label
+ for metric in _as_list(form_data.get("metrics"))
+ if (label := metric_result_label(metric))
+ ]
+ # Chart-data results unescape the flattened column names, while raw
+ # post-processing output keeps escaped separators; match either spelling.
+ prefixes = {
+ spelling
+ for label in metric_labels
+ for spelling in (label, escape_separator(label))
+ }
+ fields = [
Review Comment:
Confirmed: with one compared metric the rename step names shifted series by
the bare offset (`1 year ago, East`), so the metric-prefix filter dropped them.
Fixed in 42e861005ca93f4d00cb60f87478eeff98fd32eb: when `_time_comparison`
holds and there is one metric, field discovery also accepts each `time_compare`
offset as a series prefix (multi-metric renames already keep the `revenue, 1
year ago, …` prefix). New
`test_xy_preview_renders_offset_named_comparison_series[values|difference]`
runs the real post-processing chain, unescapes columns like the query context
processor, and asserts every non-x column, including both `1 year ago, …`
series, is folded; both cases fail without the fix.
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1398,6 +2413,112 @@ def fallback_vega_lite_preview(
return None
+def generate_xy_pivot_vega_lite_preview(
+ data: list[dict[str, Any]], form_data: dict[str, Any], *, mark: str
+) -> VegaLitePreview | None:
+ """Render flattened timeseries pivot columns without dropping grouped
series.
+
+ Folding escaped field paths resolves literal output keys without splitting
+ category values that contain escaped commas. The legend retains each
+ complete metric/category label.
+ Long-form results continue through the generic renderer.
+ """
+ from superset.mcp_service.chart.chart_helpers import _as_list
+ from superset.utils.pandas_postprocessing.utils import (
+ escape_separator,
+ FLAT_COLUMN_SEPARATOR,
+ )
+
+ if not data:
+ return None
+ dimensions = [
+ label
+ for column in _as_list(form_data.get("groupby"))
+ if (label := _form_column_label(column))
+ ]
+ if not dimensions or any(label in data[0] for label in dimensions):
+ return None
+ x_axis = _form_column_label(form_data.get("x_axis")) or "__timestamp"
+ if x_axis not in data[0]:
+ return None
+ metric_labels = [
+ label
+ for metric in _as_list(form_data.get("metrics"))
+ if (label := metric_result_label(metric))
+ ]
+ # Chart-data results unescape the flattened column names, while raw
+ # post-processing output keeps escaped separators; match either spelling.
+ prefixes = {
+ spelling
+ for label in metric_labels
+ for spelling in (label, escape_separator(label))
+ }
+ fields = [
+ field
+ for field in data[0]
+ if field != x_axis
+ and any(
+ field.startswith(prefix + FLAT_COLUMN_SEPARATOR)
+ or field.startswith(prefix + "__")
+ for prefix in prefixes
+ )
+ ]
+ if not fields and len(metric_labels) == 1 and
form_data.get("truncate_metric"):
+ # A single truncated metric drops its label from the pivoted column
+ # names, so every non-x-axis column is one category series.
+ fields = [field for field in data[0] if field != x_axis]
+ if not fields:
+ return None
+ sample = data[0][x_axis]
+ x_type = (
+ "temporal"
+ if isinstance(sample, str) and any(char in sample for char in "-/: ")
+ else "quantitative"
+ if isinstance(sample, (int, float))
+ else "nominal"
+ )
+ return VegaLitePreview(
+ specification={
+ "$schema": "https://vega.github.io/schema/vega-lite/v5.json";,
+ "data": {"values": data},
+ "transform": [
+ {
+ "fold": [
+ "".join(
+ "\\" + char if char in ".[]\\" else char for char
in field
+ )
+ for field in fields
+ ],
+ "as": ["__mcp_xy_series", "__mcp_xy_value"],
+ }
+ ],
+ "mark": mark,
+ "encoding": {
+ "x": {"field": x_axis, "type": x_type, "title": x_axis},
Review Comment:
Confirmed. Fixed in 42e861005ca93f4d00cb60f87478eeff98fd32eb: the x encoding
and the x tooltip field use the same `.`/`[`/`]`/`\` escaping as the folded
series (one `vega_field` helper), while titles keep the raw name. New
`test_xy_preview_escapes_dotted_x_axis_field` uses a real `orders.date` x-axis
result and asserts `orders\\.date` in both the encoding and tooltip; it fails
without the fix.
##########
superset/mcp_service/chart/tool/update_chart_preview.py:
##########
@@ -208,11 +245,27 @@ def update_chart_preview( # noqa: C901
or (previous_form_data or {}).get("datasource_id")
or ""
).split("__", 1)[0]
- plugin = get_registry().get(config.chart_type,
include_disabled=True)
+ # The cached chart keeps its plugin's contract for the whole
+ # update, even when its chart type is disabled for new charts.
+ contract_scope.enter_context(
+ saved_chart_contract((previous_form_data or
{}).get("viz_type"))
+ )
+ plugin = get_registry().get(config.chart_type)
dataset_rebind = previous_datasource != str(dataset.id) and (
bool(previous_datasource)
or bool(plugin and plugin.unbound_form_data_is_rebind)
)
+ if (
+ dataset_rebind
+ and previous_form_data
+ and not (plugin is not None and plugin.strict_dataset_rebind)
+ ):
+ # Match saved-chart rebinds: retain only references that
resolve
+ # against the replacement dataset before resolving omitted
roles.
+ previous_form_data = _prune_inherited_query_state(
Review Comment:
Confirmed: the list-only check let a scalar saved `groupby` survive the
prune. Fixed in 329591907772d8dd7fe6b0e5b3ac13d34ba6ba7f:
`_inherited_state_invalid_keys` reads every inherited column-list role through
`_ensure_reference_list` (the `ensureIsArray` rule), so a scalar value is
checked as a one-item list for all charts, not only Bullet. New
`test_bullet_rebind_prunes_scalar_saved_hierarchy` drives
`_prune_inherited_query_state` (the helper `update_chart_preview` calls)
against a replacement dataset: `groupby: "OldRegion"` is dropped and `groupby:
"Region"` is kept; the drop case fails without the fix.
--
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]
