aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r3944687848
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -323,6 +370,536 @@ def _generate_safe_ascii_bar_chart(data: List[Dict[str,
Any]]) -> str:
return "\n".join(lines)
+def _form_metric_label(metric: Any) -> str | None:
+ """Return the result-column label for a native QueryFormMetric."""
+ if type(metric) is str:
+ return metric
+ if type(metric) is not dict:
+ return None
+ if label := dict.get(metric, "label"):
+ return label if type(label) is str else None
+ if dict.get(metric, "expressionType") == "SQL":
+ expression = dict.get(metric, "sqlExpression")
+ return expression if type(expression) is str and expression else None
+ column = dict.get(metric, "column")
+ column_name = dict.get(column, "column_name") if type(column) is dict else
column
+ aggregate = dict.get(metric, "aggregate")
+ if type(column_name) is str and type(aggregate) is str:
+ return f"{aggregate}({column_name})"
+ return None
+
+
+def _form_column_label(column: Any) -> str | None:
+ """Return the result-column label for a native QueryFormColumn."""
+ if type(column) is str:
+ return column
+ if type(column) is not dict:
+ return None
+ for key in ("label", "column_name"):
+ if type(value := dict.get(column, key)) is str and value:
+ return value
+ return None
+
+
+def _canonical_result_field(label: str | None, row: Dict[str, Any]) -> str |
None:
+ """Resolve an exact or one unambiguous casefold result-field match."""
+ if label is None:
+ return None
+ if label in dict.keys(row):
+ return label
+ matches = [
+ field
+ for field in dict.keys(row)
+ if type(field) is str and field.casefold() == label.casefold()
+ ]
+ return matches[0] if len(matches) == 1 else None
+
+
+def _require_result_field(label: str | None, row: dict[str, Any], role: str)
-> str:
+ """Resolve a role without falling back to an unrelated result field."""
+ if not label:
+ raise BulletOutputError(f"Bullet {role} has no declared result alias")
+ if label in dict.keys(row):
+ return label
+ matches = sorted(
+ field
+ for field in dict.keys(row)
+ if type(field) is str and field.casefold() == label.casefold()
+ )
+ if len(matches) == 1:
+ return matches[0]
+ if matches:
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is ambiguous; candidates: "
+ f"{', '.join(matches)}"
+ )
+ raise BulletOutputError(
+ f"Bullet {role} alias {label!r} is missing from query output"
+ )
+
+
+def _safe_enum_backing(value: Any) -> Any:
+ """Extract Enum's stored value without public descriptors/conversions."""
+ value_type = type(value)
+ try:
+ mro = type.__getattribute__(value_type, "__mro__")
+ except (AttributeError, TypeError): # pragma: no cover - normal types
have MRO
+ return value
+ if type(mro) is not tuple or not any(base is Enum for base in mro):
+ return value
+ try:
+ backing = object.__getattribute__(value, "_value_")
+ except Exception as ex:
+ raise BulletOutputError("Bullet output contains an unreadable enum")
from ex
+ if not any(type(backing) is allowed for allowed in _ENUM_SCALAR_TYPES):
+ raise BulletOutputError("Bullet output contains an unsupported enum
value")
+ return backing
+
+
+def _decimal_javascript_string(value: Decimal) -> str:
+ """Render an exact binary64 spelling with JavaScript Number thresholds."""
+ sign, digits_tuple, exponent = Decimal.as_tuple(value)
+ if type(exponent) is not int: # finite Decimals always have an integer
exponent
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ if not any(digits_tuple):
+ return "0"
+
+ digits = "".join(str(digit) for digit in digits_tuple)
+ adjusted = len(digits) + exponent - 1
+ prefix = "-" if sign else ""
+ if -6 <= adjusted < 21:
+ point = len(digits) + exponent
+ if point <= 0:
+ text = f"0.{('0' * -point)}{digits}"
+ elif point >= len(digits):
+ text = digits + ("0" * (point - len(digits)))
+ else:
+ text = f"{digits[:point]}.{digits[point:]}"
+ if "." in text:
+ text = text.rstrip("0").rstrip(".")
+ return prefix + text
+
+ fraction = digits[1:].rstrip("0")
+ coefficient = digits[0] + (f".{fraction}" if fraction else "")
+ exponent_text = f"+{adjusted}" if adjusted >= 0 else str(adjusted)
+ return f"{prefix}{coefficient}e{exponent_text}"
+
+
+def _javascript_number_string(value: int | float | Decimal) -> str:
+ """Apply JSON-number -> IEEE-754 Number -> JavaScript String semantics.
+
+ Exact result scalars can retain precision that the frontend cannot: JSON
+ parsing first rounds a numeric token to binary64, and ``String`` then emits
+ the shortest round-tripping decimal with fixed notation for exponents in
+ [-6, 20]. Converting exact builtin scalars to an exact builtin float keeps
+ the path hook-free. Python and JavaScript use the same shortest
+ round-tripping binary64 digits; ``_decimal_javascript_string`` only adjusts
+ the notation thresholds and exponent spelling.
+
+ A finite integer or Decimal outside binary64's range becomes an infinity
+ after JSON parsing, matching JavaScript. Non-finite source values are
+ rejected by the trusted scalar normalizer before this helper is called.
+ """
+ value_type = type(value)
+ if value_type not in {int, float, Decimal}:
+ raise BulletOutputError("Bullet dimension contains an unsupported
number")
+ if value_type is float and not math.isfinite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
number")
+ if isinstance(value, Decimal) and not Decimal.is_finite(value):
+ raise BulletOutputError("Bullet dimension contains a non-finite
Decimal")
+ try:
+ number = float(value)
+ except OverflowError:
+ number = -math.inf if value < 0 else math.inf
+
+ if math.isinf(number):
+ return "-Infinity" if number < 0 else "Infinity"
+ if number == 0:
+ # String(-0) is "0" even though JSON.parse preserves negative zero.
+ return "0"
+ return _decimal_javascript_string(Decimal(float.__repr__(number)))
+
+
+def _bullet_category_value( # noqa: C901
+ value: Any, dimension: str, row_index: int
+) -> tuple[Any, str]:
+ """Return a JSON-safe value and bounded frontend ``String(value)`` text.
+
+ The trusted scalar normalizer is type-exact and does not dispatch through
+ application hooks. Vega data retains the normalized Chart Data wire value
+ (including epoch-ms temporal numbers); only the derived category key and
+ ASCII label use the JavaScript-compatible text.
+ """
+ from superset.mcp_service.chart.query_result import (
+ _bounded_utf8_length,
+ _chart_data_duration_text,
+ _chart_data_temporal_number,
+ _is_chart_data_duration_scalar,
+ _is_chart_data_temporal_scalar,
+ _normalize_trusted_scalar,
+ )
+
+ normalized: Any
+ reason: str | None
+ if _is_chart_data_temporal_scalar(value):
+ normalized, reason = _chart_data_temporal_number(value)
+ elif _is_chart_data_duration_scalar(value):
+ normalized, reason = _chart_data_duration_text(value)
+ else:
+ normalized, reason = _normalize_trusted_scalar(
+ value, max_string_bytes=_MAX_BULLET_TEXT_BYTES
+ )
+ if reason is not None:
+ if reason == "contains an unsupported or subclassed value":
+ reason = "has an unsupported value type"
+ elif "oversized string" in reason:
+ reason = "exceeds the size limit"
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} {reason}"
+ )
+
+ value_type = type(normalized)
+ if normalized is None:
+ text = "null"
+ elif value_type is str:
+ text = normalized
+ elif value_type is bool:
+ text = "true" if normalized else "false"
+ elif value_type is int or value_type is float or value_type is Decimal:
+ text = _javascript_number_string(normalized)
+ else:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} has an "
+ "unsupported value type"
+ )
+
+ if _bounded_utf8_length(text, _MAX_BULLET_TEXT_BYTES) is None:
+ raise BulletOutputError(
+ f"Bullet dimension {dimension!r} row {row_index} exceeds the size
limit"
+ )
+ return normalized, text
+
+
+def _bullet_number(value: Any, row_index: int, metric_field: str) -> float:
+ """Apply the frontend's useful ``Number(value ?? 0)`` numeric subset."""
+ value = _safe_enum_backing(value)
+ if value is None:
+ number = 0.0
+ elif type(value) is bool:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} returned a
boolean"
+ )
+ elif type(value) is int or type(value) is float or type(value) is Decimal:
+ try:
+ number = float(value)
+ except (TypeError, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ ) from ex
+ elif type(value) is str:
+ if len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ )
+ stripped = value.strip()
+ if not stripped:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not
numeric"
+ )
+ try:
+ number = float(Decimal(stripped))
+ except (InvalidOperation, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} returned "
+ f"non-numeric text"
+ ) from ex
+ else:
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is not numeric"
+ )
+ if not math.isfinite(number):
+ raise BulletOutputError(
+ f"Bullet metric {metric_field!r} row {row_index} is NaN or
infinite"
+ )
+ return number
+
+
+def _bullet_string_tokens(value: Any) -> list[str]:
+ """Parse labels exactly like the frontend's comma tokenizer."""
+ from superset.mcp_service.chart.query_result import _truncate_utf8
+
+ value = _safe_enum_backing(value)
+ if value is None:
+ return []
+ if type(value) is not str or len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError("Bullet labels must be a bounded
comma-separated list")
+ if not value.strip():
+ return []
+ tokens = value.split(",")
+ if len(tokens) > _MAX_BULLET_TOKENS:
+ raise BulletOutputError("Bullet labels exceed the item limit")
+ return [_truncate_utf8(token.strip(), _MAX_BULLET_TEXT_BYTES) for token in
tokens]
+
+
+def _unique_bullet_derived_field(
+ rows: list[dict[str, Any]], base: str, reserved: tuple[str, ...] = ()
+) -> str:
+ """Return one internal key absent from result rows and prior derived
keys."""
+ occupied = {key for row in rows for key in dict.keys(row)}
+ occupied.update(reserved)
+ candidate = base
+ suffix = 0
+ while candidate in occupied:
+ suffix += 1
+ candidate = f"{base}_{suffix}"
+ return candidate
+
+
+def _unique_bullet_category_field(rows: list[dict[str, Any]]) -> str:
+ """Return an internal category key absent from every query-result row."""
+ return _unique_bullet_derived_field(rows, "__mcp_bullet_category")
+
+
+def _validate_bullet_format(format_: Any, values: list[float]) -> str:
+ """Reject a presentation format the backend cannot reproduce."""
+ format_ = _safe_enum_backing(format_)
+ if format_ is None or format_ == "":
+ format_ = "SMART_NUMBER"
+ if type(format_) is not str or len(format_) > 50:
+ raise BulletOutputError(
+ "Bullet number format is unsupported by previews",
+ error_type="UnsupportedFormat",
+ )
+ try:
+ for value in values:
+ _format_bullet_number(format_, value)
+ except (TypeError, ValueError, OverflowError) as ex:
+ raise BulletOutputError(
+ f"Bullet number format {format_!r} is unsupported by previews",
+ error_type="UnsupportedFormat",
+ ) from ex
+ return format_
+
+
+def _format_bullet_number(format_: str, value: float) -> str:
+ """Format finite Bullet values, including the full binary-float range."""
+ from superset.utils.number_format import format_numeric
+
+ try:
+ return format_numeric(format_, value)
+ except OverflowError:
+ # SMART_NUMBER's significant-digit rounding can overflow a finite float
+ # near DBL_MAX. Scientific repr remains deterministic and informative.
+ if format_ in {"SMART_NUMBER", "SMART_NUMBER_SIGNED"} and
math.isfinite(value):
+ prefix = "+" if format_ == "SMART_NUMBER_SIGNED" and value > 0
else ""
+ return prefix + repr(value)
+ raise
+
+
+def _containing_bullet_range_label(
+ measure: float, ranges: list[float], labels: list[str]
+) -> str | None:
+ """Match the frontend's labelled containing-range tooltip selection."""
+ ascending = sorted(zip(ranges, labels, strict=True), key=lambda entry:
entry[0])
+ for threshold, label in ascending:
+ if measure <= threshold:
+ return label or None
+ if ascending and ascending[-1][1]:
+ return f"> {ascending[-1][1]}"
+ return None
+
+
+def resolve_bullet_render_model( # noqa: C901
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> BulletRenderModel:
+ """Resolve and validate every Bullet row and presentation control."""
+ if type(data) is not list:
+ raise BulletOutputError("Bullet query output must be an array of
objects")
+ for row_index in range(list.__len__(data)):
+ row = list.__getitem__(data, row_index)
+ if type(row) is not dict:
+ raise BulletOutputError("Bullet query output must be an array of
objects")
+ if dict.__len__(row) > _MAX_BULLET_FIELDS:
+ raise BulletOutputError("Bullet query row exceeds the field limit")
+ for key in dict.keys(row):
+ if type(key) is not str:
+ raise BulletOutputError("Bullet query row keys must be
strings")
+ if len(key) > _MAX_BULLET_FIELD_BYTES:
+ raise BulletOutputError("Bullet query row key exceeds the size
limit")
+
+ if type(form_data) is not dict:
+ raise BulletOutputError("Bullet form data must be an object")
+
+ metric_label = _form_metric_label(dict.get(form_data, "metric"))
+ if not metric_label:
+ raise BulletOutputError("Bullet metric has no declared result alias")
+ raw_groupby = dict.get(form_data, "groupby")
+ if raw_groupby is None:
+ raw_groupby = []
+ if type(raw_groupby) is not list:
+ raise BulletOutputError("Bullet dimensions must be an array")
+ dimension_labels = [
+ _form_column_label(list.__getitem__(raw_groupby, index))
+ for index in range(list.__len__(raw_groupby))
+ ]
+ if any(not label for label in dimension_labels):
+ raise BulletOutputError("Bullet dimension has no declared result
alias")
+
+ if data:
+ first_row = list.__getitem__(data, 0)
+ metric_field = _require_result_field(metric_label, first_row, "metric")
+ dimensions = [
+ _require_result_field(label, first_row, "dimension")
+ for label in dimension_labels
+ ]
+ else:
+ # The frontend accepts empty results. Ungrouped charts retain one
+ # zero-valued measure; grouped charts retain the declared roles but no
+ # categories or rows are fabricated.
+ metric_field = metric_label
+ dimensions = [label for label in dimension_labels if label is not None]
+
+ measures: list[float] = []
+ copied_rows: list[dict[str, Any]] = []
+ for index in range(list.__len__(data)):
+ row = list.__getitem__(data, index)
+ row_metric_field = _require_result_field(
+ metric_label, row, f"metric row {index}"
+ )
+ measure = _bullet_number(
+ dict.__getitem__(row, row_metric_field), index, metric_field
+ )
+ # Reserve every exact output key so the internal Vega category alias
+ # cannot collide with an unselected result field. Unselected values are
+ # deliberately replaced with None rather than converted or serialized.
+ copied: dict[str, Any] = dict.fromkeys(dict.keys(row))
+ copied[metric_field] = measure
+ for label, dimension in zip(dimension_labels, dimensions, strict=True):
+ row_dimension = _require_result_field(label, row, f"dimension row
{index}")
+ dimension_value, _ = _bullet_category_value(
+ dict.__getitem__(row, row_dimension), dimension, index
+ )
+ copied[dimension] = dimension_value
+ copied_rows.append(copied)
+ measures.append(measure)
+
+ # The frontend validates/coerces the whole result array but renders only
+ # the first row for an ungrouped aggregate.
+ if not dimensions:
+ copied_rows = copied_rows[:1]
+ measures = measures[:1]
+ if not copied_rows:
+ copied_rows = [{metric_field: 0.0}]
+ measures = [0.0]
+
+ ranges = _strict_bullet_numeric_tokens(dict.get(form_data, "ranges"),
"ranges")
+ if not ranges:
+ # Match Bullet/transformProps.ts: the largest measure drives one
+ # qualitative band whose upper threshold is 110% of that measure.
+ ranges = [0.0, max(measures, default=0.0) * 1.1]
+ markers = _strict_bullet_numeric_tokens(dict.get(form_data, "markers"),
"markers")
+ marker_lines = _strict_bullet_numeric_tokens(
+ dict.get(form_data, "marker_lines"), "marker lines"
+ )
+ all_numbers = [*measures, *ranges, *markers, *marker_lines]
+ if any(not math.isfinite(value) for value in all_numbers):
+ raise BulletOutputError("Bullet presentation values must be finite")
+
+ show_labels = dict.get(form_data, "show_labels", False)
+ show_legend = dict.get(form_data, "show_legend", False)
+ if type(show_labels) is not bool or type(show_legend) is not bool:
+ raise BulletOutputError("Bullet label and legend controls must be
booleans")
+
+ range_labels = _bullet_string_tokens(dict.get(form_data, "range_labels"))
+ marker_labels = _bullet_string_tokens(dict.get(form_data, "marker_labels"))
+ marker_line_labels = _bullet_string_tokens(
+ dict.get(form_data, "marker_line_labels")
+ )
+ for role, labels, values in (
+ ("range", range_labels, ranges),
+ ("marker", marker_labels, markers),
+ ("marker line", marker_line_labels, marker_lines),
+ ):
+ if labels and len(labels) != len(values):
Review Comment:
Fixed in 1e3150c8ae8d8cb5ed1ace38de6159cf7ac2558c. Removed label/value
cardinality rejection from both typed validation and the render model.
Missing/empty range labels stay blank; marker and marker-line labels fall back
by original index to the formatted number. The containing-range tooltip also
handles short lists, and sorted bands retain their own original labels. Extra
labels are ignored, matching Explore.
Evidence:
test_bullet_short_labels_and_case_distinct_dimensions_reach_fastmcp exercises
real saved and cached get_chart_preview calls in both ASCII and Vega-Lite (4
passing cases), including ranges 10,20,30 with only Low and numeric marker/line
fallbacks. The saved update regression preserves a short label list; the
frontend transformProps parity regression pins the same behavior. Focused
backend: 450 passed; frontend: 9 suites / 122 passed; branch-wide pre-commit
passed. Normal push completed; required exact-head CI is pending.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2014,6 +2099,462 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class BulletChartConfig(BaseChartConfig):
+ """Typed contract for the ECharts Bullet visualization (viz_type
``bullet``).
+
+ Semantic field names are exposed to MCP clients while validation aliases
and
+ the native adapter accept saved Explore ``form_data`` without weakening the
+ unknown-field checks that catch misspelled controls.
+ """
+
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["bullet"] = "bullet"
+ metric: ColumnRef = Field(
+ ...,
+ description=(
+ "Numeric measure shown by each bullet bar. Use aggregate for a
SIMPLE "
+ "metric, saved_metric=True for a dataset metric, or sql_expression
"
+ "with a unique label."
+ ),
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ validation_alias=AliasChoices("dimensions", "groupby"),
+ description=(
+ "Optional category hierarchy; the frontend renders one bullet row
per "
+ "unique combination (native form_data: groupby). Omit to preserve
a "
+ "saved hierarchy on update; pass [] to clear it."
+ ),
+ max_length=20,
+ )
+ filters: List[FilterConfig] | None = Field(
+ None,
+ description=(
+ "Structured WHERE filters. Native SIMPLE adhoc_filters are
accepted; "
+ "free-form SQL filters are rejected."
+ ),
+ max_length=100,
+ )
+ time_range: str | None = Field(
+ None,
+ min_length=1,
+ max_length=1000,
+ description=(
+ "Optional Superset time range such as 'Last 30 days' or "
+ "'2025-01-01 : 2025-12-31'. Set temporal_column to choose its
column."
+ ),
+ )
+ row_limit: int = Field(
+ 10000,
+ ge=1,
+ le=50000,
+ description="Maximum grouped bullet rows returned by the query",
+ )
+ order_by: List[SortByConfig] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("order_by", "orderby", "order_by_cols"),
+ max_length=20,
+ description=(
+ "Stable row ordering by a dimension name or by the metric's output
"
+ "label/name. Native orderby pairs and order_by_cols JSON pairs are
"
+ "accepted for saved-form-data round trips."
+ ),
+ )
+
+ # Presentation fields map one-for-one onto Bullet/transformProps.ts
controls.
+ ranges: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Qualitative range thresholds shaded behind the measure",
+ )
+ range_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("range_labels", "rangeLabels"),
+ max_length=100,
+ )
+ markers: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Target values drawn as point markers",
+ )
+ marker_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_labels", "markerLabels"),
+ max_length=100,
+ )
+ marker_lines: List[float] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_lines", "markerLines"),
+ max_length=100,
+ description="Reference values drawn as vertical lines",
+ )
+ marker_line_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_line_labels",
"markerLineLabels"),
+ max_length=100,
+ )
+ y_axis_format: str = Field(
+ "SMART_NUMBER",
+ validation_alias=AliasChoices("y_axis_format", "yAxisFormat"),
+ max_length=100,
+ )
+ show_labels: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_labels", "showLabels"),
+ )
+ show_legend: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_legend", "showLegend"),
+ )
+
+ @staticmethod
+ def _adapt_native_metric(value: Any) -> Any:
+ """Translate QueryFormMetric shapes into the shared ColumnRef
contract."""
+ if isinstance(value, str):
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict):
+ return value
+ if "expressionType" not in value:
+ # QueryObject's documented legacy saved-metric representation is a
+ # label-only object. Keep this adapter deliberately narrow: objects
+ # carrying ad-hoc fields must declare expressionType explicitly,
and
+ # semantic ColumnRef objects continue through normal validation.
+ if set(value) == {"label"}:
+ label = value["label"]
+ if not isinstance(label, str) or not label or len(label) > 255:
+ raise ValueError(
+ "legacy saved metric label must be a non-empty string
of "
+ "at most 255 characters"
+ )
+ return {"name": label, "saved_metric": True}
+ return value
+ expression_type = value.get("expressionType")
+ if expression_type == "SQL":
+ return {
+ "sql_expression": value.get("sqlExpression"),
+ "label": value.get("label"),
+ }
+ if expression_type != "SIMPLE":
+ raise ValueError("metric.expressionType must be 'SIMPLE' or 'SQL'")
+ column = value.get("column")
+ if isinstance(column, dict):
+ name = column.get("column_name")
+ else:
+ name = column
+ return {
+ "name": name,
+ "aggregate": value.get("aggregate"),
+ "label": value.get("label"),
+ }
+
+ @staticmethod
+ def _canonical_dimension_alias(value: Any, field_name: str) -> list[str]:
+ """Canonicalize semantic/native dimension aliases for conflict
checks."""
+ if not isinstance(value, list):
+ raise ValueError(f"{field_name} must be an array")
+ canonical: list[str] = []
+ for index, item in enumerate(value):
+ name: str | None
+ if isinstance(item, str):
+ name = item
+ elif isinstance(item, ColumnRef):
+ name = item.name
+ elif isinstance(item, dict):
+ name = next(
+ (
+ item[key]
+ for key in ("name", "column_name", "column")
+ if isinstance(item.get(key), str)
+ ),
+ None,
+ )
+ else:
+ name = None
+ if not name:
+ raise ValueError(
+ f"{field_name}[{index}] must identify a physical column"
+ )
+ canonical.append(name)
+ return canonical
+
+ @staticmethod
+ def _adapt_native_order_by(value: Any) -> Any: # noqa: C901
+ if value is None:
+ return []
+ if not isinstance(value, list):
+ raise ValueError("order_by must be an array")
+ result: list[Any] = []
+ for index, entry in enumerate(value):
+ if isinstance(entry, str):
+ if len(entry) > 2000:
+ raise ValueError(f"order_by[{index}] is too long")
+ try:
+ entry = json.loads(entry)
+ except json.JSONDecodeError:
+ # A bare output/column name is the ergonomic typed form.
+ result.append({"column": entry, "ascending": False})
+ continue
+ if isinstance(entry, dict):
+ result.append(entry)
+ continue
+ if not isinstance(entry, (list, tuple)) or len(entry) != 2:
+ raise ValueError(
+ f"order_by[{index}] must be [column, ascending_boolean]"
+ )
+ target, ascending = entry
+ if isinstance(target, dict):
+ target = target.get("label") or target.get("metric_name")
+ if not isinstance(target, str) or not target:
+ raise ValueError(f"order_by[{index}] needs a column or metric
label")
+ if not isinstance(ascending, bool):
+ raise ValueError(f"order_by[{index}] ascending value must be
boolean")
+ result.append({"column": target, "ascending": ascending})
+ return result
+
+ @staticmethod
+ def _adapt_native_filters(data: dict[str, Any]) -> None: # noqa: C901
+ if "adhoc_filters" not in data:
+ return
+ if "filters" in data:
+ raise ValueError("Use either filters or native adhoc_filters, not
both")
+ raw_filters = data.pop("adhoc_filters")
+ if not isinstance(raw_filters, list):
+ raise ValueError("adhoc_filters must be an array")
+ filters: list[dict[str, Any]] = []
+ for index, raw_filter in enumerate(raw_filters):
+ if not isinstance(raw_filter, dict):
+ raise ValueError(f"adhoc_filters[{index}] must be an object")
+ if raw_filter.get("expressionType") != "SIMPLE":
+ raise ValueError(
+ f"adhoc_filters[{index}] must use expressionType='SIMPLE'"
+ )
+ if raw_filter.get("clause") not in (None, "WHERE"):
+ raise ValueError(f"adhoc_filters[{index}] must use
clause='WHERE'")
+ subject = raw_filter.get("subject")
+ operator = raw_filter.get("operator")
+ comparator = raw_filter.get("comparator")
+ if operator == "TEMPORAL_RANGE":
+ if not isinstance(subject, str) or not subject:
+ raise ValueError(
+ f"adhoc_filters[{index}] temporal filter needs subject"
+ )
+ data.setdefault("temporal_column", subject)
+ if isinstance(comparator, str) and comparator.casefold() !=
"no filter":
+ data.setdefault("time_range", comparator)
+ continue
+ if not isinstance(operator, str):
+ raise ValueError(f"adhoc_filters[{index}] needs an operator")
+ operator_map = {
+ "==": "=",
+ "EQUALS": "=",
+ "NOT_EQUALS": "!=",
+ "LESS_THAN": "<",
+ "LESS_THAN_OR_EQUAL": "<=",
+ "GREATER_THAN": ">",
+ "GREATER_THAN_OR_EQUAL": ">=",
+ "NOT_IN": "NOT IN",
+ "IS_NULL": "IS NULL",
+ "IS_NOT_NULL": "IS NOT NULL",
+ }
+ operator = operator_map.get(operator, operator)
+ filters.append({"column": subject, "op": operator, "value":
comparator})
+ data["filters"] = filters
+
+ @model_validator(mode="before")
+ @classmethod
+ def adapt_native_form_data(cls, raw: Any) -> Any: # noqa: C901
+ """Accept recognized saved Bullet form_data and reject ambiguous
state."""
+ if not isinstance(raw, dict):
+ return raw
+ data = dict(raw)
+ if "dimensions" in data and "groupby" in data:
+ dimensions = cls._canonical_dimension_alias(
+ data["dimensions"], "dimensions"
+ )
+ groupby = cls._canonical_dimension_alias(data["groupby"],
"groupby")
+ if dimensions != groupby:
+ raise ValueError(
+ "Conflicting Bullet dimension aliases: 'dimensions' and "
+ "native 'groupby' must identify the same physical columns
in "
+ "the same order; provide only one or make them equivalent"
+ )
+ # Avoid relying on AliasChoices precedence or JSON key order.
+ data.pop("groupby")
+ if data.get("viz_type") == "bullet":
+ data.setdefault("chart_type", "bullet")
+ data.pop("viz_type", None)
+ for key in (
+ "annotation_layers",
+ "dashboards",
+ "datasource",
+ "datasource_id",
+ "datasource_type",
+ "extra_form_data",
+ "slice_id",
+ "slice_name",
+ ):
+ data.pop(key, None)
+
+ if (marker_key := "_mcp_dashboard_time_filter_subject") in data:
+ marker = data.pop(marker_key)
+ if not isinstance(marker, str) or not marker:
+ raise ValueError(f"{marker_key} must be a physical column
name")
+ raw_filters = data.get("adhoc_filters")
+ if not isinstance(raw_filters, list):
+ raise ValueError(
+ f"{marker_key} requires an adhoc_filters array containing
its "
+ "generated binding"
+ )
+ provenance_matches = [
+ filter_
+ for filter_ in raw_filters
+ if isinstance(filter_, dict)
+ and filter_.get("subject") == marker
+ and filter_.get("operator") == "TEMPORAL_RANGE"
+ ]
+ if len(provenance_matches) != 1:
+ raise ValueError(
+ f"{marker_key} must match exactly one TEMPORAL_RANGE
filter "
+ f"for subject {marker!r}; found {len(provenance_matches)}"
+ )
+ data.setdefault("temporal_column", marker)
+
+ if "metric" in data:
+ data["metric"] = cls._adapt_native_metric(data["metric"])
+ for key in ("groupby", "dimensions"):
+ if key in data:
+ if not isinstance(data[key], list):
+ raise ValueError(f"{key} must be an array")
+ data[key] = [
+ {"name": item} if isinstance(item, str) else item
+ for item in data[key]
+ ]
+ for key in ("order_by", "orderby", "order_by_cols"):
+ if key in data:
+ data[key] = cls._adapt_native_order_by(data[key])
+ cls._adapt_native_filters(data)
+ return data
+
+ @field_validator("ranges", "markers", "marker_lines", mode="before")
+ @classmethod
+ def tokenize_native_numeric_lists(cls, value: Any) -> Any:
+ """Parse numeric controls without creating values for empty tokens."""
+ if value is None:
+ return []
+ if isinstance(value, str):
+ return [token.strip() for token in value.split(",") if
token.strip()]
+ return value
+
+ @field_validator(
+ "range_labels", "marker_labels", "marker_line_labels", mode="before"
+ )
+ @classmethod
+ def tokenize_native_label_lists(cls, value: Any) -> Any:
+ """Parse label controls while preserving positional empty tokens."""
+ if value is None or value == "":
+ return []
+ if isinstance(value, str):
+ return [token.strip() for token in value.split(",")]
+ return value
+
+ @field_validator("ranges", "markers", "marker_lines")
+ @classmethod
+ def reject_non_finite_values(cls, values: List[float]) -> List[float]:
+ if any(not math.isfinite(value) for value in values):
+ raise ValueError("Bullet thresholds and markers must be finite
numbers")
+ return values
+
+ @field_validator("range_labels", "marker_labels", "marker_line_labels")
+ @classmethod
+ def validate_presentation_labels(cls, labels: List[str]) -> List[str]:
+ result: list[str] = []
+ for label in labels:
+ if "," in label:
+ raise ValueError(
+ "Bullet labels cannot contain commas because the frontend "
+ "comma-separated controls have no escaping"
+ )
+ if label == "":
+ result.append("")
+ continue
+ sanitized = sanitize_user_input(
+ label, "Bullet label", max_length=200, allow_empty=True
+ )
+ if sanitized is not None:
+ result.append(sanitized)
+ return result
+
+ @field_validator("time_range")
+ @classmethod
+ def sanitize_time_range(cls, value: str | None) -> str | None:
+ return sanitize_user_input(
+ value, "Time range", max_length=1000, allow_empty=True
+ )
+
+ @model_validator(mode="after")
+ def validate_roles_and_outputs(self) -> "BulletChartConfig": # noqa: C901
+ dimensions = self.dimensions or []
+ seen_names: set[str] = set()
+ for index, dimension in enumerate(dimensions):
+ _reject_sql_expression_on_dimension(dimension,
f"dimensions[{index}]")
+ if dimension.saved_metric or dimension.aggregate:
+ raise ValueError(
+ f"dimensions[{index}] must be a physical dimension, not a
metric"
+ )
+ name = dimension.name or ""
+ if name.casefold() in seen_names:
Review Comment:
Fixed in 1e3150c8ae8d8cb5ed1ace38de6159cf7ac2558c. Role/output collision
checks use exact physical names, preserving Region and region (and a
case-distinct metric alias). Dataset resolution still prefers exact names and
rejects ambiguous non-exact lookup. Sort normalization resolves the original
output role before canonicalization instead of storing case-folded dictionary
keys that could overwrite one another.
Evidence: test_bullet_exact_case_dimensions_survive_normalization_and_query
asserts both query columns, distinct sort targets, distinct result keys, and
ambiguous lookup rejection.
test_bullet_short_labels_and_case_distinct_dimensions_reach_fastmcp exercises
both fields through saved/cached public previews in ASCII and Vega-Lite; the
frontend transformProps regression verifies exact-key grouping. Exact
duplicates still fail. Focused backend: 450 passed; frontend: 9 suites / 122
passed; branch-wide pre-commit passed. Normal push completed; required
exact-head CI is pending.
--
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]
