codeant-ai-for-open-source[bot] commented on code in PR #37142:
URL: https://github.com/apache/superset/pull/37142#discussion_r2696198713
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -161,6 +161,24 @@ async def get_chart_data( # noqa: C901
or form_data.get("row_limit")
or current_app.config["ROW_LIMIT"]
)
+
+ # Handle different chart types that have different form_data
structures
+ # Some charts use "metric" (singular), not "metrics" (plural):
+ # - big_number, big_number_total
+ # - pop_kpi (BigNumberPeriodOverPeriod)
+ # These charts also don't have groupby columns
+ viz_type = chart.viz_type or ""
+ single_metric_types = ("big_number", "pop_kpi")
+ if viz_type.startswith("big_number") or viz_type in
single_metric_types:
+ # These chart types use "metric" (singular)
+ metric = form_data.get("metric")
+ metrics = [metric] if metric else []
+ groupby_columns: list[str] = [] # These charts don't
group by
Review Comment:
**Suggestion:** Using the runtime-evaluated type annotation `list[str]` on
`groupby_columns` can raise a TypeError on Python versions <3.9 because
built-in generics were not subscriptable at runtime; replace with the
typing-compatible `List[str]` which is already imported to ensure
compatibility. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ get_chart_data crashes for big_number fallback charts.
- ⚠️ MCP data retrieval fails for older unsaved charts.
```
</details>
```suggestion
groupby_columns: List[str] = [] # These charts don't
group by
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the MCP service with this PR code and Python 3.8 runtime.
2. Trigger get_chart_data() fallback branch by requesting a chart without
saved
query_context:
call get_chart_data in superset/mcp_service/chart/tool/get_chart_data.py
(function
get_chart_data) such that execution reaches the fallback branch (see
lines ~146-176
in that file). The variable annotation on line 176 is executed in
function scope.
3. Execution hits the branch for single-metric charts (viz_type starting with
"big_number")
and evaluates the variable annotation at line 176: "groupby_columns:
list[str] = []".
4. On Python <3.9 this attempts to subscript built-in 'list' at runtime and
raises
TypeError,
causing get_chart_data to raise and return an InternalError ChartError
instead of data.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/tool/get_chart_data.py
**Line:** 176:176
**Comment:**
*Type Error: Using the runtime-evaluated type annotation `list[str]` on
`groupby_columns` can raise a TypeError on Python versions <3.9 because
built-in generics were not subscriptable at runtime; replace with the
typing-compatible `List[str]` which is already imported to ensure compatibility.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
