codeant-ai-for-open-source[bot] commented on code in PR #43575:
URL: https://github.com/apache/superset/pull/43575#discussion_r3868699315
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,90 @@ def position(self) -> dict[str, Any]:
return {}
@property
- def tabs(self) -> dict[str, Any]:
+ def tabs(self) -> dict[str, Any]: # noqa: C901
+ if not isinstance(self.position, dict):
+ logger.warning("Dashboard %s: layout is not a mapping", self.id)
+ return {}
if self.position == {}:
return {}
- def get_node(node_id: str) -> dict[str, Any]:
+ def get_node(node_id: str) -> Optional[dict[str, Any]]:
"""
Helper function for getting a node from the position_data
"""
- return self.position[node_id]
+ return self.position.get(node_id)
+
+ def register_tab(node: dict[str, Any]) -> None:
+ """
+ Helper function for titling a TAB node and adding it to all_tabs
+ """
+ meta = node.get("meta")
+ if not isinstance(meta, dict):
+ meta = {}
+ if "text" not in meta:
+ logger.warning(
+ "Dashboard %s: tab node %s has no title in the layout",
+ self.id,
+ node.get("id"),
+ )
+ node["title"] = meta.get("text", "")
+ node_id = node.get("id")
Review Comment:
**Suggestion:** The fallback only handles a missing `text` key; it does not
handle an unusable value such as `meta: {"text": null}`. Such a TAB is
documented and tested as needing an empty title, but this code stores `None` in
both the tree and `all_tabs`, producing a non-string API payload despite
`TabsPayloadSchema` declaring string titles. Normalize non-string title values
to the empty string. [api mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Tabs API can emit null or non-string titles.
- โ ๏ธ Clients expecting `TabsPayloadSchema` strings may reject responses.
- โ ๏ธ Tab labels become inconsistent for malformed metadata.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4984c7e17d56493197be19047603038f&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=4984c7e17d56493197be19047603038f&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/models/dashboard.py
**Line:** 407:408
**Comment:**
*Api Mismatch: The fallback only handles a missing `text` key; it does
not handle an unusable value such as `meta: {"text": null}`. Such a TAB is
documented and tested as needing an empty title, but this code stores `None` in
both the tree and `all_tabs`, producing a non-string API payload despite
`TabsPayloadSchema` declaring string titles. Normalize non-string title values
to the empty string.
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%2F43575&comment_hash=ba5be636e1c2daf35cd663335bde347cdf019126b4767201855994c9d35acc41&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43575&comment_hash=ba5be636e1c2daf35cd663335bde347cdf019126b4767201855994c9d35acc41&reaction=dislike'>๐</a>
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,90 @@ def position(self) -> dict[str, Any]:
return {}
@property
- def tabs(self) -> dict[str, Any]:
+ def tabs(self) -> dict[str, Any]: # noqa: C901
+ if not isinstance(self.position, dict):
+ logger.warning("Dashboard %s: layout is not a mapping", self.id)
+ return {}
if self.position == {}:
return {}
- def get_node(node_id: str) -> dict[str, Any]:
+ def get_node(node_id: str) -> Optional[dict[str, Any]]:
"""
Helper function for getting a node from the position_data
"""
- return self.position[node_id]
+ return self.position.get(node_id)
+
+ def register_tab(node: dict[str, Any]) -> None:
+ """
+ Helper function for titling a TAB node and adding it to all_tabs
+ """
+ meta = node.get("meta")
+ if not isinstance(meta, dict):
+ meta = {}
+ if "text" not in meta:
+ logger.warning(
+ "Dashboard %s: tab node %s has no title in the layout",
+ self.id,
+ node.get("id"),
+ )
+ node["title"] = meta.get("text", "")
+ node_id = node.get("id")
+ if node_id is None:
+ logger.warning(
+ "Dashboard %s: skipping tab node with no id in the layout",
+ self.id,
+ )
+ return
+ node["value"] = node_id
+ all_tabs[node_id] = node["title"]
def build_tab_tree(
node: dict[str, Any], children: list[dict[str, Any]]
) -> None:
"""
Function for building the tab tree structure and list of all tabs
"""
+ if "type" not in node:
+ logger.warning(
+ "Dashboard %s: skipping untyped layout node %s",
+ self.id,
+ node.get("id"),
+ )
+ return
+ # A node whose type is not one of the four below is walked through
+ # without contributing to the tree, exactly as an untabbed layout
+ # element always has been.
+ node_type = node["type"]
new_children: list[dict[str, Any]] = []
# new children to overwrite parent's children
for child_id in node.get("children", []):
child = get_node(child_id)
Review Comment:
**Suggestion:** The defensive walk still assumes `children` is iterable. A
valid JSON layout can contain `children: null` or a scalar, causing `TypeError`
at this loop before the malformed-child guard runs and allowing the dashboard
tabs endpoint to return 500. Only iterate when `children` is a list (or
otherwise normalize invalid values to an empty list). [type error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ Malformed layouts can abort dashboard tab traversal.
- โ Dashboard updates can fail in `process_tab_diff`.
- โ ๏ธ Tabs GET returns 400 instead of usable partial results.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d09b65ceb229422d8a785fdfe2e27f9b&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=d09b65ceb229422d8a785fdfe2e27f9b&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/models/dashboard.py
**Line:** 438:439
**Comment:**
*Type Error: The defensive walk still assumes `children` is iterable. A
valid JSON layout can contain `children: null` or a scalar, causing `TypeError`
at this loop before the malformed-child guard runs and allowing the dashboard
tabs endpoint to return 500. Only iterate when `children` is a list (or
otherwise normalize invalid values to an empty list).
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%2F43575&comment_hash=1244f5e83b44bebb5074aff7ca3c87d87d99e221fc1f76e527188399021c0054&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43575&comment_hash=1244f5e83b44bebb5074aff7ca3c87d87d99e221fc1f76e527188399021c0054&reaction=dislike'>๐</a>
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,90 @@ def position(self) -> dict[str, Any]:
return {}
@property
- def tabs(self) -> dict[str, Any]:
+ def tabs(self) -> dict[str, Any]: # noqa: C901
+ if not isinstance(self.position, dict):
+ logger.warning("Dashboard %s: layout is not a mapping", self.id)
+ return {}
if self.position == {}:
return {}
- def get_node(node_id: str) -> dict[str, Any]:
+ def get_node(node_id: str) -> Optional[dict[str, Any]]:
"""
Helper function for getting a node from the position_data
"""
- return self.position[node_id]
+ return self.position.get(node_id)
+
+ def register_tab(node: dict[str, Any]) -> None:
+ """
+ Helper function for titling a TAB node and adding it to all_tabs
+ """
+ meta = node.get("meta")
+ if not isinstance(meta, dict):
+ meta = {}
+ if "text" not in meta:
+ logger.warning(
+ "Dashboard %s: tab node %s has no title in the layout",
+ self.id,
+ node.get("id"),
+ )
+ node["title"] = meta.get("text", "")
+ node_id = node.get("id")
+ if node_id is None:
+ logger.warning(
+ "Dashboard %s: skipping tab node with no id in the layout",
+ self.id,
+ )
+ return
+ node["value"] = node_id
+ all_tabs[node_id] = node["title"]
Review Comment:
**Suggestion:** The new no-id protection only excludes `None`. A valid JSON
TAB can have an object or array as its `id`; assigning that value as a
dictionary key raises `TypeError`, so one malformed tab still takes down the
entire tabs response. Restrict TAB identifiers to hashable string values or
skip invalid identifiers. [type error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ Invalid TAB identifiers abort tab registration.
- โ Dashboard updates may fail during stored-layout inspection.
- โ ๏ธ Tabs responses become unavailable for malformed layouts.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=25884b9bdb6844029302cf441a9e030f&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=25884b9bdb6844029302cf441a9e030f&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/models/dashboard.py
**Line:** 415:416
**Comment:**
*Type Error: The new no-id protection only excludes `None`. A valid
JSON TAB can have an object or array as its `id`; assigning that value as a
dictionary key raises `TypeError`, so one malformed tab still takes down the
entire tabs response. Restrict TAB identifiers to hashable string values or
skip invalid identifiers.
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%2F43575&comment_hash=47fd10f47217c0478d5be5bace808a1c1a23f7a260b580eb85d04699aed903ba&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43575&comment_hash=47fd10f47217c0478d5be5bace808a1c1a23f7a260b580eb85d04699aed903ba&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]
