Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
sadpandajoe merged PR #38409: URL: https://github.com/apache/superset/pull/38409 -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4031937889 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
github-advanced-security[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2911831815 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,22 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Uses specific Unicode blocks to avoid overly permissive ranges. +_EMOJI_RE = re.compile( +"[" +"\U0001f300-\U0001f5ff" # Misc Symbols and Pictographs Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2267) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,22 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Uses specific Unicode blocks to avoid overly permissive ranges. +_EMOJI_RE = re.compile( +"[" +"\U0001f300-\U0001f5ff" # Misc Symbols and Pictographs Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2265) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,22 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Uses specific Unicode blocks to avoid overly permissive ranges. +_EMOJI_RE = re.compile( +"[" +"\U0001f300-\U0001f5ff" # Misc Symbols and Pictographs Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2264) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,22 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Uses specific Unicode blocks to avoid overly permissive ranges. +_EMOJI_RE = re.compile( +"[" +"\U0001f300-\U0001f5ff" # Misc Symbols and Pictographs Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2266) -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2911817283 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Fixed — replaced broad Unicode ranges with specific block ranges (Misc Symbols, Emoticons, Transport, Dingbats, etc.) to avoid overlapping character ranges. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Fixed — replaced broad Unicode ranges with specific block ranges (Misc Symbols, Emoticons, Transport, Dingbats, etc.) to avoid overlapping character ranges. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Fixed — replaced broad Unicode ranges with specific block ranges (Misc Symbols, Emoticons, Transport, Dingbats, etc.) to avoid overlapping character ranges. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,17 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff" Review Comment: Fixed — replaced broad Unicode ranges with specific block ranges (Misc Symbols, Emoticons, Transport, Dingbats, etc.) to avoid overlapping character ranges. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4030749966 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
github-advanced-security[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2910819505 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,17 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2262) -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
codeant-ai-for-open-source[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4030144331 ## **Sequence Diagram** This PR updates the add_chart_to_existing_dashboard MCP tool so that the optional target_tab parameter is honored by resolving the correct tab (by ID or flexible name match from ROOT_ID and GRID_ID) before inserting the chart into the dashboard layout. ```mermaid sequenceDiagram participant Caller participant MCPTool as MCP tool participant Dashboard as Dashboard backend Caller->>MCPTool: add_chart_to_existing_dashboard with dashboard id, chart id, target_tab MCPTool->>Dashboard: Load dashboard layout and chart Dashboard-->>MCPTool: Return layout and chart MCPTool->>MCPTool: Resolve target tab from layout using target_tab (ID or normalized name, fallback to first tab) MCPTool->>Dashboard: Update layout to add chart under resolved tab Dashboard-->>MCPTool: Save and return updated dashboard MCPTool-->>Caller: Return success with new chart position ``` --- *Generated by [CodeAnt AI](https://codeant.ai)* -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4027625859 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4027259632 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
github-advanced-security[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2908218854 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2258) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2260) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -44,6 +45,15 @@ logger = logging.getLogger(__name__) +# Compiled regex for stripping common emoji Unicode ranges from tab text. +# Not exhaustive (omits flags, skin-tone modifiers, etc.) but sufficient for +# flexible matching of dashboard tab labels. +_EMOJI_RE = re.compile( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \u2702-\u27b0 in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2259) -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4011122933 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2895039118 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,106 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str: return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +cleaned = re.sub( +r"[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" +r"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" +r"\U200D\UFE0F]+", Review Comment: Thanks for confirming the fix. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str: return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) Review Comment: Good catch — if a tab's meta.text is null in the stored JSON, we'd pass None into _normalize_tab_text and re.sub would crash. I'll add a guard to handle None text values before passing to the regex. Thanks. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Thanks for the regex analysis. The character ranges are intentional — they target specific Unicode emoji blocks. I'll review the overlapping ranges and clean them up to be more precise. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Acknowledged — will clean up the overlapping range. Thanks. ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: Same as above — will address the overlapping ranges in the regex. Thanks for the thorough analysis. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
codeant-ai-for-open-source[bot] commented on code in PR #38409:
URL: https://github.com/apache/superset/pull/38409#discussion_r2894943678
##
superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py:
##
@@ -63,35 +63,107 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str:
return row_key
-def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None:
+def _normalize_tab_text(text: str) -> str:
+"""Strip emoji and extra whitespace from tab text for flexible matching."""
+import re
+
+# Remove emoji characters (Unicode emoji ranges)
Review Comment:
**Suggestion:** If a tab's `meta.text` value is `null` in the stored JSON
(which becomes `None` after `json.loads`), `_match_tab_in_children` will pass
`None` into `_normalize_tab_text`, causing `re.sub` to raise a `TypeError` and
breaking the tool for those dashboards; adding a small guard in
`_normalize_tab_text` to handle `None` avoids this runtime failure without
changing behavior for valid strings. [type error]
Severity Level: Major ⚠️
```mdx
- ❌ MCP add_chart_to_existing_dashboard crashes for dashboards with null tab
text.
- ⚠️ Users cannot programmatically add charts to affected tabbed dashboards.
```
```suggestion
if text is None:
return ""
```
Steps of Reproduction ✅
```mdx
1. Start Superset with this PR code and ensure MCP tools are enabled so
`add_chart_to_existing_dashboard` in
`superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py` is
callable.
2. Create or modify a dashboard whose `position_json` (loaded in
`add_chart_to_existing_dashboard` around the layout parsing block) contains
a TAB
component under `ROOT_ID` or `GRID_ID` whose JSON has `"meta": {"text":
null}`; after
`json.loads` this becomes `meta["text"] is None`.
3. Invoke the MCP tool `add_chart_to_existing_dashboard` with a non-empty
`target_tab` (so
`_find_tab_insert_target` at ~line 140 is called with `target_tab` and
`_collect_tabs_groups` at lines 107–125 finds the TABS group containing the
TAB whose
`meta.text` is `None`).
4. During the loop in `_match_tab_in_children` at lines 82–104, `tab_text`
is set to
`None`, then `_normalize_tab_text(tab_text)` at line 102 calls `re.sub(...,
text=None)`
(function at lines 66–79), causing `TypeError: expected string or bytes-like
object`; this
bubbles up to the outer `except` in `add_chart_to_existing_dashboard`, which
logs an error
and returns an `AddChartToDashboardResponse` with `error="Failed to add
chart to
dashboard: expected string or bytes-like object"`.
```
Prompt for AI Agent 🤖
```mdx
This is a comment left during a code review.
**Path:**
superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
**Line:** 70:70
**Comment:**
*Type Error: If a tab's `meta.text` value is `null` in the stored JSON
(which becomes `None` after `json.loads`), `_match_tab_in_children` will pass
`None` into `_normalize_tab_text`, causing `re.sub` to raise a `TypeError` and
breaking the tool for those dashboards; adding a small guard in
`_normalize_tab_text` to handle `None` avoids this runtime failure without
changing behavior for valid strings.
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.
```
👍
| 👎
--
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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
github-advanced-security[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2894943699 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2251) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \ufffd-\ufffd in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2253) ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,107 @@ return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +# Note: using regular strings (not raw) so Python processes \U escapes +cleaned = re.sub( +"[\U0001f300-\U0001f9ff\U2600-\U27bf\Ufe00-\Ufe0f" Review Comment: ## Overly permissive regular expression range Suspicious character range that overlaps with \u2702-\u27b0 in the same character class. [Show more details](https://github.com/apache/superset/security/code-scanning/2252) -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2894939422 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,106 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str: return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +cleaned = re.sub( +r"[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" +r"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" +r"\U200D\UFE0F]+", Review Comment: Great! The fix removes the raw string prefix, allowing Python to process \U escapes as Unicode code points instead of literals. This ensures the regex correctly matches emoji characters for tab text normalization. **superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py** ``` "[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" "\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" "\U200D\UFE0F]+", ``` -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2894933518 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,106 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str: return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +cleaned = re.sub( +r"[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" +r"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" +r"\U200D\UFE0F]+", Review Comment: Great catch\! Fixed in d7a4665 — removed the raw string prefix so Python processes the `\U` escapes as proper Unicode code points instead of matching literal characters. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4010811370 Good point\! I'll add unit tests for emoji tab names and tabs under ROOT_ID in a follow-up. The manual testing covered those cases, but having them in the test suite would be the right thing to do. Thanks for the review\! -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
sadpandajoe commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4008599584 nit: didn't see tests with emojis or tabs under ROOT_ID, but if you tested these manually should be good to go. May want to consider adding them later to ensure things are working properly. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on code in PR #38409: URL: https://github.com/apache/superset/pull/38409#discussion_r2892376676 ## superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py: ## @@ -63,35 +63,106 @@ def _find_next_row_position(layout: Dict[str, Any]) -> str: return row_key -def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None: +def _normalize_tab_text(text: str) -> str: +"""Strip emoji and extra whitespace from tab text for flexible matching.""" +import re + +# Remove emoji characters (Unicode emoji ranges) +cleaned = re.sub( +r"[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" +r"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" +r"\U200D\UFE0F]+", Review Comment: Incorrect emoji regex pattern The regex in `_normalize_tab_text` uses incorrect Unicode escape syntax with double backslashes (`\\U`), which matches literal strings like '\U0001F300' instead of actual emoji characters. This prevents proper emoji stripping for flexible tab matching. The pattern should use single backslashes in a regular string to enable Unicode character matching. Code suggestion Check the AI-generated fix before applying ``` - r"[\\U0001F300-\\U0001F9FF\\U2600-\\U27BF\\UFE00-\\UFE0F" - r"\\U0001FA00-\\U0001FA6F\\U0001FA70-\\U0001FAFF\\U2702-\\U27B0" -r"\\U200D\\UFE0F]+", +"[\U0001F300-\U0001F9FF\U2600-\U27BF\UFE00-\UFE0F" +"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U2702-\U27B0" +"\U200D\UFE0F]+", ``` Code Review Run #141bf2 --- Should Bito avoid suggestions like this for future reviews? (https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules) - [ ] 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4007778179 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4003646681 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4001044598 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4000589310 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on code in PR #38409:
URL: https://github.com/apache/superset/pull/38409#discussion_r2886228168
##
superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py:
##
@@ -82,13 +86,25 @@ def _find_tab_insert_target(layout: Dict[str, Any]) -> str
| None:
for child_id in grid.get("children", []):
child = layout.get(child_id)
if child and child.get("type") == "TABS":
-# Found a TABS component; use its first TAB child
tabs_children = child.get("children", [])
-if tabs_children:
-first_tab_id = tabs_children[0]
-first_tab = layout.get(first_tab_id)
-if first_tab and first_tab.get("type") == "TAB":
-return first_tab_id
+if not tabs_children:
+continue
+
+# When a target_tab is specified, try to resolve it by name or ID
+if target_tab:
+for tab_id in tabs_children:
+tab = layout.get(tab_id)
+if not tab or tab.get("type") != "TAB":
+continue
+tab_text = (tab.get("meta") or {}).get("text", "")
+if target_tab in (tab_id, tab_text):
+return tab_id
+
+# Fallback: return the first TAB child
+first_tab_id = tabs_children[0]
+first_tab = layout.get(first_tab_id)
+if first_tab and first_tab.get("type") == "TAB":
+return first_tab_id
return None
Review Comment:
Fixed in 1f48535. Extracted `_match_tab_in_children()` helper and changed
fallback logic: the function now searches ALL TABS groups before falling back
to the first TAB. Previously it returned early from the first TABS container's
fallback without checking subsequent groups.
--
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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
codecov[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-4000391809 ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38409?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 55.51%. Comparing base ([`a79dcbb`](https://app.codecov.io/gh/apache/superset/commit/a79dcbbb66d987d26d68420691b9e87247b66dfd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1f48535`](https://app.codecov.io/gh/apache/superset/commit/1f48535ee741009abccb5fcd89adbff1d9ab3c88?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 1 commits behind head on master. :x: Your project check has failed because the head coverage (99.24%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage. Additional details and impacted files ```diff @@Coverage Diff @@ ## master #38409 +/- ## == - Coverage 64.71% 55.51% -9.21% == Files1815 2488 +673 Lines 71911 123828 +51917 Branches2291228801+5889 == + Hits4654068741 +22201 - Misses 2537154781 +29410 - Partials0 306 +306 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/38409/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hive](https://app.codecov.io/gh/apache/superset/pull/38409/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.07% <ø> (?)` | | | [presto](https://app.codecov.io/gh/apache/superset/pull/38409/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.09% <ø> (?)` | | | [python](https://app.codecov.io/gh/apache/superset/pull/38409/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `42.76% <ø> (?)` | | | [unit](https://app.codecov.io/gh/apache/superset/pull/38409/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38409?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
netlify[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-3999764586 ### ✅ Deploy Preview for *superset-docs-preview* ready! | Name | Link | |:-:|| |🔨 Latest commit | 4f1ef5a9d29c429d02fb12696b8b9b2d4613a7d5 | |🔍 Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69a887f1a9ec340008a11d57 | |😎 Deploy Preview | [https://deploy-preview-38409--superset-docs-preview.netlify.app](https://deploy-preview-38409--superset-docs-preview.netlify.app) | |📱 Preview on mobile | Toggle QR Code... _Use your smartphone camera to open QR code link._ | --- _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._ -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
codeant-ai-for-open-source[bot] commented on code in PR #38409:
URL: https://github.com/apache/superset/pull/38409#discussion_r2885672251
##
superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py:
##
@@ -82,13 +86,25 @@ def _find_tab_insert_target(layout: Dict[str, Any]) -> str
| None:
for child_id in grid.get("children", []):
child = layout.get(child_id)
if child and child.get("type") == "TABS":
-# Found a TABS component; use its first TAB child
tabs_children = child.get("children", [])
-if tabs_children:
-first_tab_id = tabs_children[0]
-first_tab = layout.get(first_tab_id)
-if first_tab and first_tab.get("type") == "TAB":
-return first_tab_id
+if not tabs_children:
+continue
+
+# When a target_tab is specified, try to resolve it by name or ID
+if target_tab:
+for tab_id in tabs_children:
+tab = layout.get(tab_id)
+if not tab or tab.get("type") != "TAB":
+continue
+tab_text = (tab.get("meta") or {}).get("text", "")
+if target_tab in (tab_id, tab_text):
+return tab_id
+
+# Fallback: return the first TAB child
+first_tab_id = tabs_children[0]
+first_tab = layout.get(first_tab_id)
+if first_tab and first_tab.get("type") == "TAB":
+return first_tab_id
return None
Review Comment:
**Suggestion:** The tab selection helper returns a fallback tab from the
first TABS container inside the loop, so if a requested tab name/ID only exists
in a later TABS group, the function never searches those later groups and
incorrectly inserts the chart into the first tab set (or fails to find any TAB
when the first group's children are non-TABs), leading to charts appearing
under the wrong tab or outside any tab container. [logic error]
Severity Level: Major ⚠️
```mdx
- ❌ MCP add_chart_to_existing_dashboard misplaces charts in wrong tab.
- ⚠️ Automations cannot reliably target specific tabs on complex dashboards.
```
```suggestion
first_tab_fallback: str | None = None
for child_id in grid.get("children", []):
child = layout.get(child_id)
if not child or child.get("type") != "TABS":
continue
tabs_children = child.get("children", [])
if not tabs_children:
continue
# Record the first TAB child across all TABS containers as fallback
if first_tab_fallback is None:
for tab_id in tabs_children:
tab = layout.get(tab_id)
if tab and tab.get("type") == "TAB":
first_tab_fallback = tab_id
break
# When a target_tab is specified, try to resolve it by name or ID
if target_tab:
for tab_id in tabs_children:
tab = layout.get(tab_id)
if not tab or tab.get("type") != "TAB":
continue
tab_text = (tab.get("meta") or {}).get("text", "")
if target_tab in (tab_id, tab_text):
return tab_id
return first_tab_fallback
```
Steps of Reproduction ✅
```mdx
1. Create a Superset dashboard with a v2 layout where `GRID_ID` has multiple
`TABS`
children in its `children` array (e.g. two separate tab groups), and ensure
this layout is
persisted in `Dashboard.position_json` for that dashboard
(`superset/models/dashboard.py:426-428` governs DB access).
2. In the Superset MCP environment, call the
`add_chart_to_existing_dashboard` tool
(`superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py`,
function
`add_chart_to_existing_dashboard`) with `request.dashboard_id` set to that
dashboard,
`request.chart_id` to an existing chart, and `request.target_tab` equal to
the `meta.text`
(or ID) of a `TAB` component that exists **only** under the second `TABS`
group, not the
first.
3. During the request, the tool parses `dashboard.position_json` into
`current_layout` and
invokes `_find_tab_insert_target(current_layout,
target_tab=request.target_tab)` at lines
303-307 in `add_chart_to_existing_dashboard.py` to resolve where to insert
the new row.
4. `_find_tab_insert_target` (lines 66-108) iterates `GRID_ID["children"]`,
encounters the
first `TABS` container, fails to find `target_tab` among its `TAB` children,
then executes
the fallback block and returns the first `TAB` of that first group, never
examining later
`TABS` containers; `_add_chart_to_layout` and `_ensure_layout_structure`
then insert the
chart row under thi
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38409: URL: https://github.com/apache/superset/pull/38409#issuecomment-3999734196 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi closed pull request #38372: fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards URL: https://github.com/apache/superset/pull/38372 -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
aminghadersohi commented on PR #38372: URL: https://github.com/apache/superset/pull/38372#issuecomment-3999731354 Reopening with a clean branch name. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
codecov[bot] commented on PR #38372: URL: https://github.com/apache/superset/pull/38372#issuecomment-3994475714 ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38372?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 54.59%. Comparing base ([`f2f5559`](https://app.codecov.io/gh/apache/superset/commit/f2f55591eccef795f816043b93876e6ca4beacd3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4f1ef5a`](https://app.codecov.io/gh/apache/superset/commit/4f1ef5a9d29c429d02fb12696b8b9b2d4613a7d5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 7 commits behind head on master. :x: Your project check has failed because the head coverage (96.03%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage. Additional details and impacted files ```diff @@Coverage Diff @@ ## master #38372 +/- ## == - Coverage 64.29% 54.59% -9.71% == Files1811 2484 +673 Lines 71491 123334 +51843 Branches2277528642+5867 == + Hits4596767329 +21362 - Misses 2552455690 +30166 - Partials0 315 +315 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/38372/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hive](https://app.codecov.io/gh/apache/superset/pull/38372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.11% <ø> (?)` | | | [presto](https://app.codecov.io/gh/apache/superset/pull/38372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.13% <ø> (?)` | | | [python](https://app.codecov.io/gh/apache/superset/pull/38372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.21% <ø> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38372?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- 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]
Re: [PR] fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards [superset]
bito-code-review[bot] commented on PR #38372: URL: https://github.com/apache/superset/pull/38372#issuecomment-3994459153 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). -- 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]
