codeant-ai-for-open-source[bot] commented on code in PR #43476:
URL: https://github.com/apache/superset/pull/43476#discussion_r3845957989
##########
superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py:
##########
@@ -59,14 +60,12 @@ def _find_chart_keys(layout: Dict[str, Any], chart_id: int)
-> list[str]:
A chart can legitimately appear more than once in a layout (e.g. under
multiple tabs), so all occurrences are returned.
"""
- # Accept both int and string chartId โ position_json is
user/frontend-authored
- # and imported or hand-edited layouts may store chartId as a string.
return [
key
for key, node in layout.items()
if isinstance(node, dict)
and node.get("type") == "CHART"
- and (node.get("meta") or {}).get("chartId") in (chart_id,
str(chart_id))
+ and normalize_chart_id((node.get("meta") or {}).get("chartId")) ==
chart_id
Review Comment:
**Suggestion:** `normalize_chart_id` accepts non-canonical decimal strings
such as `001`, so this line removes the matching layout chart even though
`_clean_json_metadata` only removes the canonical forms `1` and `"1"`. The
chart can therefore be detached while stale `001` references remain in
timed-refresh, immune-slice, filter-scope, expanded-slice, or default-filter
metadata. Reject leading-zero strings in normalization or make metadata cleanup
use the same normalization. [logic error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Chart removal leaves stale expanded-slice metadata.
- โ ๏ธ Timed-refresh metadata can retain detached chart IDs.
- โ ๏ธ Filter and default-filter references may target removed charts.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f23daaae458b4fe3b0ef301f012b62a1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f23daaae458b4fe3b0ef301f012b62a1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py
**Line:** 68:68
**Comment:**
*Logic Error: `normalize_chart_id` accepts non-canonical decimal
strings such as `001`, so this line removes the matching layout chart even
though `_clean_json_metadata` only removes the canonical forms `1` and `"1"`.
The chart can therefore be detached while stale `001` references remain in
timed-refresh, immune-slice, filter-scope, expanded-slice, or default-filter
metadata. Reject leading-zero strings in normalization or make metadata cleanup
use the same normalization.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43476&comment_hash=a469df16e91e001dc608dd0ed1640aec8de27c7c19a2dc1b1454f541a9a47027&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43476&comment_hash=a469df16e91e001dc608dd0ed1640aec8de27c7c19a2dc1b1454f541a9a47027&reaction=dislike'>๐</a>
--
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]
