gabotorresruiz commented on code in PR #44948:
URL: https://github.com/apache/superset/pull/44948#discussion_r4190166592


##########
superset/mcp_service/chart/big_number_headline.py:
##########
@@ -0,0 +1,379 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Headline value of Big Number charts.
+
+A Big Number chart renders one number. For the trendline variant 
(``big_number``)
+that number is derived client-side from the time series; for 
``big_number_total``
+it is the single metric value. Neither is any of the first sample rows, so this
+module reproduces the frontend computation:
+
+- ``aggregationChoices`` in ``superset-ui-chart-controls`` 
(``customControls.tsx``)
+- ``BigNumberWithTrendline/transformProps.ts`` and 
``BigNumberTotal/transformProps.ts``
+
+A headline is only returned when it is exact. Otherwise ``value`` is null with 
a
+``reason``: a wrong number is worse than none.
+"""
+
+from __future__ import annotations
+
+import math
+import statistics
+from collections.abc import Callable, Mapping, Sequence
+from datetime import date, datetime, timezone
+from decimal import Decimal
+from typing import Any, cast
+
+from superset.mcp_service.chart.schemas import BigNumberHeadline
+from superset.superset_typing import Metric
+from superset.utils.core import DTTM_ALIAS, get_metric_name
+
+BIG_NUMBER_TRENDLINE_VIZ_TYPE = "big_number"
+BIG_NUMBER_TOTAL_VIZ_TYPE = "big_number_total"
+
+DEFAULT_AGGREGATION = "LAST_VALUE"
+RAW_AGGREGATION = "raw"
+
+# Metric-value transforms for the trend series. Keys and order mirror the
+# frontend's `aggregationChoices`. `LAST_VALUE` and `raw` receive values 
ordered
+# newest first and take the first, so they need no entry beyond that.
+_AGGREGATIONS: dict[str, Callable[[list[float]], float | None]] = {
+    "raw": lambda values: values[0] if values else None,
+    "LAST_VALUE": lambda values: values[0] if values else None,
+    "sum": lambda values: sum(values) if values else None,
+    "mean": lambda values: sum(values) / len(values) if values else None,
+    "min": lambda values: min(values) if values else None,
+    "max": lambda values: max(values) if values else None,
+    "median": lambda values: statistics.median(values) if values else None,
+}
+
+# Rolling types that make the query add a `rolling` (sum, mean, std) or `cum`
+# (cumsum) post-processing step, as in the frontend's `rollingWindowOperator`.
+_ROLLING_OPERATIONS = {
+    "cumsum": "cum",
+    "sum": "rolling",
+    "mean": "rolling",
+    "std": "rolling",
+}
+
+
+def is_big_number_viz_type(viz_type: str | None) -> bool:
+    """Whether the visualization displays a Big Number headline."""
+    return viz_type in (BIG_NUMBER_TRENDLINE_VIZ_TYPE, 
BIG_NUMBER_TOTAL_VIZ_TYPE)
+
+
+def executed_query_facts(query_context: Any) -> tuple[int | None, list[str]]:
+    """Row limit and post-processing operations of the first executed query.
+
+    Read from the query that actually ran, so it reflects any row-limit 
override
+    and shows whether the chart's advanced analytics were part of the query.
+    """
+    queries = getattr(query_context, "queries", None) or []
+    if not queries:
+        return None, []
+    first = queries[0]
+    row_limit = getattr(first, "row_limit", None)
+    operations = [
+        str(step.get("operation"))
+        for step in getattr(first, "post_processing", None) or []
+        if isinstance(step, Mapping) and step.get("operation")
+    ]
+    return (row_limit if isinstance(row_limit, int) else None), operations
+
+
+def _unavailable(aggregation: str | None, reason: str) -> BigNumberHeadline:
+    """Return an unavailable headline with its explanation."""
+    return BigNumberHeadline(value=None, aggregation=aggregation, 
reason=reason)
+
+
+def _parse_date_ms(value: str) -> int | None:
+    """Epoch milliseconds for an ISO-8601 string, else None (frontend:
+    strict `dayjs.utc` parse in `parseMetricValue`)."""
+    try:
+        parsed = datetime.fromisoformat(value)
+    except ValueError:
+        return None
+    if parsed.tzinfo is None:
+        parsed = parsed.replace(tzinfo=timezone.utc)
+    return int(parsed.timestamp() * 1000)
+
+
+def _parse_metric_value(value: Any) -> int | float | None:
+    """Mirror of the frontend's `parseMetricValue`, plus JSON-safety: numbers
+    pass through, date strings become epoch ms, anything else (including NaN 
and
+    infinities, which serialize as null) is null."""
+    if isinstance(value, bool) or value is None:
+        return None
+    if isinstance(value, Decimal):
+        value = float(value)
+    if isinstance(value, (int, float)):
+        try:
+            return value if math.isfinite(value) else None
+        except OverflowError:
+            return None
+    if isinstance(value, datetime):
+        if value.tzinfo is None:
+            value = value.replace(tzinfo=timezone.utc)
+        return int(value.timestamp() * 1000)
+    if isinstance(value, str):
+        return _parse_date_ms(value)
+    return None
+
+
+def _timestamp_ms(value: Any) -> float | None:
+    """Sortable time value for an x-axis cell (the API serializes these as 
epoch
+    ms; direct query results may carry datetimes)."""
+    if isinstance(value, date) and not isinstance(value, datetime):
+        value = datetime(value.year, value.month, value.day, 
tzinfo=timezone.utc)
+    return _parse_metric_value(value)
+
+
+def _metric_label(form_data: Mapping[str, Any]) -> str | None:
+    """Resolve the metric label using the frontend column-name aliases."""
+    metric = form_data.get("metric")
+    if not metric:
+        return None
+    if (
+        isinstance(metric, Mapping)
+        and not metric.get("label")
+        and metric.get("expressionType") == "SIMPLE"
+    ):
+        column = metric.get("column")
+        if isinstance(column, Mapping) and column.get("columnName"):
+            return f"{metric.get('aggregate')}({column['columnName']})"
+    try:
+        return get_metric_name(cast(Metric, metric)) or None
+    except (ValueError, TypeError, AttributeError):
+        return None
+
+
+def _x_axis_labels(form_data: Mapping[str, Any]) -> list[str]:
+    """Candidate time-column labels: the configured x-axis, then 
`__timestamp`."""
+    labels: list[str] = []
+    x_axis = form_data.get("x_axis")
+    if isinstance(x_axis, str) and x_axis:
+        labels.append(x_axis)
+    elif isinstance(x_axis, Mapping):
+        label = x_axis.get("label") or x_axis.get("column_name")
+        if isinstance(label, str) and label:
+            labels.append(label)
+    labels.append(DTTM_ALIAS)

Review Comment:
   Not a blocker, and maybe not even reachable, so mostly a question. 
`_x_axis_labels` resolves the time column from `x_axis` and then `__timestamp`, 
but the query these rows come from resolves it with 
`resolve_big_number_columns` (`chart_helpers.py:561`), which also accepts 
`granularity_sqla` when `x_axis` is absent and returns that column. 
`build_single_query_dict` sets no `is_timeseries`, so the result column carries 
the granularity column's name rather than `__timestamp`, and the lookup here 
finds nothing. With `form_data = {"metric": "SUM(sales)", "granularity_sqla": 
"ds"}` and rows keyed `ds` at 1000/2000/3000 I get `value` null with the 
undated-rows reason, while the identical rows keyed `__timestamp` give `30`.
   
   It declines rather than answering wrong, so this costs a headline rather 
than being incorrect, but the chart does render one in that case: 
`getXAxisColumn` falls back to `DTTM_ALIAS` for exactly that form data, so the 
frontend reads `__timestamp` and shows the newest value. Appending the 
`granularity_sqla` column to the candidates here would close it. Are 
`granularity_sqla`-only Big Numbers still a thing in saved `params`, or does 
the x-axis migration always backfill `x_axis`?



-- 
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]

Reply via email to