bito-code-review[bot] commented on code in PR #37214:
URL: https://github.com/apache/superset/pull/37214#discussion_r2699296537
##########
superset/mcp_service/chart/tool/generate_chart.py:
##########
@@ -136,22 +139,33 @@ async def generate_chart( # noqa: C901
)
from superset.mcp_service.chart.validation import ValidationPipeline
- is_valid, parsed_request, validation_error = (
- ValidationPipeline.validate_request(request.model_dump())
+ validation_result = ValidationPipeline.validate_request_with_warnings(
+ request.model_dump()
)
- if is_valid and parsed_request is not None:
+
+ if validation_result.is_valid and validation_result.request is not
None:
# Use the validated request going forward
- request = parsed_request
- if not is_valid:
+ request = validation_result.request
+
+ # Capture runtime warnings (informational, not blocking)
+ if validation_result.warnings:
+ runtime_warnings = validation_result.warnings.get("warnings", [])
+ if runtime_warnings:
+ await ctx.info(
+ "Runtime suggestions: %s" % (";
".join(runtime_warnings[:3]),)
+ )
+
+ if not validation_result.is_valid:
execution_time = int((time.time() - start_time) * 1000)
- assert validation_error is not None # Type narrowing for mypy
+ assert validation_result.error is not None # Type narrowing for
mypy
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Replace assert with proper exception handling</b></div>
<div id="fix">
Replace `assert` statement with explicit exception raising for production
code. The `assert` on line 160 should use a proper exception like `ValueError`
or `RuntimeError`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
execution_time = int((time.time() - start_time) * 1000)
if validation_result.error is None:
raise RuntimeError("Validation error object is missing")
````
</div>
</details>
</div>
<small><i>Code Review Run #3ed33d</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]
