Skip to content

fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards#38409

Open
aminghadersohi wants to merge 4 commits intoapache:masterfrom
aminghadersohi:amin/fix-target-tab-placement
Open

fix(mcp): honor target_tab parameter when adding charts to tabbed dashboards#38409
aminghadersohi wants to merge 4 commits intoapache:masterfrom
aminghadersohi:amin/fix-target-tab-placement

Conversation

@aminghadersohi
Copy link
Contributor

@aminghadersohi aminghadersohi commented Mar 4, 2026

User description

SUMMARY

When adding charts to a dashboard that uses tabs via the add_chart_to_existing_dashboard MCP tool, the target_tab parameter was accepted without error but completely ignored. Charts were always placed in the first tab or under GRID_ID at the root level, where they become invisible ghost entries — registered in the dashboard metadata but never rendered in the UI.

Root causes fixed:

  1. target_tab was never passed to _find_tab_insert_target() — the parameter was accepted but silently discarded
  2. TABS search only checked GRID_ID children — Superset dashboards can place TABS under ROOT_ID (which is the common pattern), so TABS were never found
  3. Exact tab name matching failed with emoji — tab text like 🧭 Exploratory wouldn't match when user passes "Exploratory", added flexible emoji-stripped case-insensitive matching

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend logic change, no UI modifications.

TESTING INSTRUCTIONS

  1. Create a dashboard with multiple tabs (e.g., "Sales Overview", "Exploratory")
  2. Call add_chart_to_existing_dashboard with target_tab set to the second tab's name (without emoji)
  3. Verify the chart appears inside the specified tab (not the first tab, and not invisible under GRID_ID)
  4. Call without target_tab — chart should still land in the first tab (backward compat)

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

MCP Testing (via localhost MCP service on branch amin/fix-target-tab-placement)

Test 1: Target tab with emoji text — "Exploratory" matches "🧭 Exploratory"

  • get_dashboard_info(6) → Sales Dashboard has tabs: "🎯 Sales Overview" (TAB-d-E0Zc1cTH) and "🧭 Exploratory" (TAB-4fthLQmdX)
  • add_chart_to_existing_dashboard(dashboard_id=6, chart_id=2, target_tab="Exploratory")
  • Result: CHART-2 parents = ["ROOT_ID", "TABS-e5Ruro0cjP", "TAB-4fthLQmdX", ...] — correctly placed in Exploratory tab ✅

Test 2: Target tab with emoji text — "Sales Overview" matches "🎯 Sales Overview"

  • add_chart_to_existing_dashboard(dashboard_id=6, chart_id=3, target_tab="Sales Overview")
  • Result: CHART-3 parents = ["ROOT_ID", "TABS-e5Ruro0cjP", "TAB-d-E0Zc1cTH", ...] — correctly placed in Sales Overview tab ✅

Test 3: ROOT_ID TABS search (bug found & fixed)

  • Before fix: TABS under ROOT_ID were missed because _find_tab_insert_target only searched GRID_ID children
  • After fix: Both ROOT_ID and GRID_ID children are searched for TABS components ✅

Test 4: Unit tests

  • pytest tests/unit_tests/mcp_service/dashboard/ -x -q → 45 passed ✅

CodeAnt-AI Description

Honor target_tab when adding charts to tabbed dashboards

What Changed

  • Charts added via the tool now land in the requested dashboard tab when a matching tab name or component ID is provided; if no match, the first tab is used.
  • Tab discovery now checks TABS groups under both ROOT_ID and GRID_ID so tabs placed in either location are found.
  • Tab name matching accepts case-insensitive names and strips leading emoji from display text so names like "🧭 Exploratory" match "Exploratory".
  • Added unit and integration tests that verify placing charts into a specific tab by name and ensure new rows are not placed under GRID_ID (avoiding invisible chart entries).

Impact

✅ Charts land in the intended dashboard tab
✅ No more invisible charts under GRID_ID
✅ Targeting tabs works with emoji-prefixed tab names

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #46cae8

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 4f1ef5a..4f1ef5a
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added the change:backend Requires changing the backend label Mar 4, 2026
@netlify
Copy link

netlify bot commented Mar 4, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 0707512
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69a9e022b9043600088d40fd
😎 Deploy Preview https://deploy-preview-38409--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@aminghadersohi aminghadersohi force-pushed the amin/fix-target-tab-placement branch from 4f1ef5a to def55b1 Compare March 4, 2026 21:02
@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 11.62791% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.96%. Comparing base (0dbd4c5) to head (0707512).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../dashboard/tool/add_chart_to_existing_dashboard.py 11.62% 38 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #38409      +/-   ##
==========================================
+ Coverage   64.82%   64.96%   +0.14%     
==========================================
  Files        1815     2515     +700     
  Lines       71917   126769   +54852     
  Branches    22915    29237    +6322     
==========================================
+ Hits        46618    82359   +35741     
- Misses      25299    42981   +17682     
- Partials        0     1429    +1429     
Flag Coverage Δ
hive 41.63% <11.62%> (?)
mysql 63.38% <11.62%> (?)
postgres 63.45% <11.62%> (?)
presto 41.64% <11.62%> (?)
python 65.13% <11.62%> (?)
sqlite 63.07% <11.62%> (?)
unit 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #9001ca

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: def55b1..1f48535
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #c68503

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 1f48535..2f45396
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi force-pushed the amin/fix-target-tab-placement branch from 2f45396 to 77af2d2 Compare March 5, 2026 08:20
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 5, 2026

Code Review Agent Run #9e6756

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 6226e66..77af2d2
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

…hboards

The target_tab parameter on add_chart_to_existing_dashboard was accepted
but silently ignored, causing charts to always land in the first tab (or
under GRID_ID where they become invisible).  _find_tab_insert_target now
resolves target_tab by display name or component ID before falling back
to the first tab.
Previously the tab selection helper returned the fallback from the first
TABS container without searching later groups, causing charts to be
placed in the wrong tab when the target only exists in a later group.
Two bugs fixed in add_chart_to_existing_dashboard:

1. Tab search only checked GRID_ID children for TABS components, but
   Superset dashboards can place TABS under ROOT_ID. Now searches both
   ROOT_ID and GRID_ID children.

2. Tab name matching was exact-only, failing when tab text contains
   emoji prefixes (e.g., "🧭 Exploratory"). Added flexible matching
   that strips emoji and compares case-insensitively.
@aminghadersohi aminghadersohi force-pushed the amin/fix-target-tab-placement branch from 77af2d2 to 0707512 Compare March 5, 2026 19:57
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:L This PR changes 100-499 lines, ignoring generated files label Mar 5, 2026
Copy link
Contributor

@bito-code-review bito-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #141bf2

Actionable Suggestions - 1
  • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py - 1
Review Details
  • Files reviewed - 2 · Commit Range: 71afcd8..0707512
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +72 to +74
r"[\U0001F300-\U0001F9FF\U00002600-\U000027BF\U0000FE00-\U0000FE0F"
r"\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U00002702-\U000027B0"
r"\U0000200D\U0000FE0F]+",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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\\U00002600-\\U000027BF\\U0000FE00-\\U0000FE0F"
 -        r"\\U0001FA00-\\U0001FA6F\\U0001FA70-\\U0001FAFF\\U00002702-\\U000027B0"
 -        r"\\U0000200D\\U0000FE0F]+",
 +    "[\U0001F300-\U0001F9FF\U00002600-\U000027BF\U0000FE00-\U0000FE0F"
 +        "\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\U00002702-\U000027B0"
 +        "\U0000200D\U0000FE0F]+",

Code Review Run #141bf2


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/L size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant