bito-code-review[bot] commented on code in PR #38403:
URL: https://github.com/apache/superset/pull/38403#discussion_r2930719416
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>Missing input validation in big number config</b></div>
<div id="fix">
The new _pre_validate_big_number_config method lacks validation for the
'metric' field type and 'aggregate' value validity. Without these checks,
invalid inputs like non-dict metrics or unsupported aggregates could pass
pre-validation, potentially causing runtime errors or incorrect chart behavior
during Pydantic validation or chart generation. Add isinstance checks and
aggregate validation for robustness.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- 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",
- )
-
+ metric = config.get("metric", {})
+ if not isinstance(metric, dict):
+ return False, ChartGenerationError(
+ error_type="invalid_metric_type",
+ message="Big Number metric must be a dictionary",
+ details="The 'metric' field must be an object with 'name'
and 'aggregate'",
+ suggestions=[
+ "Provide metric as: {'name': 'column', 'aggregate':
'SUM'}",
+ ],
+ error_code="INVALID_METRIC_TYPE",
+ )
+
+ if 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",
- )
-
+ "Add 'aggregate' to your metric: "
+ "{'name': 'col', 'aggregate': 'SUM'}",
+ "Valid aggregates: SUM, COUNT, AVG, MIN, MAX",
+ ],
+ error_code="MISSING_BIG_NUMBER_AGGREGATE",
+ )
+
+ if metric.get("aggregate") and metric["aggregate"] not in ["SUM",
"COUNT", "AVG", "MIN", "MAX"]:
+ return False, ChartGenerationError(
+ error_type="invalid_metric_aggregate",
+ message="Big Number metric aggregate must be one of: SUM,
COUNT, AVG, MIN, MAX",
+ details="The 'aggregate' field must be a valid aggregate
function",
+ suggestions=[
+ "Use one of: SUM, COUNT, AVG, MIN, MAX",
+ "Example: 'aggregate': 'SUM'",
+ ],
+ error_code="INVALID_BIG_NUMBER_AGGREGATE",
+ )
+
```
</div>
</details>
</div>
<small><i>Code Review Run #d63583</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
