aminghadersohi commented on code in PR #44684:
URL: https://github.com/apache/superset/pull/44684#discussion_r4113623978
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1257,6 +1257,32 @@ def generate_bubble_vega_lite_preview(
)
+def _resolve_y_metric_column(row: Dict[str, Any], metrics: List[Any]) -> Any:
+ """Pick the y-axis column for a Vega-Lite preview.
+
+ Prefers the first chart metric whose result label is present in the row.
+ Falls back to a name/value heuristic only when no metric label matches.
+ Booleans are never treated as numeric in the fallback.
+ """
+ for metric in metrics:
+ label = metric_result_label(metric)
+ if label is not None and label in row:
Review Comment:
Validated and fixed in 81ddcfdbc8def8d791c6dbb09474123fdc760e4d (included in
head 9bd69646d9fd9e1319309af7cc5b7947c6bdcb1b). Matched string/boolean metrics
are skipped; a later numeric metric is preferred, and an invalid-only match no
longer falls back to a numeric dimension. Null values remain eligible so a null
first row does not redirect a valid metric. Five parameterized regression cases
failed before the fix and pass after it. Targeted preview-utils suite: 25
passed; touched-file pre-commit, including mypy and pylint: passed.
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1257,6 +1257,32 @@ def generate_bubble_vega_lite_preview(
)
+def _resolve_y_metric_column(row: Dict[str, Any], metrics: List[Any]) -> Any:
+ """Pick the y-axis column for a Vega-Lite preview.
+
+ Prefers the first chart metric whose result label is present in the row.
+ Falls back to a name/value heuristic only when no metric label matches.
+ Booleans are never treated as numeric in the fallback.
+ """
+ for metric in metrics:
+ label = metric_result_label(metric)
+ if label is not None and label in row:
+ return label
+
+ for col in row.keys():
+ # Check if this is a metric column (usually has aggregation in name)
+ if any(
+ agg in str(col).upper()
+ for agg in ["SUM", "AVG", "COUNT", "MIN", "MAX", "TOTAL"]
+ ):
+ return col
Review Comment:
Validated and fixed in 81ddcfdbc8def8d791c6dbb09474123fdc760e4d (included in
head 9bd69646d9fd9e1319309af7cc5b7947c6bdcb1b). The fallback requires an actual
numeric, non-boolean value rather than accepting aggregation substrings in
column names. Regression cases for string summary and boolean maximum_status
dimensions both failed before the fix and pass after it. Targeted preview-utils
suite: 25 passed; touched-file pre-commit, including mypy and pylint: passed.
--
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]
