codeant-ai-for-open-source[bot] commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r3906292876
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -469,11 +567,126 @@ def _is_nan(value: Any) -> bool:
return False
+def _bullet_numeric_tokens(value: Any) -> list[float]:
+ """Parse native comma-separated Bullet threshold controls."""
+ if isinstance(value, str):
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif isinstance(value, list):
+ tokens = value
+ else:
+ return []
+ result: list[float] = []
+ for token in tokens:
+ try:
+ number = float(token)
+ except (TypeError, ValueError):
+ continue
+ if not _is_nan(number) and math.isfinite(number):
+ result.append(number)
+ return result
+
+
+def _generate_bullet_vega_lite_preview(
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> VegaLitePreview | None:
+ """Build a horizontal layered preview faithful to Bullet result roles."""
+ metric_field, dimensions = _bullet_result_roles(data, form_data)
+ if metric_field is None:
+ return None
+
+ category_field = "__mcp_bullet_category"
+ values = []
+ for index, row in enumerate(data):
+ copied = dict(row)
+ copied[category_field] = (
+ ", ".join(str(row.get(field, "")) for field in dimensions)
+ if dimensions
+ else str(index + 1)
+ )
+ values.append(copied)
+
+ y_encoding = {
+ "field": category_field,
+ "type": "nominal",
+ "title": ", ".join(dimensions) if dimensions else None,
+ "sort": None,
+ }
+ tooltip = [
+ *({"field": field, "type": "nominal"} for field in dimensions),
+ {"field": metric_field, "type": "quantitative"},
+ ]
+ layers: list[dict[str, Any]] = []
+ for index, threshold in enumerate(
+ sorted(_bullet_numeric_tokens(form_data.get("ranges")), reverse=True)
+ ):
+ layers.append(
+ {
+ "mark": {
+ "type": "rect",
+ "opacity": max(0.08, 0.28 - index * 0.04),
+ },
+ "encoding": {
+ "x": {"datum": 0, "type": "quantitative"},
+ "x2": {"datum": threshold},
Review Comment:
**Suggestion:** Range rectangles have no `y` encoding, so grouped Bullet
previews render threshold backgrounds across the chart instead of one
background band per category row. [logic error]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55dc227cbd314162883dc677be6e31af&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=55dc227cbd314162883dc677be6e31af&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/preview_utils.py
**Line:** 628:630
**Comment:**
*Logic Error: Range rectangles have no `y` encoding, so grouped Bullet
previews render threshold backgrounds across the chart instead of one
background band per category row.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=3a5c37f928269ffae84d050c2b06f74b097cbb4bec371e95de2a3da7a5ef1c99&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=3a5c37f928269ffae84d050c2b06f74b097cbb4bec371e95de2a3da7a5ef1c99&reaction=dislike'>๐</a>
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -323,6 +325,102 @@ 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 isinstance(metric, str):
+ return metric
+ if not isinstance(metric, dict):
+ return None
+ if label := metric.get("label"):
+ return label if isinstance(label, str) else None
+ column = metric.get("column")
+ column_name = column.get("column_name") if isinstance(column, dict) else
column
+ aggregate = metric.get("aggregate")
+ if isinstance(column_name, str) and isinstance(aggregate, 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 isinstance(column, str):
+ return column
+ if not isinstance(column, dict):
+ return None
+ for key in ("label", "column_name"):
+ if isinstance(value := column.get(key), str) and value:
+ return value
+ return None
+
+
+def _canonical_result_field(label: str | None, row: Dict[str, Any]) -> str |
None:
+ """Resolve a query result field by exact match, then unambiguous
casefold."""
+ if label is None:
+ return None
+ if label in row:
+ return label
+ matches = [field for field in row if field.casefold() == label.casefold()]
+ return matches[0] if len(matches) == 1 else None
+
+
+def _bullet_result_roles(
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> tuple[str | None, list[str]]:
+ """Resolve Bullet metric and full category hierarchy in query output."""
+ if not data:
+ return None, []
+ first_row = data[0]
+ metric_field = _canonical_result_field(
+ _form_metric_label(form_data.get("metric")), first_row
+ )
+ if metric_field is None:
+ metric_field = next(
+ (
+ field
+ for field, value in first_row.items()
+ if isinstance(value, (int, float)) and not _is_nan(value)
+ ),
+ None,
+ )
Review Comment:
**Suggestion:** When the metric label is absent from results, this fallback
selects the first numeric field, so a numeric dimension can be rendered as the
Bullet measure. [incorrect variable usage]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=85cb1d636aea49b6aa0862104d7ec879&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=85cb1d636aea49b6aa0862104d7ec879&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/preview_utils.py
**Line:** 373:384
**Comment:**
*Incorrect Variable Usage: When the metric label is absent from
results, this fallback selects the first numeric field, so a numeric dimension
can be rendered as the Bullet measure.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=4ec059a2b99b8cfff1f87add7d89f0a34538bd807e5bcf1436c29c19186047a2&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=4ec059a2b99b8cfff1f87add7d89f0a34538bd807e5bcf1436c29c19186047a2&reaction=dislike'>๐</a>
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2014,6 +2016,385 @@ 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):
+ legacy = re.fullmatch(
+ r"(sum|avg|min|max|count|count_distinct)__(.+)",
+ value,
+ flags=re.IGNORECASE,
+ )
+ if legacy:
+ return {
+ "name": legacy.group(2),
+ "aggregate": legacy.group(1).upper(),
+ }
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict) or "expressionType" not in value:
+ 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 _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 = {"==": "=", "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 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",
+ ):
+ data.pop(key, None)
+
+ marker_key = "_mcp_dashboard_time_filter_subject"
+ if marker := data.pop(marker_key, None):
+ if not isinstance(marker, str):
+ raise ValueError(f"{marker_key} must be a physical column
name")
+ 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]
Review Comment:
**Suggestion:** When both `dimensions` and `groupby` are present, the alias
silently chooses `dimensions`, so conflicting saved and semantic hierarchies
can update the chart with the wrong grouping. [api mismatch]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1e04d58b3a2449c0b643a921cd6b3f39&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1e04d58b3a2449c0b643a921cd6b3f39&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/schemas.py
**Line:** 2265:2271
**Comment:**
*Api Mismatch: When both `dimensions` and `groupby` are present, the
alias silently chooses `dimensions`, so conflicting saved and semantic
hierarchies can update the chart with the wrong grouping.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=639e0106c120088ec00b7ae4097f4e2bd110c3164101d99d5cece0803b1c0d7b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43770&comment_hash=639e0106c120088ec00b7ae4097f4e2bd110c3164101d99d5cece0803b1c0d7b&reaction=dislike'>๐</a>
--
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]
