bito-code-review[bot] commented on code in PR #38403:
URL: https://github.com/apache/superset/pull/38403#discussion_r2931890813


##########
superset/mcp_service/chart/validation/schema_validator.py:
##########
@@ -282,6 +286,60 @@ def _pre_validate_pie_config(
 
         return True, None
 
+    @staticmethod
+    def _pre_validate_big_number_config(
+        config: Dict[str, Any],
+    ) -> Tuple[bool, ChartGenerationError | None]:
+        """Pre-validate big number chart configuration."""
+        if "metric" not in config:
+            return False, ChartGenerationError(
+                error_type="missing_metric",
+                message="Big Number chart missing required field: metric",
+                details="Big Number charts require a 'metric' field "
+                "specifying the value to display",
+                suggestions=[
+                    "Add 'metric' with name and aggregate: "
+                    "{'name': 'revenue', 'aggregate': 'SUM'}",
+                    "The aggregate function is required (SUM, COUNT, AVG, MIN, 
MAX)",
+                    "Example: {'chart_type': 'big_number', "
+                    "'metric': {'name': 'sales', 'aggregate': 'SUM'}}",
+                ],
+                error_code="MISSING_BIG_NUMBER_METRIC",
+            )
+
+        metric = config.get("metric", {})
+        if isinstance(metric, dict) and not metric.get("aggregate"):
+            return False, ChartGenerationError(
+                error_type="missing_metric_aggregate",
+                message="Big Number metric must include an aggregate function",
+                details="The metric 'aggregate' field is required "
+                "for Big Number charts",
+                suggestions=[
+                    "Add 'aggregate' to your metric: "
+                    "{'name': 'col', 'aggregate': 'SUM'}",
+                    "Valid aggregates: SUM, COUNT, AVG, MIN, MAX",
+                ],
+                error_code="MISSING_BIG_NUMBER_AGGREGATE",
+            )

Review Comment:
   <!-- Bito Reply -->
   This change enhances the `_pre_validate_big_number_config` function by 
adding type validation for the `metric` field, ensuring it's a dictionary 
before checking for the aggregate. It also validates that the aggregate value 
is one of the allowed functions (SUM, COUNT, AVG, MIN, MAX). This catches 
invalid inputs early, preventing them from reaching Pydantic and providing 
clearer error messages.



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