aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4240826317
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -3489,7 +3521,655 @@ def _as_column_list(value: Any) -> list[Any]:
return [value]
-def analyze_chart_capabilities(viz_type: str | None, config: Any) ->
ChartCapabilities:
+_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 _bullet_token_list(values: Sequence[str | int | float]) -> str:
+ """Serialize typed Bullet controls to the frontend's comma-separated
form."""
+ tokens: list[str] = []
+ for value in values:
+ if isinstance(value, float):
+ token = repr(value)
+ # ``100`` parses back to the same binary float as ``100.0`` and
+ # preserves the frontend's established compact integer spelling.
+ if token.endswith(".0") and not (
+ value == 0.0 and math.copysign(1.0, value) < 0
+ ):
+ token = token[:-2]
+ tokens.append(token)
+ else:
+ tokens.append(str(value))
+ return ",".join(tokens)
+
+
+def map_bullet_config(config: BulletChartConfig) -> Dict[str, Any]: # noqa:
C901
+ """Map typed Bullet config to ``Bullet/buildQuery`` and transformProps.
+
+ The frontend buildQuery replaces the generic query fields with exactly one
+ metric and the groupby hierarchy. Presentation controls stay in native
+ snake_case form_data; the chart plugin camelizes them for transformProps.
+ """
+ if (
+ config.dimensions is None
+ and config._inherited_groupby is None
+ and config.order_by
+ ):
+ # An update resolves its saved hierarchy before mapping. Without one,
+ # creation must validate sort targets against an empty hierarchy.
+ BulletChartConfig.model_validate(
+ {**config.model_dump(exclude_unset=True), "dimensions": []}
+ )
+ metric = create_metric_object(config.metric)
+ form_data: Dict[str, Any] = {
+ "viz_type": "bullet",
+ "metric": metric,
+ }
+
+ # Optional semantic/query fields are emitted only when explicitly supplied.
+ # This lets update_chart and update_chart_preview preserve native saved
state,
+ # while an explicit empty value still clears it through the generic merge
path.
+ # The row limit always carries the schema default so a new chart is
bounded;
+ # update merging restores the saved limit when the caller omits it.
+ if "dimensions" in config.model_fields_set:
+ form_data["groupby"] = [dimension.name for dimension in
config.dimensions or []]
+ form_data["row_limit"] = config.row_limit
+ if "time_range" in config.model_fields_set:
+ form_data["time_range"] = config.time_range
+
+ if config.order_by:
+ dimensions = config.order_dimensions
+ orderby: list[list[Any]] = []
+ for order in config.order_by:
+ role, index = resolve_bullet_order_target(
+ order.column, dimensions, config.metric
+ )
+ if role == "metric":
+ order_target: Any = metric
+ else:
+ if index is None: # Defensive: resolver pairs dimensions with
indexes.
+ raise ValueError("Bullet dimension order target has no
index")
+ dimension = dimensions[index]
+ order_target = (
+ dimension.name if isinstance(dimension, ColumnRef) else
dimension
+ )
+ orderby.append([order_target, order.ascending])
+ form_data["orderby"] = orderby
+ elif "order_by" in config.model_fields_set:
+ form_data["orderby"] = []
+
+ presentation_fields: dict[str, tuple[str, Any]] = {
+ "ranges": ("ranges", _bullet_token_list(config.ranges)),
+ "range_labels": (
+ "range_labels",
+ _bullet_token_list(config.range_labels),
+ ),
+ "markers": ("markers", _bullet_token_list(config.markers)),
+ "marker_labels": (
+ "marker_labels",
+ _bullet_token_list(config.marker_labels),
+ ),
+ "marker_lines": (
+ "marker_lines",
+ _bullet_token_list(config.marker_lines),
+ ),
+ "marker_line_labels": (
+ "marker_line_labels",
+ _bullet_token_list(config.marker_line_labels),
+ ),
+ "y_axis_format": ("y_axis_format", config.y_axis_format),
+ "show_labels": ("show_labels", config.show_labels),
+ "show_legend": ("show_legend", config.show_legend),
+ }
+ for field_name, (form_key, value) in presentation_fields.items():
+ if field_name in config.model_fields_set:
+ form_data[form_key] = value
+
+ _add_adhoc_filters(form_data, config.filters)
+ if config.filters == [] and "filters" in config.model_fields_set:
+ form_data["adhoc_filters"] = []
+ if config.time_range and config.temporal_column:
+ _ensure_temporal_adhoc_filter(form_data, config.temporal_column)
+ for filter_ in form_data.get("adhoc_filters", []):
+ if (
+ isinstance(filter_, dict)
+ and filter_.get("operator") ==
FilterOperator.TEMPORAL_RANGE.value
+ and filter_.get("subject") == config.temporal_column
+ and filter_.get("comparator") == NO_TIME_RANGE
+ ):
+ filter_["comparator"] = config.time_range
+ return form_data
+
+
+def _normalize_native_filter_aliases(form_data: Mapping[str, Any]) ->
Dict[str, Any]:
+ """Fold legacy WHERE, HAVING, and filter predicates into adhoc controls."""
+ 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)
+ return normalized
+
+
+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
+
+ normalized = _normalize_native_filter_aliases(form_data)
+ if "groupby" in normalized and not isinstance(normalized["groupby"], list):
+ # Bullet/buildQuery and transformProps read ensureIsArray(groupby).
+ normalized["groupby"] = bullet_groupby_list(normalized["groupby"])
+ 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",
+ # Native query context (dashboard/native filter predicates and time
+ # overrides) that buildQueryObject applies on top of the controls.
+ "extra_form_data",
+ "extra_filters",
+ 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 _bullet_output_labels(
+ form_data: Mapping[str, Any],
+) -> tuple[set[str], dict[str, Any]]:
+ """Return a Bullet state's dimension output labels and metric outputs."""
+ from superset.mcp_service.chart.chart_helpers import _column_label,
_metric_label
+
+ dimensions = {
+ label
+ for column in bullet_groupby_list(form_data.get("groupby"))
+ if (label := _column_label(column)) is not None
+ }
+ metrics = form_data.get("metrics") or []
+ if not isinstance(metrics, (list, tuple)):
+ metrics = [metrics]
+ metric_outputs = {
+ label: metric
+ for metric in [form_data.get("metric"), *metrics]
+ if (label := _metric_label(metric)) is not None
+ }
+ return dimensions, metric_outputs
+
+
+def bullet_groupby_list(groupby: Any) -> list[Any]:
+ """Normalize a saved Bullet hierarchy like the frontend
``ensureIsArray``."""
+ if groupby is None:
+ return []
+ return list(groupby) if isinstance(groupby, (list, tuple)) else [groupby]
+
+
+def _orderby_for_final_output_roles(
+ existing_form_data: Mapping[str, Any], new_form_data: Mapping[str, Any]
+) -> Any:
+ """Drop sorts on removed output roles and rebind inherited metric
expressions.
+
+ Native ordering may also rank by a saved metric or column that is not a
+ displayed output (``get_sqla_query`` resolves it independently). Those
+ sorters never named a Bullet role, so a role change does not remove them.
+ """
+ from superset.mcp_service.chart.chart_helpers import _column_label,
_metric_label
+
+ saved = existing_form_data.get("orderby")
+ if not isinstance(saved, list):
+ return saved
+ outputs, metric_outputs = _bullet_output_labels(new_form_data)
+ outputs.update(metric_outputs)
+ previous_dimensions, previous_metrics =
_bullet_output_labels(existing_form_data)
+ previous_outputs = previous_dimensions | set(previous_metrics)
+ retained = []
+ for entry in saved:
+ if isinstance(entry, (list, tuple)) and entry:
+ target = entry[0]
+ label = (
+ _metric_label(target)
+ or _column_label(target)
+ or target.get("metric_name")
+ if isinstance(target, Mapping)
+ else target
+ )
+ if (
+ isinstance(label, str)
+ and label not in outputs
+ and label in previous_outputs
+ ):
+ continue
+ if (
+ isinstance(target, Mapping)
+ and isinstance(label, str)
+ and label in metric_outputs
+ ):
+ # Label equality identifies an output role, not expression
+ # equality: execute the final metric, never the saved
expression.
+ entry = [metric_outputs[label], *entry[1:]]
+ 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_)
+
+
+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.
+ """
+ # A missing saved viz_type is a boundary too, matching
merge_chart_form_data.
+ if existing_form_data.get("viz_type") != new_form_data.get("viz_type"):
+ return
+ existing_form_data = _normalize_native_filter_aliases(existing_form_data)
+ # The initial overlay may carry legacy keys from saved form data. Filter
+ # omission/replacement below owns the complete canonical predicate
sequence.
+ for key in ("where", "having", "filters"):
+ new_form_data.pop(key, None)
+ 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
+ if (
+ existing_binding is None
+ and existing_subject is None
+ and range_explicit
+ and not subject_authoritative
+ and "filters" not in explicit_fields
+ ):
+ # A range-only update on an unmarked chart adopts its saved temporal
+ # filter even when Explore bound it to a column other than the mapper's
+ # proposed subject. Zero or several native ranges stay ambiguous, so
the
+ # proposed binding is added instead of guessing which one to rewrite.
+ unmarked_matches = [
+ filter_
+ for filter_ in existing_filters
+ if isinstance(filter_, dict)
+ and filter_.get("operator") == FilterOperator.TEMPORAL_RANGE.value
+ and filter_.get("expressionType") == "SIMPLE"
+ and filter_.get("clause") == "WHERE"
+ and isinstance(filter_.get("subject"), str)
+ ]
+ if len(unmarked_matches) == 1:
+ existing_binding = unmarked_matches[0]
+ existing_subject = existing_binding["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
Review Comment:
Valid, fixed in 6c28acc6da586e89417493be58f4f090efe18e27. With explicit
filters, an explicit time_range and no saved binding to rewrite, the merge now
falls back to the mapper's generated binding instead of dropping it, so
region=North plus Last month saves both filters. Regression test:
test_bullet_explicit_filters_and_range_without_saved_binding_keep_range.
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1809,7 +1822,1063 @@ def generate_xy_vega_lite_preview(
spec["transform"] = [
{"fold": series_columns, "as": [series_field, value_field]}
]
- encoding["y"] = {"field": value_field, "type": "quantitative"}
+ # Vega-Lite stacks bar and area marks colored by a nominal field
+ # by default; follow the chart's saved ``stack`` control instead.
+ encoding["y"] = {
+ "field": value_field,
+ "type": "quantitative",
+ "stack": _xy_vega_lite_stack(form_data.get("stack")),
+ }
encoding["color"] = {"field": series_field, "type": "nominal"}
+ if (
+ spec.get("mark") == "bar"
+ and encoding["y"]["stack"] is None
+ and "x" in encoding
+ ):
+ # Unstacked bars sit side by side like Explore's grouped bars.
+ # Offset channels need a discrete band x scale, so temporal
+ # values stay date-formatted through a millisecond time unit so
+ # sub-second timestamps keep distinct bands as in Explore.
+ x_encoding = encoding["x"]
+ if x_encoding.get("type") == "temporal":
+ x_encoding["timeUnit"] = (
+ "yearmonthdatehoursminutessecondsmilliseconds"
+ )
+ x_encoding["type"] = "ordinal"
+ encoding["xOffset"] = {"field": series_field, "type":
"nominal"}
return preview
+
+
+def _xy_vega_lite_stack(stack: Any) -> str | None:
+ """Map the ECharts timeseries ``stack`` control to a Vega-Lite stack."""
+ if not stack:
+ return None
+ return "normalize" if stack == "Expand" else "zero"
+
+
+def _truncate_utf8(value: str, max_bytes: int) -> str:
+ """Return bounded, replacement-decoded UTF-8 text.
+
+ Encoding even the non-truncated path is intentional: Python strings may
+ contain unpaired surrogates, while MCP/JSON responses must always be valid
+ UTF-8. Slicing by characters before encoding also prevents an
+ attacker-sized string from being encoded in full.
+ """
+ if max_bytes <= 0:
+ return ""
+ candidate = value[:max_bytes]
+ encoded = candidate.encode("utf-8", errors="replace")
+ if len(encoded) <= max_bytes and len(candidate) == len(value):
+ return encoded.decode("utf-8", errors="replace")
+ suffix = "... [truncated]"
+ suffix_bytes = suffix.encode()
+ if max_bytes <= len(suffix_bytes):
+ return suffix_bytes[:max_bytes].decode("ascii")
+ content_limit = max(0, max_bytes - len(suffix_bytes))
+ content = encoded[:content_limit].decode("utf-8", errors="ignore")
+ return content + suffix
+
+
+_MAX_BULLET_FIELDS = 256
+
+
+_MAX_BULLET_FIELD_BYTES = 1000
+
+
+_MAX_BULLET_TEXT_BYTES = 2000
+
+
+# ECMAScript WhiteSpace and LineTerminator characters used by trim/Number.
+_JAVASCRIPT_WHITESPACE = (
+ "\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003\u2004"
+ "\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff"
+)
+
+
+_MAX_BULLET_TOKENS = 256
+
+
+# A comma-joined presentation control holds up to _MAX_BULLET_TOKENS tokens of
+# _MAX_BULLET_TEXT_BYTES each, so the whole-control bound must cover that many
+# tokens plus separators rather than reuse the per-token bound.
+_MAX_BULLET_CONTROL_CHARS = _MAX_BULLET_TOKENS * (_MAX_BULLET_TEXT_BYTES + 1)
+
+
+_ENUM_SCALAR_TYPES = (str, int, float, bool, Decimal)
+
+
+class BulletOutputError(ValueError):
+ """A Bullet query result cannot be rendered without guessing its roles."""
+
+ def __init__(self, message: str, error_type: str =
"MalformedBulletOutput") -> None:
+ super().__init__(message)
+ self.error_type = error_type
+
+
+@dataclass(frozen=True)
+class BulletRenderModel:
+ """Strict, frontend-aligned data and presentation roles for one preview."""
+
+ rows: list[dict[str, Any]]
+ metric_field: str
+ dimensions: list[str]
+ measures: list[float]
+ ranges: list[float]
+ range_labels: list[str]
+ markers: list[float]
+ marker_labels: list[str]
+ marker_lines: list[float]
+ marker_line_labels: list[str]
+ y_axis_format: str
+ show_labels: bool
+ show_legend: bool
+
+
+def _form_metric_label(metric: Any) -> str | None:
+ """Return the result-column label for a native QueryFormMetric."""
+ if type(metric) is str:
+ return metric
+ if type(metric) is not dict:
+ return None
+ if label := dict.get(metric, "label"):
+ return label if type(label) is str else None
+ if dict.get(metric, "expressionType") == "SQL":
+ expression = dict.get(metric, "sqlExpression")
+ return expression if type(expression) is str and expression else None
+ column = dict.get(metric, "column")
+ column_name = dict.get(column, "column_name") if type(column) is dict else
column
+ aggregate = dict.get(metric, "aggregate")
+ if type(column_name) is str and type(aggregate) is str:
+ return f"{aggregate}({column_name})"
+ return None
+
+
+def _form_column_label(column: Any) -> str | None:
+ """Return the result-column label for a native QueryFormColumn."""
+ if type(column) is str:
+ return column
+ if type(column) is not dict:
+ return None
+ # getColumnLabel: an adhoc column without a label is keyed by its SQL.
+ for key in ("label", "sqlExpression", "column_name"):
+ if type(value := dict.get(column, key)) is str and value:
+ return value
+ return None
+
+
+def _require_result_field(label: str | None, row: dict[str, Any], role: str)
-> str:
+ """Resolve a role without falling back to an unrelated result field."""
+ if not label:
+ raise BulletOutputError(f"Bullet {role} has no declared result alias")
+ if label in dict.keys(row):
+ return label
+ matches = sorted(
+ field
+ for field in dict.keys(row)
+ if type(field) is str and field.casefold() == label.casefold()
+ )
+ if len(matches) == 1:
+ return matches[0]
+ if matches:
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is ambiguous; candidates: "
+ f"{', '.join(matches)}"
+ )
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is missing from query output"
+ )
+
+
+def _safe_enum_backing(value: Any) -> Any:
+ """Extract Enum's stored value without public descriptors/conversions."""
+ value_type = type(value)
+ try:
+ mro = type.__getattribute__(value_type, "__mro__")
+ except (AttributeError, TypeError): # pragma: no cover - normal types
have MRO
+ return value
+ if type(mro) is not tuple or not any(base is Enum for base in mro):
+ return value
+ try:
+ backing = object.__getattribute__(value, "_value_")
+ except Exception as ex:
+ raise BulletOutputError("Bullet output contains an unreadable enum")
from ex
+ if not any(type(backing) is allowed for allowed in _ENUM_SCALAR_TYPES):
+ raise BulletOutputError("Bullet output contains an unsupported enum
value")
+ return backing
+
+
+def _decimal_javascript_string(value: Decimal) -> str:
+ """Render an exact binary64 spelling with JavaScript Number thresholds."""
+ sign, digits_tuple, exponent = Decimal.as_tuple(value)
+ if type(exponent) is not int: # finite Decimals always have an integer
exponent
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ if not any(digits_tuple):
+ return "0"
+
+ digits = "".join(str(digit) for digit in digits_tuple)
+ adjusted = len(digits) + exponent - 1
+ prefix = "-" if sign else ""
+ if -6 <= adjusted < 21:
+ point = len(digits) + exponent
+ if point <= 0:
+ text = f"0.{('0' * -point)}{digits}"
+ elif point >= len(digits):
+ text = digits + ("0" * (point - len(digits)))
+ else:
+ text = f"{digits[:point]}.{digits[point:]}"
+ if "." in text:
+ text = text.rstrip("0").rstrip(".")
+ return prefix + text
+
+ fraction = digits[1:].rstrip("0")
+ coefficient = digits[0] + (f".{fraction}" if fraction else "")
+ exponent_text = f"+{adjusted}" if adjusted >= 0 else str(adjusted)
+ return f"{prefix}{coefficient}e{exponent_text}"
+
+
+def _javascript_number_string(value: int | float | Decimal) -> str:
+ """Apply JSON-number -> IEEE-754 Number -> JavaScript String semantics.
+
+ Exact result scalars can retain precision that the frontend cannot: JSON
+ parsing first rounds a numeric token to binary64, and ``String`` then emits
+ the shortest round-tripping decimal with fixed notation for exponents in
+ [-6, 20]. Converting exact builtin scalars to an exact builtin float keeps
+ the path hook-free. Python and JavaScript use the same shortest
+ round-tripping binary64 digits; ``_decimal_javascript_string`` only adjusts
+ the notation thresholds and exponent spelling.
+
+ A finite integer or Decimal outside binary64's range becomes an infinity
+ after JSON parsing, matching JavaScript. Non-finite source values are
+ rejected by the trusted scalar normalizer before this helper is called.
+ """
+ value_type = type(value)
+ if value_type not in {int, float, Decimal}:
+ raise BulletOutputError("Bullet dimension contains an unsupported
number")
+ if value_type is float and not math.isfinite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
number")
+ if isinstance(value, Decimal) and not Decimal.is_finite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ try:
+ number = float(value)
+ except OverflowError:
+ number = -math.inf if value < 0 else math.inf
+
+ if math.isinf(number):
+ return "-Infinity" if number < 0 else "Infinity"
+ if number == 0:
+ # String(-0) is "0" even though JSON.parse preserves negative zero.
+ return "0"
+ return _decimal_javascript_string(Decimal(float.__repr__(number)))
+
+
+def _bullet_category_value( # noqa: C901
+ value: Any, dimension: str, row_index: int
+) -> tuple[Any, str]:
+ """Return a JSON-safe value and bounded frontend ``String(value)`` text.
+
+ The trusted scalar normalizer is type-exact and does not dispatch through
+ application hooks. Vega data retains the normalized Chart Data wire value
+ (including epoch-ms temporal numbers); only the derived category key and
+ ASCII label use the JavaScript-compatible text.
+ """
+ from superset.mcp_service.chart.query_result import (
+ _bounded_utf8_length,
+ _chart_data_duration_text,
+ _chart_data_temporal_number,
+ _is_chart_data_duration_scalar,
+ _is_chart_data_temporal_scalar,
+ _normalize_scalar,
+ )
+
+ normalized: Any
+ reason: str | None
+ if type(value) in {list, tuple, dict}:
+ data, failure = first_query_data(
+ {"queries": [{"data": [{"value": value}]}]},
temporal_json_numbers=True
+ )
+ if failure is not None or data is None:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} "
+ "has an invalid or unbounded container value"
+ )
+ normalized = data[0]["value"]
+ reason = None
+ elif _is_chart_data_temporal_scalar(value := _safe_enum_backing(value)):
+ normalized, reason = _chart_data_temporal_number(value)
+ elif _is_chart_data_duration_scalar(value):
+ normalized, reason = _chart_data_duration_text(value)
+ else:
+ normalized, reason = _normalize_scalar(value)
+ if (
+ reason is None
+ and normalized is None
+ and value is not None
+ and type(value) in {float, np.float16, np.float32, np.float64}
+ and math.isinf(float(value))
+ ):
+ # NaN keeps the shared null normalization, but an infinite source
+ # value has no faithful category text and is rejected.
+ reason = "contains a non-finite number"
+ if (
+ reason is None
+ and type(normalized) is str
+ and _bounded_utf8_length(normalized, _MAX_BULLET_TEXT_BYTES) is
None
Review Comment:
Valid, fixed in 6c28acc6da586e89417493be58f4f090efe18e27.
resolve_bullet_render_model and _bullet_category_value take bound_labels; the
raw-data paths (normalize_query_result and sanitize_data_rows, used by
get_chart_data and JSON/CSV/Excel exports) pass False, so dimension strings are
checked against the shared MAX_RESULT_STRING_LENGTH and no label text is
derived. The ASCII and Vega preview renderers still apply the 2,000-byte label
bound. Regression test:
test_bullet_raw_rows_keep_shared_limit_for_long_dimension_text.
--
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]
