Skip to content

feat(mcp): add duplicate_dashboard tool - #40959

Merged
rusackas merged 9 commits into
masterfrom
mcp-duplicate-dashboard
Jun 30, 2026
Merged

feat(mcp): add duplicate_dashboard tool#40959
rusackas merged 9 commits into
masterfrom
mcp-duplicate-dashboard

Conversation

@aminghadersohi

Copy link
Copy Markdown
Contributor

SUMMARY

Adds a new duplicate_dashboard MCP tool to the Superset MCP service. The canonical AI workflow this enables: "create a regional/staging variant of this dashboard" — clone a template dashboard, then edit the copy.

Design:

  • Wraps the existing CopyDashboardCommand (the same backend used by the frontend "Save as" flow on /api/v1/dashboard/<id>/copy/).
  • The source dashboard can be identified by numeric ID, UUID, or slug (resolved via DashboardDAO.get_by_id_or_slug, which applies dashboard access filters).
  • DashboardCopySchema's copy data contract requires json_metadata; the tool builds it server-side from the source dashboard's current state (json_metadata plus a positions key from position_json), mirroring exactly what the frontend "Save as" sends. json_metadata is intentionally not exposed as a tool parameter.
  • duplicate_slices: bool = false — when true, every chart on the source dashboard is deep-copied into a new chart object owned by the caller; when false the copy references the same charts.
  • The new title is sanitized for XSS using the same pattern as generate_dashboard (nh3-based sanitize_user_input), with a warning surfaced when content is stripped and a hard error when the title sanitizes to nothing.
  • Structured error responses for source-not-found, access-denied, forbidden (DASHBOARD_RBAC), and copy failures; errors are wrapped for LLM-context safety.
  • Registered as a mutate tool (class_permission_name="Dashboard", method_permission_name="write") and added to MCP_CACHE_CONFIG["excluded_tools"] so responses are never cached.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — MCP service tool, no UI changes.

TESTING INSTRUCTIONS

  1. Start the MCP service and connect an MCP client.
  2. Call duplicate_dashboard with {"dashboard_id": <existing id|uuid|slug>, "dashboard_title": "My Copy"} — verify a new dashboard is created referencing the same charts, and the returned URL opens it.
  3. Call again with "duplicate_slices": true — verify the new dashboard contains new chart objects (different chart IDs) with the same layout.
  4. Verify error responses for a non-existent dashboard ID and for a dashboard the user cannot access.
  5. Run unit tests: pytest tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py

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

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.18792% with 107 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.45%. Comparing base (84e07df) to head (d8e1d7e).
⚠️ Report is 69 commits behind head on master.

Files with missing lines Patch % Lines
.../mcp_service/dashboard/tool/duplicate_dashboard.py 17.92% 87 Missing ⚠️
superset/mcp_service/dashboard/schemas.py 52.38% 20 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40959      +/-   ##
==========================================
- Coverage   64.48%   64.45%   -0.04%     
==========================================
  Files        2664     2665       +1     
  Lines      146083   146353     +270     
  Branches    33698    33729      +31     
==========================================
+ Hits        94207    94329     +122     
- Misses      50168    50305     +137     
- Partials     1708     1719      +11     
Flag Coverage Δ
hive 39.19% <28.18%> (-0.03%) ⬇️
mysql 57.90% <28.18%> (-0.05%) ⬇️
postgres 57.96% <28.18%> (-0.05%) ⬇️
presto 40.76% <28.18%> (-0.04%) ⬇️
python 59.39% <28.18%> (-0.06%) ⬇️
sqlite 57.60% <28.18%> (-0.08%) ⬇️
unit 100.00% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

@aminghadersohi
aminghadersohi marked this pull request as ready for review June 12, 2026 21:20
@bito-code-review

bito-code-review Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #e866e5

Actionable Suggestions - 0
Additional Suggestions - 2
  • superset/mcp_service/dashboard/schemas.py - 1
    • Missing schema unit tests · Line 804-841
      DuplicateDashboardResponse lacks direct unit tests. While the error sanitization validator is exercised indirectly via integration tests, BITO.md rule [11731] requires dedicated unit tests for new MCP mutation tool schemas covering request validation, success paths, error handling, and schema serialization. Add tests to test_dashboard_schemas.py following the pattern used for GenerateDashboardResponse.
  • superset/mcp_service/dashboard/tool/duplicate_dashboard.py - 1
    • Inconsistent ctx logging on early returns · Line 164-177
      Early-return error paths at lines 164 and 171 return a DuplicateDashboardResponse with an error but emit no ctx-level log. Add await ctx.warning() calls before those returns for consistency with the DashboardForbiddenError handler and to ensure MCP-level observability for lookup failures.
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py - 1
Review Details
  • Files reviewed - 6 · Commit Range: 5495f98..5495f98
    • superset/mcp_service/app.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • superset/mcp_service/mcp_config.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_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

Comment thread tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
Comment thread tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
Comment thread tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
Comment thread tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
Comment thread tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
Comment thread tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py Outdated
@netlify

netlify Bot commented Jun 15, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 96fd06f
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a3d7c695a208c000853773e
😎 Deploy Preview https://deploy-preview-40959--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
@bito-code-review

bito-code-review Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #fe4939

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 5495f98..8ee4891
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.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

bito-code-review Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #9d6b93

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 8ee4891..5e9923e
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_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 mcp-duplicate-dashboard branch from 5e9923e to 96fd06f Compare June 25, 2026 19:07
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py Outdated
@bito-code-review

bito-code-review Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #98b6be

Actionable Suggestions - 0
Additional Suggestions - 1
  • tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py - 1
    • Missing test coverage · Line 878-878
      No test covers the `duplicate_slices` boolean field on `DuplicateDashboardRequest`. Verify the field defaults to `False` and optionally test non-boolean input rejection.
      Code suggestion
      --- a/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
      +++ b/tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
       @@ -885,3 +885,14 @@ class TestDuplicateDashboardRequestTitleSanitization:
                )
                assert req.sanitization_warnings == []
       
      +    def test_duplicate_slices_defaults_to_false(self) -> None:
      +        """The duplicate_slices field defaults to False."""
      +        req = DuplicateDashboardRequest(dashboard_id=1, dashboard_title="Test")
      +        assert req.duplicate_slices is False
      +
      +    def test_duplicate_slices_can_be_set_true(self) -> None:
      +        """The duplicate_slices field can be set to True."""
      +        req = DuplicateDashboardRequest(dashboard_id=1, dashboard_title="Test", duplicate_slices=True)
      +        assert req.duplicate_slices is True
Review Details
  • Files reviewed - 7 · Commit Range: 9e9005c..2c88975
    • superset/mcp_service/app.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • superset/mcp_service/mcp_config.py
    • tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_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 mcp-duplicate-dashboard branch from 2c88975 to 05dabda Compare June 26, 2026 14:40
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
Adds a duplicate_dashboard MCP tool that clones an existing dashboard
via CopyDashboardCommand. The source dashboard can be identified by
numeric ID, UUID, or slug. By default the copy references the same
charts; duplicate_slices=true deep-copies every chart into new objects
owned by the caller.

The tool builds the required json_metadata payload (source metadata
plus a positions key from position_json), mirroring what the frontend
"Save as" flow sends to the /copy/ endpoint. The new title is
sanitized for XSS, and the tool is excluded from MCP response caching.
Address review feedback: emit ctx.warning on not-found / access-denied
early returns for observability parity, and add dedicated schema unit
tests for DuplicateDashboardRequest sanitization and
DuplicateDashboardResponse error wrapping.
Address review feedback on duplicate_dashboard:
- Route the new dashboard's response through
  _sanitize_dashboard_info_for_llm_context so user-controlled title,
  description, chart, and tag text are wrapped as untrusted before
  reaching LLM context, matching the standard dashboard serializers.
- Fail fast when the source has charts but its layout maps none: an
  empty/invalid position_json would otherwise make set_dash_metadata
  rebuild the copy with zero slices, silently producing an empty copy.
- Add tests for both paths and docstrings/type hints on new test code.
The post-copy re-fetch's SQLAlchemyError handler returned a minimal
response without rolling back the failed transaction, leaving the shared
db.session in an invalid state for the rest of the request lifecycle.
Mirror the recovery pattern already used by generate_dashboard and
add_chart_to_existing_dashboard. Add a regression test asserting the
rollback runs and the fallback response is still produced.
CopyDashboardCommand re-parses the source's stored params/json_metadata
via set_dash_metadata; on malformed JSON this raises ValueError/
JSONDecodeError, which the transaction handler does not wrap as
DashboardCopyError (it only catches SQLAlchemyError). The error escaped
as a hard tool failure instead of a structured response. Catch the parse
error and return a normal DuplicateDashboardResponse error.

Also extract _resolve_source / _refetch_and_serialize / _safe_rollback
helpers to keep duplicate_dashboard under the C901 complexity limit, and
add a regression test for the malformed-metadata path.
…ilure

_resolve_source only caught DashboardNotFoundError/AccessDeniedError, so
a transient SQLAlchemyError from get_by_id_or_slug propagated to the broad
except Exception and re-raised as a hard tool failure. Catch SQLAlchemyError
during lookup, roll back, log with traceback, and return a generic
structured error (no DB-internal leak), matching generate_dashboard and
update_dashboard. Add a regression test.
@aminghadersohi
aminghadersohi force-pushed the mcp-duplicate-dashboard branch from d36e38c to 594d264 Compare June 26, 2026 15:44
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py Outdated
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py Outdated
@bito-code-review

bito-code-review Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #2e5f2d

Actionable Suggestions - 0
Additional Suggestions - 3
  • superset/mcp_service/dashboard/tool/duplicate_dashboard.py - 3
    • Inconsistent logging format · Line 377-379
      The ctx.error message at lines 377-379 omits `dashboard_id` while all other ctx.error calls in this function include it. This breaks logging consistency: compare "Dashboard duplication failed parsing source metadata: %s: %s" with the pattern at line 348 "Dashboard duplication forbidden: dashboard_id=%s". Include the dashboard_id for consistent structured logging.
      Code suggestion
      --- a/superset/mcp_service/dashboard/tool/duplicate_dashboard.py
      +++ b/superset/mcp_service/dashboard/tool/duplicate_dashboard.py
       @@ -374,8 +374,10 @@ async def duplicate_dashboard(
                # only wraps SQLAlchemyError. Return a structured response instead of
                # letting it escape as a hard tool failure.
                _safe_rollback("dashboard duplication")
                await ctx.error(
      -            "Dashboard duplication failed parsing source metadata: %s: %s"
      -            % (type(exc).__name__, str(exc))
      +            "Dashboard duplication failed parsing source metadata: dashboard_id=%s, %s: %s"
      +            % (request.dashboard_id, type(exc).__name__, str(exc))
                )
    • Missing test coverage for TypeError · Line 370-370
      The new `except (ValueError, TypeError)` handler at line 370 catches both exception types, but existing test `test_malformed_metadata_returns_structured_error` only covers ValueError (line 473). TypeError coverage is missing. Per project testing standards, all error paths in new MCP tools should be tested.
      Code suggestion
      --- a/tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
      +++ b/tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
       @@ -483,6 +483,38 @@ async def test_malformed_metadata_returns_structured_error(
            assert "metadata is invalid" in (content["error"] or "")
       
       
      +@patch("superset.commands.dashboard.copy.CopyDashboardCommand")
      +@patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
      +@pytest.mark.asyncio
      +async def test_type_error_in_copy_returns_structured_error(
      +    mock_get_by_id_or_slug: Mock,
      +    mock_copy_cmd_cls: Mock,
      +    mcp_server: object,
      +) -> None:
      +    """TypeError from copy command yields a structured error, not a crash.
      +
      +    Malformed stored metadata can also surface as a TypeError (e.g., when
      +    accessing None as a dict). This test ensures the ValueError/TypeError
      +    handler covers both exception types.
      +    """
      +    source = _mock_dashboard(id=2, slices=[_mock_chart(id=20)])
      +    mock_get_by_id_or_slug.return_value = source
      +    mock_copy_cmd_cls.return_value.run.side_effect = TypeError("'NoneType' object is not subscriptable")
      +
      +    with patch("superset.db.session"):
      +        async with Client(mcp_server) as client:
      +            result = await client.call_tool(
      +                "duplicate_dashboard",
      +                {"request": {"dashboard_id": 2, "dashboard_title": "Copy"}},
      +            )
      +
      +    content = result.structured_content
      +    assert content["dashboard"] is None
      +    assert "metadata is invalid" in (content["error"] or "")
      +
      +
        @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
        @pytest.mark.asyncio
        async def test_lookup_db_error_returns_structured_error(
    • Missing test for exception re-raise · Line 388-393
      The generic `except Exception as exc: ... raise` block at lines 388-393 logs the error then re-raises, but there's no test verifying this re-raise behavior. Adding a test ensures this fallback path is exercised.
      Code suggestion
      --- a/tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
      +++ b/tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
       @@ -518,6 +518,33 @@ async def test_lookup_db_error_returns_structured_error(
            assert "secret_table" not in error
       
       
      +@patch("superset.commands.dashboard.copy.CopyDashboardCommand")
      +@patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
      +@pytest.mark.asyncio
      +async def test_unexpected_exception_is_logged_and_re_raised(
      +    mock_get_by_id_or_slug: Mock,
      +    mock_copy_cmd_cls: Mock,
      +    mcp_server: object,
      +) -> None:
      +    """Unexpected exceptions are logged and re-raised, not caught silently.
      +
      +    The generic Exception handler logs the error then re-raises to ensure
      +    unexpected failures surface to the caller rather than returning a
      +    false success.
      +    """
      +    source = _mock_dashboard(id=3, slices=[_mock_chart(id=30)])
      +    mock_get_by_id_or_slug.return_value = source
      +    mock_copy_cmd_cls.return_value.run.side_effect = RuntimeError("unexpected failure")
      +
      +    with patch("superset.db.session"):
      +        async with Client(mcp_server) as client:
      +            with pytest.raises(RuntimeError, match="unexpected failure"):
      +                await client.call_tool(
      +                    "duplicate_dashboard",
      +                    {"request": {"dashboard_id": 3, "dashboard_title": "Copy"}},
      +                )
      +
      +
        def test_title_xss_only_rejected_by_schema() -> None:
Review Details
  • Files reviewed - 7 · Commit Range: 6ea70f3..594d264
    • superset/mcp_service/app.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • superset/mcp_service/mcp_config.py
    • tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_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

When the source dashboard's json_metadata cannot be decoded or is not a
JSON object, surface a non-fatal warning in the response rather than
silently defaulting to {{}}, so callers know the duplicate's aesthetic
settings (colors, native filters, etc.) were reset to defaults while
chart content was preserved. Adds a regression test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread superset/mcp_service/dashboard/schemas.py
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py Outdated
…n duplicate_dashboard

- Replace _positions_reference_charts (bool) with _get_layout_chart_ids
  (frozenset[int]) so callers can cross-reference against source.slices.
  A layout that references chart IDs that don't exist on the source would
  silently produce a copy with no charts; the tool now fails fast with a
  structured error and directs the user to repair the source dashboard.

- Remove the try/except that silently defaulted malformed json_metadata
  to {}.  A dashboard with unparseable json_metadata would lose its
  filter config, color scheme, and other settings in the copy; letting
  JSONDecodeError (a ValueError subclass) propagate to the outer
  (ValueError, TypeError) handler produces the same structured "metadata
  is invalid" error as the CopyDashboardCommand parse-failure path.

- Update tests: rename test_malformed_json_metadata_warns_not_fails to
  test_malformed_json_metadata_in_source_returns_structured_error and
  flip expectations from warn-and-succeed to fail-fast; add new test
  test_stale_layout_chart_ids_returns_structured_error.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread superset/mcp_service/dashboard/tool/duplicate_dashboard.py Outdated
``set_dash_metadata`` can raise ``KeyError`` when a position layout node
is missing expected keys (e.g. ``node['meta']['chartId']`` on a malformed
CHART entry). Add ``KeyError`` to the ``(ValueError, TypeError)`` handler
so a corrupted source layout produces a structured error response instead
of an unhandled exception. Also add ``dashboard_id`` to the ctx.error
log message in this handler for consistency with all other error branches.
Add regression test ``test_key_error_in_copy_command_returns_structured_error``.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread superset/mcp_service/dashboard/schemas.py
@aminghadersohi
aminghadersohi requested review from gabotorresruiz and rusackas and removed request for rebenitez1802 June 29, 2026 18:09
@rusackas

Copy link
Copy Markdown
Member

These two guards (empty/stale layout) don't exist on the REST /copy/ endpoint, which would just produce the copy. Dashboards created via API or import can legitimately carry slices without position_json entries; for those this tool now hard-fails where "Save as" would succeed. I'm fine with the fail-fast, but please call out this intentional divergence in the docstring/PR description.

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See above comment :)

@gabotorresruiz gabotorresruiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@bito-code-review

bito-code-review Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #c47b56

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 594d264..d8e1d7e
    • superset/mcp_service/dashboard/tool/duplicate_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ 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

@rusackas
rusackas merged commit c11fa20 into master Jun 30, 2026
59 checks passed
@rusackas
rusackas deleted the mcp-duplicate-dashboard branch June 30, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants