codeant-ai-for-open-source[bot] commented on code in PR #40957:
URL: https://github.com/apache/superset/pull/40957#discussion_r3493553568
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -871,6 +902,32 @@ def sanitize_dashboard_title(cls, v: str | None) -> str |
None:
v, "Dashboard title", max_length=500, allow_empty=True
)
+ @field_validator("slug")
+ @classmethod
+ def normalize_slug(cls, v: str | None) -> str | None:
+ """Normalize the slug to match the REST DashboardPutSchema contract.
+
+ Mirrors ``BaseDashboardSchema.post_load``: strip, replace spaces with
+ hyphens, and drop characters outside ``[\\w-]`` so the tool cannot
+ persist slugs the REST update path would have cleaned.
+
+ Whitespace-only inputs normalize to ``""`` (clears the slug), matching
+ REST schema behavior. Raises ``ValueError`` when a non-whitespace input
+ normalizes to empty (e.g. ``"!!!"``), preventing accidental slug
clearing.
+ """
+ if not v:
+ return v
+ stripped = v.strip()
Review Comment:
**Suggestion:** Add an explicit type annotation for this local variable to
satisfy the type-hint requirement for relevant new variables. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is newly added Python code and the local variable `stripped` is
assigned without a type hint even though its type is clearly `str`. That
matches the custom rule requiring type hints on relevant variables that can be
annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=abe4864dd0e4485b94aca1b39b4cad44&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=abe4864dd0e4485b94aca1b39b4cad44&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/schemas.py
**Line:** 920:920
**Comment:**
*Custom Rule: Add an explicit type annotation for this local variable
to satisfy the type-hint requirement for relevant new variables.
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%2F40957&comment_hash=1b753b97ab97296cc8564d7081e2be8306b06b361df6b46597280a911ee0b44f&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=1b753b97ab97296cc8564d7081e2be8306b06b361df6b46597280a911ee0b44f&reaction=dislike'>π</a>
##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -113,22 +113,58 @@ def _merge_json_metadata(dashboard: Any, overrides:
dict[str, Any]) -> str:
existing: dict[str, Any] = {}
if dashboard.json_metadata:
try:
- parsed = json.loads(dashboard.json_metadata)
- if isinstance(parsed, dict):
+ if isinstance(parsed := json.loads(dashboard.json_metadata), dict):
Review Comment:
**Suggestion:** Add an explicit type annotation for the parsed metadata
value by assigning it to a typed local variable before the `isinstance` check
instead of introducing it via walrus assignment. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The rule requires type hints for relevant variables that can be annotated.
Here `parsed` is introduced via a walrus assignment without any explicit type
annotation, and it is a local value that could be typed before the `isinstance`
check.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8bb7deef50904fda886f4eb63624c4b2&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=8bb7deef50904fda886f4eb63624c4b2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<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/update_dashboard.py
**Line:** 116:116
**Comment:**
*Custom Rule: Add an explicit type annotation for the parsed metadata
value by assigning it to a typed local variable before the `isinstance` check
instead of introducing it via walrus assignment.
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%2F40957&comment_hash=1c355f74bc9def504a4db14c9cb88b0186318eada9bb765b55d977eee77a12ac&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=1c355f74bc9def504a4db14c9cb88b0186318eada9bb765b55d977eee77a12ac&reaction=dislike'>π</a>
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -871,6 +902,32 @@ def sanitize_dashboard_title(cls, v: str | None) -> str |
None:
v, "Dashboard title", max_length=500, allow_empty=True
)
+ @field_validator("slug")
+ @classmethod
+ def normalize_slug(cls, v: str | None) -> str | None:
+ """Normalize the slug to match the REST DashboardPutSchema contract.
+
+ Mirrors ``BaseDashboardSchema.post_load``: strip, replace spaces with
+ hyphens, and drop characters outside ``[\\w-]`` so the tool cannot
+ persist slugs the REST update path would have cleaned.
+
+ Whitespace-only inputs normalize to ``""`` (clears the slug), matching
+ REST schema behavior. Raises ``ValueError`` when a non-whitespace input
+ normalizes to empty (e.g. ``"!!!"``), preventing accidental slug
clearing.
+ """
+ if not v:
+ return v
+ stripped = v.strip()
+ if not stripped:
+ return "" # whitespace-only β same as empty string (clears slug)
+ normalized = re.sub(r"[^\w\-]+", "", stripped.replace(" ", "-"))
Review Comment:
**Suggestion:** Add an explicit type annotation for this normalized value so
newly added variable assignments remain fully typed. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is also newly added Python code, and `normalized` is a local variable
with an obvious concrete type (`str`) that is left unannotated. That is a real
violation of the type-hint requirement for relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=680c7785734b43ea9cb3424293c007a9&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=680c7785734b43ea9cb3424293c007a9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/schemas.py
**Line:** 923:923
**Comment:**
*Custom Rule: Add an explicit type annotation for this normalized value
so newly added variable assignments remain fully typed.
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%2F40957&comment_hash=41f7beffc2574bc28f28e5b4177a49d5c0132c5ff3bdcf5974de7921bcea46cf&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=41f7beffc2574bc28f28e5b4177a49d5c0132c5ff3bdcf5974de7921bcea46cf&reaction=dislike'>π</a>
##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -113,22 +113,58 @@ def _merge_json_metadata(dashboard: Any, overrides:
dict[str, Any]) -> str:
existing: dict[str, Any] = {}
if dashboard.json_metadata:
try:
- parsed = json.loads(dashboard.json_metadata)
- if isinstance(parsed, dict):
+ if isinstance(parsed := json.loads(dashboard.json_metadata), dict):
existing = parsed
except (ValueError, TypeError):
pass
existing.update(overrides)
return json.dumps(existing)
+# Typed json_metadata convenience fields. Each maps 1:1 to a json_metadata
+# key but is exposed as a validated field so an LLM does not have to hand-build
+# the raw ``json_metadata_overrides`` dict for common toggles.
+_TYPED_METADATA_FIELDS: tuple[str, ...] = (
+ "cross_filters_enabled",
+ "refresh_frequency",
+ "filter_bar_orientation",
+)
+
+
+def _collect_metadata_overrides(request: UpdateDashboardRequest) -> dict[str,
Any]:
+ """Combine the generic ``json_metadata_overrides`` with the typed fields.
+
+ A key set via both a typed field and the generic dict is ambiguous, so a
+ collision raises ``ValueError``. Otherwise the typed fields are layered on
+ top of the generic overrides. The generic dict stays as an escape hatch for
+ keys without a typed field.
+ """
+ overrides: dict[str, Any] = dict(request.json_metadata_overrides or {})
+ typed: dict[str, Any] = {
+ field: value
+ for field in _TYPED_METADATA_FIELDS
+ if (value := getattr(request, field)) is not None
+ }
+ if clashes := sorted(set(overrides) & set(typed)):
Review Comment:
**Suggestion:** Introduce a typed local variable for the overlap keys before
the conditional so the computed clash list is explicitly annotated.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a new local variable created by walrus assignment and it is not
type-annotated. Since it is a simple computed collection, it falls under the
ruleβs requirement to add type hints to relevant variables that can be
annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1a8750d9fa1845ebaca70dd2032392fb&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=1a8750d9fa1845ebaca70dd2032392fb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<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/update_dashboard.py
**Line:** 148:148
**Comment:**
*Custom Rule: Introduce a typed local variable for the overlap keys
before the conditional so the computed clash list is explicitly annotated.
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%2F40957&comment_hash=1befe93c4e874ce02fecf1d8d93e5fe8b6232a8621c139a5c66a150086342f5c&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=1befe93c4e874ce02fecf1d8d93e5fe8b6232a8621c139a5c66a150086342f5c&reaction=dislike'>π</a>
##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -152,57 +188,123 @@ def _apply_field_updates(dashboard: Any, request:
UpdateDashboardRequest) -> lis
dashboard.position_json = json.dumps(request.position_json)
changed.append("position_json")
- if request.json_metadata_overrides is not None:
- dashboard.json_metadata = _merge_json_metadata(
- dashboard, request.json_metadata_overrides
- )
+ metadata_overrides: dict[str, Any] = _collect_metadata_overrides(request)
+ if metadata_overrides:
+ dashboard.json_metadata = _merge_json_metadata(dashboard,
metadata_overrides)
changed.append("json_metadata")
if request.css is not None:
dashboard.css = request.css or None
changed.append("css")
+ if request.tags is not None:
+ # Reuse the same helper the REST UpdateDashboardCommand uses so tag
+ # association semantics (custom-tag full replacement) stay identical.
+ from superset.commands.utils import update_tags
+ from superset.tags.models import ObjectType
+
+ update_tags(ObjectType.dashboard, dashboard.id, dashboard.tags,
request.tags)
+ changed.append("tags")
+
return changed
+def _validate_update_request(
+ dashboard: Any, request: UpdateDashboardRequest
+) -> DashboardError | None:
+ """Pre-flight validation mirroring the REST update path.
+
+ Runs before any mutation so the tool rejects the same payloads the REST
+ ``DashboardPutSchema`` / ``UpdateDashboardCommand`` would β invalid CSS,
+ conflicting metadata keys, and unauthorized or unknown tag IDs β returning
a
+ structured error instead of failing deep inside the commit.
+ """
+ from marshmallow import ValidationError as MarshmallowValidationError
+
+ from superset.commands.exceptions import (
+ TagForbiddenError,
+ TagNotFoundValidationError,
+ )
+ from superset.commands.utils import validate_tags
+ from superset.dashboards.schemas import validate_css
+ from superset.tags.models import ObjectType
+
+ # Empty string clears CSS (no validation needed); only validate real
content.
+ if request.css:
+ try:
+ validate_css(request.css)
+ except MarshmallowValidationError as ex:
+ detail = (
+ "; ".join(str(m) for m in ex.messages)
+ if isinstance(ex.messages, list)
+ else str(ex.messages)
+ )
Review Comment:
**Suggestion:** Add a type annotation for the constructed CSS validation
message variable to keep local error payload values explicitly typed.
[custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The variable `detail` is a locally derived string value used in the error
message, and it is assigned without an explicit type annotation. That matches
the type-hint rule for relevant variables that can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a262e15cc36e4548acbdca53f1934b58&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=a262e15cc36e4548acbdca53f1934b58&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<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/update_dashboard.py
**Line:** 237:241
**Comment:**
*Custom Rule: Add a type annotation for the constructed CSS validation
message variable to keep local error payload values explicitly typed.
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%2F40957&comment_hash=686bc2b5adfb38ea88b8b8de107905fdcff541c5580611a5c20cc97d91b1ad24&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=686bc2b5adfb38ea88b8b8de107905fdcff541c5580611a5c20cc97d91b1ad24&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]
