sadpandajoe commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4243794597
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -3985,6 +4104,487 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class BulletChartConfig(BaseChartConfig):
+ """Config for bullet charts (viz_type ``bullet``)."""
+
+ # Semantic field names are exposed to MCP clients; validation aliases and
the
+ # native adapter accept saved Explore ``form_data`` without weakening the
+ # unknown-field checks that catch misspelled controls.
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ # Saved query roles are internal sort context, never typed authoring input.
+ # Keeping them out of dimensions preserves omission and native SQL shapes.
+ _inherited_groupby: list[str | dict[str, Any]] | None =
PrivateAttr(default=None)
+
+ @property
+ def order_dimensions(self) -> Sequence[ColumnRef | str | dict[str, Any]]:
+ """Return authored dimensions or the inherited native sort
hierarchy."""
+ if self.dimensions is None and self._inherited_groupby is not None:
+ return self._inherited_groupby
+ return self.dimensions or []
+
+ chart_type: Literal["bullet"] = "bullet"
+ metric: ColumnRef = Field(
+ ...,
+ description="Numeric measure shown by each bullet bar",
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ validation_alias=AliasChoices("dimensions", "groupby"),
+ description=(
+ "Category hierarchy; one bullet row per unique combination. Omit
to "
+ "keep the saved hierarchy on update; [] clears it."
+ ),
+ max_length=20,
+ )
+ filters: List[FilterConfig] | None = Field(None, max_length=100)
+ time_range: str | None = Field(
+ None,
+ min_length=1,
+ max_length=1000,
+ description=(
+ "Superset time range, e.g. 'Last 30 days' or '2025-01-01 :
2025-12-31'"
+ ),
+ )
+ row_limit: int = Field(
+ 10000,
+ ge=1,
+ le=50000,
+ description="Maximum bullet rows",
+ )
+ order_by: List[SortByConfig] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("order_by", "orderby", "order_by_cols"),
+ max_length=20,
+ description="Row order by a dimension name or the metric output
label/name",
+ )
+
+ # Presentation fields map one-for-one onto Bullet/transformProps.ts
controls.
+ ranges: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Qualitative range thresholds shaded behind the measure",
+ )
+ range_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("range_labels", "rangeLabels"),
+ max_length=100,
+ )
+ markers: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Target values drawn as point markers",
+ )
+ marker_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_labels", "markerLabels"),
+ max_length=100,
+ )
+ marker_lines: List[float] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_lines", "markerLines"),
+ max_length=100,
+ description="Reference values drawn as vertical lines",
+ )
+ marker_line_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_line_labels",
"markerLineLabels"),
+ max_length=100,
+ )
+ y_axis_format: str = Field(
+ "SMART_NUMBER",
+ validation_alias=AliasChoices("y_axis_format", "yAxisFormat"),
+ max_length=100,
+ )
+ show_labels: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_labels", "showLabels"),
+ )
+ show_legend: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_legend", "showLegend"),
+ )
+
+ @staticmethod
+ def _adapt_native_metric(value: Any) -> Any:
+ """Translate QueryFormMetric shapes into the shared ColumnRef
contract."""
+ if isinstance(value, str):
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict):
+ return value
+ if "expressionType" not in value:
+ # QueryObject's documented legacy saved-metric representation is a
+ # label-only object. Keep this adapter deliberately narrow: objects
+ # carrying ad-hoc fields must declare expressionType explicitly,
and
+ # semantic ColumnRef objects continue through normal validation.
+ if set(value) == {"label"}:
+ label = value["label"]
+ if not isinstance(label, str) or not label or len(label) > 255:
+ raise ValueError(
+ "legacy saved metric label must be a non-empty string
of "
+ "at most 255 characters"
+ )
+ return {"name": label, "saved_metric": True}
+ return value
+ expression_type = value.get("expressionType")
+ if expression_type == "SQL":
+ return {
+ "sql_expression": value.get("sqlExpression"),
+ "label": value.get("label") or value.get("sqlExpression"),
+ }
+ if expression_type != "SIMPLE":
+ raise ValueError("metric.expressionType must be 'SIMPLE' or 'SQL'")
+ column = value.get("column")
+ if isinstance(column, dict):
+ name = column.get("column_name")
+ else:
+ name = column
+ return {
+ "name": name,
+ "aggregate": value.get("aggregate"),
+ "label": value.get("label"),
+ }
+
+ @staticmethod
+ def _canonical_dimension_alias(value: Any, field_name: str) -> list[str]:
+ """Canonicalize semantic/native dimension aliases for conflict
checks."""
+ if not isinstance(value, list):
+ raise ValueError(f"{field_name} must be an array")
+ canonical: list[str] = []
+ for index, item in enumerate(value):
+ name: str | None
+ if isinstance(item, str):
+ name = item
+ elif isinstance(item, ColumnRef):
+ name = item.name
+ elif isinstance(item, dict):
+ name = next(
+ (
+ item[key]
+ for key in ("name", "column_name", "column")
+ if isinstance(item.get(key), str)
+ ),
+ None,
+ )
+ else:
+ name = None
+ if not name:
+ raise ValueError(
+ f"{field_name}[{index}] must identify a physical column"
+ )
+ canonical.append(name)
+ return canonical
+
+ @staticmethod
+ def _adapt_native_order_by(value: Any) -> Any: # noqa: C901
+ if value is None:
+ return []
+ if not isinstance(value, list):
+ raise ValueError("order_by must be an array")
+ result: list[Any] = []
+ for index, entry in enumerate(value):
+ if isinstance(entry, str):
+ if len(entry) > 2000:
+ raise ValueError(f"order_by[{index}] is too long")
+ try:
+ entry = json.loads(entry)
+ except json.JSONDecodeError:
+ # A bare output/column name is the ergonomic typed form.
+ result.append({"column": entry, "ascending": False})
+ continue
+ if isinstance(entry, dict):
+ result.append(entry)
+ continue
+ if not isinstance(entry, (list, tuple)) or len(entry) != 2:
+ raise ValueError(
+ f"order_by[{index}] must be [column, ascending_boolean]"
+ )
+ target, ascending = entry
+ if isinstance(target, dict):
+ target = target.get("label") or target.get("metric_name")
Review Comment:
`_adapt_native_metric` gives an unlabeled SQL metric the label
`sqlExpression`, but here only `label` or `metric_name` is read from a metric
object. A native Bullet with `metric: {expressionType: "SQL", sqlExpression:
"SUM(revenue)"}` and `orderby: [[<same metric object>, false]]` therefore hits
the "needs a column or metric label" error even though the metric itself
validates. Should this fall back to `sqlExpression` the same way the metric
adapter does?
--
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]
