fix(mcp): re-read json_metadata from re-fetched dashboard before cleanup

Moves the json_metadata read from before UpdateDashboardCommand.run() to
after the re-fetch so cleanup is applied to the latest persisted state.
The old approach read a pre-command snapshot and wrote it back after the
re-fetch, silently overwriting any concurrent edits to dashboard metadata.

Updates test_json_metadata_cleanup to place json_metadata on the re-fetched
dashboard mock (updated_dashboard), reflecting the new read path.

superset/mcp_service/app.py

@@ -131,6 +131,7 @@ Dashboard Management:
 - generate_dashboard: Create a dashboard from chart IDs (requires write access)
 - update_dashboard: Update an existing dashboard's title/description/slug/published/layout/theme/CSS (requires write access; ownership-checked per-instance)
 - add_chart_to_existing_dashboard: Add a chart to an existing dashboard (requires write access)
+- remove_chart_from_dashboard: Remove a chart from an existing dashboard (requires write access)
 
 Annotation Layers:
 - list_annotation_layers: List annotation layers with advanced filters (1-based pagination)
@@ -425,7 +426,8 @@ Input format:
 {_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets,
   charts, or dashboards). SQL execution is a separate permission — see execute_sql below.
 - Write tools (generate_chart, generate_dashboard, update_chart, create_dataset, create_virtual_dataset,
-  save_sql_query, add_chart_to_existing_dashboard, update_chart_preview) require write
+  save_sql_query, add_chart_to_existing_dashboard, remove_chart_from_dashboard,
+  update_chart_preview) require write
   permissions. These tools are only listed for users who have the necessary access.
   If a write tool does not appear in the tool list, the current user lacks write access.
 - execute_sql requires SQL Lab access (execute_sql_query permission), which is separate
@@ -695,6 +697,7 @@ from superset.mcp_service.dashboard.tool import (  # noqa: F401, E402
     get_dashboard_info,
     get_dashboard_layout,
     list_dashboards,
+    remove_chart_from_dashboard,
     update_dashboard,
 )
 from superset.mcp_service.database.tool import (  # noqa: F401, E402

superset/mcp_service/dashboard/schemas.py

@@ -600,6 +600,58 @@ class AddChartToDashboardResponse(BaseModel):
         return sanitize_for_llm_context(value, field_path=("error",))
 
 
+class RemoveChartFromDashboardRequest(BaseModel):
+    """Request schema for removing a chart from an existing dashboard."""
+
+    dashboard_id: int = Field(
+        ..., description="ID of the dashboard to remove the chart from"
+    )
+    chart_id: int = Field(
+        ..., description="ID of the chart to remove from the dashboard"
+    )
+
+
+class RemoveChartFromDashboardResponse(BaseModel):
+    """Response schema for removing a chart from a dashboard."""
+
+    dashboard: DashboardInfo | None = Field(
+        None, description="The updated dashboard info, if successful"
+    )
+    dashboard_url: str | None = Field(
+        None, description="URL to view the updated dashboard"
+    )
+    removed_layout_keys: list[str] = Field(
+        default_factory=list,
+        description=(
+            "Layout component IDs that were removed from position_json "
+            "(the CHART components plus any ROW/COLUMN containers that "
+            "became empty as a result)."
+        ),
+    )
+    error: str | None = Field(None, description="Error message, if operation failed")
+    permission_denied: bool = Field(
+        default=False,
+        description=(
+            "True when the operation failed because the current user does not "
+            "have edit rights on the target dashboard. When True, inform the "
+            "user — do NOT attempt a workaround without confirming first."
+        ),
+    )
+
+    @field_validator("error")
+    @classmethod
+    def sanitize_error_for_llm_context(cls, value: str | None) -> str | None:
+        """Wrap error text before it is exposed to LLM context.
+
+        The error may echo dashboard-controlled text (e.g. the dashboard
+        title), which must be wrapped so the LLM treats it as data, not
+        instructions.
+        """
+        if value is None:
+            return value
+        return sanitize_for_llm_context(value, field_path=("error",))
+
+
 class GenerateDashboardRequest(BaseModel):
     """Request schema for generating a dashboard."""
 

superset/mcp_service/dashboard/tool/__init__.py

@@ -20,6 +20,7 @@ from .generate_dashboard import generate_dashboard
 from .get_dashboard_info import get_dashboard_info
 from .get_dashboard_layout import get_dashboard_layout
 from .list_dashboards import list_dashboards
+from .remove_chart_from_dashboard import remove_chart_from_dashboard
 from .update_dashboard import update_dashboard
 
 __all__ = [
@@ -28,5 +29,6 @@ __all__ = [
     "get_dashboard_layout",
     "generate_dashboard",
     "add_chart_to_existing_dashboard",
+    "remove_chart_from_dashboard",
     "update_dashboard",
 ]

superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py

@@ -0,0 +1,491 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+MCP tool: remove_chart_from_dashboard
+
+This tool removes a chart from an existing dashboard. It is the inverse of
+add_chart_to_existing_dashboard: it deletes the chart's CHART component(s)
+from position_json (pruning ROW/COLUMN containers that become empty),
+removes the chart from the dashboard's slices relationship, and cleans
+stale references to the chart from json_metadata (expanded_slices,
+timed_refresh_immune_slices, filter_scopes, default_filters).
+"""
+
+import logging
+from typing import Any, Dict
+
+from fastmcp import Context
+from sqlalchemy.exc import SQLAlchemyError
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.commands.exceptions import CommandException
+from superset.extensions import event_logger
+from superset.mcp_service.dashboard.schemas import (
+    DashboardInfo,
+    RemoveChartFromDashboardRequest,
+    RemoveChartFromDashboardResponse,
+    serialize_chart_summary,
+)
+from superset.mcp_service.privacy import user_can_view_data_model_metadata
+from superset.mcp_service.utils.url_utils import get_superset_base_url
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+# Container types that should be deleted once they have no children left.
+# TAB/TABS/GRID/ROOT containers are intentionally kept even when empty —
+# deleting a TAB would silently change the dashboard's visible structure.
+_PRUNABLE_TYPES = ("ROW", "COLUMN")
+
+
+def _find_chart_keys(layout: Dict[str, Any], chart_id: int) -> list[str]:
+    """Return all layout keys of CHART components referencing *chart_id*.
+
+    A chart can legitimately appear more than once in a layout (e.g. under
+    multiple tabs), so all occurrences are returned.
+    """
+    # Accept both int and string chartId — position_json is user/frontend-authored
+    # and imported or hand-edited layouts may store chartId as a string.
+    return [
+        key
+        for key, node in layout.items()
+        if isinstance(node, dict)
+        and node.get("type") == "CHART"
+        and (node.get("meta") or {}).get("chartId") in (chart_id, str(chart_id))
+    ]
+
+
+def _find_parent_key(layout: Dict[str, Any], component_key: str) -> str | None:
+    """Find the component whose children list contains *component_key*.
+
+    The reverse lookup scans children lists instead of trusting the
+    ``parents`` metadata on the node, which can be stale in hand-edited or
+    programmatically generated layouts.
+    """
+    for key, node in layout.items():
+        if not isinstance(node, dict):
+            continue
+        children = node.get("children")
+        if isinstance(children, list) and component_key in children:
+            return key
+    return None
+
+
+def _remove_component_and_prune(
+    layout: Dict[str, Any], component_key: str
+) -> list[str]:
+    """Remove *component_key* from the layout and prune empty containers.
+
+    Walks up the parent chain deleting ROW/COLUMN containers that become
+    empty as a result of the removal, so no orphaned wrapper nodes are left
+    behind. Returns the list of removed layout keys.
+    """
+    removed: list[str] = []
+    parent_key = _find_parent_key(layout, component_key)
+
+    layout.pop(component_key, None)
+    removed.append(component_key)
+
+    child_key = component_key
+    while parent_key is not None:
+        parent = layout.get(parent_key)
+        if not isinstance(parent, dict):
+            break
+        children = parent.get("children")
+        if isinstance(children, list):
+            parent["children"] = [c for c in children if c != child_key]
+        if parent.get("type") in _PRUNABLE_TYPES and not parent.get("children"):
+            grandparent_key = _find_parent_key(layout, parent_key)
+            layout.pop(parent_key, None)
+            removed.append(parent_key)
+            child_key = parent_key
+            parent_key = grandparent_key
+        else:
+            break
+
+    return removed
+
+
+def _remove_chart_from_layout(layout: Dict[str, Any], chart_id: int) -> list[str]:
+    """Remove every CHART component for *chart_id* from the layout.
+
+    Returns all removed layout keys (charts plus pruned containers).
+    """
+    removed: list[str] = []
+    for chart_key in _find_chart_keys(layout, chart_id):
+        # The chart key may already be gone if it shared a pruned container.
+        if chart_key in layout:
+            removed.extend(_remove_component_and_prune(layout, chart_key))
+    return removed
+
+
+def _remove_id_from_list(values: Any, chart_id: int) -> tuple[Any, bool]:
+    """Return (new_list, changed) with *chart_id* removed from a list of IDs.
+
+    Handles both int and str representations since json_metadata is
+    user/frontend-authored and not strictly typed.
+    """
+    if not isinstance(values, list):
+        return values, False
+    filtered = [v for v in values if v != chart_id and v != str(chart_id)]
+    return filtered, len(filtered) != len(values)
+
+
+def _clean_json_metadata(metadata: Dict[str, Any], chart_id: int) -> bool:
+    """Remove stale references to *chart_id* from a json_metadata dict.
+
+    Cleans ``expanded_slices`` (dict keyed by chart ID), ``filter_scopes``
+    (dict keyed by filter chart ID, with per-column ``immune`` ID lists),
+    ``timed_refresh_immune_slices`` (list of chart IDs), and
+    ``default_filters`` (a JSON-encoded string whose keys are chart IDs).
+    Mutates *metadata* in place and returns True when anything changed.
+    """
+    changed = False
+    chart_key = str(chart_id)
+
+    expanded_slices = metadata.get("expanded_slices")
+    if isinstance(expanded_slices, dict) and chart_key in expanded_slices:
+        del expanded_slices[chart_key]
+        changed = True
+
+    immune_slices, immune_changed = _remove_id_from_list(
+        metadata.get("timed_refresh_immune_slices"), chart_id
+    )
+    if immune_changed:
+        metadata["timed_refresh_immune_slices"] = immune_slices
+        changed = True
+
+    filter_scopes = metadata.get("filter_scopes")
+    if isinstance(filter_scopes, dict):
+        if chart_key in filter_scopes:
+            del filter_scopes[chart_key]
+            changed = True
+        for column_scopes in filter_scopes.values():
+            if not isinstance(column_scopes, dict):
+                continue
+            for column_config in column_scopes.values():
+                if not isinstance(column_config, dict):
+                    continue
+                immune, immune_changed = _remove_id_from_list(
+                    column_config.get("immune"), chart_id
+                )
+                if immune_changed:
+                    column_config["immune"] = immune
+                    changed = True
+
+    # default_filters is stored as a JSON-encoded string within json_metadata,
+    # with chart IDs as string keys (mirrors DashboardDAO.set_dash_metadata).
+    default_filters_raw = metadata.get("default_filters")
+    if isinstance(default_filters_raw, str):
+        try:
+            default_filters = json.loads(default_filters_raw)
+            if isinstance(default_filters, dict) and chart_key in default_filters:
+                del default_filters[chart_key]
+                metadata["default_filters"] = json.dumps(default_filters)
+                changed = True
+        except (json.JSONDecodeError, TypeError):
+            pass
+    elif isinstance(default_filters_raw, dict) and chart_key in default_filters_raw:
+        del default_filters_raw[chart_key]
+        changed = True
+
+    return changed
+
+
+def _find_and_authorize_dashboard(
+    dashboard_id: int,
+) -> tuple[Any, RemoveChartFromDashboardResponse | None]:
+    """Return (dashboard, None) on success or (None, error_response) on failure.
+
+    Handles both the not-found case and the ownership check so the main tool
+    function doesn't need two separate branches for these pre-conditions.
+    """
+    from superset import security_manager
+    from superset.daos.dashboard import DashboardDAO
+    from superset.exceptions import SupersetSecurityException
+
+    dashboard = DashboardDAO.find_by_id(dashboard_id)
+    if not dashboard:
+        return None, RemoveChartFromDashboardResponse(
+            dashboard=None,
+            dashboard_url=None,
+            error=(
+                f"Dashboard with ID {dashboard_id} not found."
+                " Use list_dashboards to get valid dashboard IDs."
+            ),
+        )
+
+    try:
+        security_manager.raise_for_ownership(dashboard)
+    except SupersetSecurityException:
+        return None, RemoveChartFromDashboardResponse(
+            dashboard=None,
+            dashboard_url=None,
+            permission_denied=True,
+            error=(
+                f"You don't have permission to edit dashboard "
+                f"'{dashboard.dashboard_title}' (ID: {dashboard_id}). "
+                "Inform the user and do not attempt a workaround without "
+                "their confirmation."
+            ),
+        )
+
+    return dashboard, None
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Dashboard",
+    method_permission_name="write",
+    annotations=ToolAnnotations(
+        title="Remove chart from dashboard",
+        readOnlyHint=False,
+        destructiveHint=True,
+    ),
+)
+def remove_chart_from_dashboard(  # noqa: C901 — complexity is structural (layout traversal + multi-step authorization), not accidental
+    request: RemoveChartFromDashboardRequest, ctx: Context
+) -> RemoveChartFromDashboardResponse:
+    """
+    Remove a chart from an existing dashboard.
+
+    Deletes the chart's layout component(s) from the dashboard (all
+    occurrences, including under tabs), prunes rows/columns left empty by
+    the removal, detaches the chart from the dashboard, and cleans stale
+    chart references from dashboard metadata (expanded_slices,
+    timed_refresh_immune_slices, filter_scopes, default_filters). The chart
+    itself is NOT deleted and remains available to other dashboards.
+    """
+    try:
+        from superset import db
+        from superset.commands.dashboard.update import UpdateDashboardCommand
+
+        # Validate dashboard exists and user has edit permission
+        with event_logger.log_context(
+            action="mcp.remove_chart_from_dashboard.validation"
+        ):
+            dashboard, auth_error = _find_and_authorize_dashboard(request.dashboard_id)
+            if auth_error is not None:
+                return auth_error
+
+        # Remove the chart from the layout tree
+        with event_logger.log_context(action="mcp.remove_chart_from_dashboard.layout"):
+            try:
+                current_layout = json.loads(dashboard.position_json or "{}")
+            except (json.JSONDecodeError, TypeError):
+                current_layout = {}
+            if not isinstance(current_layout, dict):
+                current_layout = {}
+
+            remaining_slices = [
+                slc for slc in dashboard.slices if slc.id != request.chart_id
+            ]
+            chart_in_slices = len(remaining_slices) != len(dashboard.slices)
+
+            removed_keys = _remove_chart_from_layout(current_layout, request.chart_id)
+
+            if not removed_keys and not chart_in_slices:
+                return RemoveChartFromDashboardResponse(
+                    dashboard=None,
+                    dashboard_url=None,
+                    error=(
+                        f"Chart {request.chart_id} is not in dashboard "
+                        f"{request.dashboard_id}. Use get_dashboard_info to "
+                        "see which charts the dashboard contains."
+                    ),
+                )
+
+        # Update the dashboard
+        with event_logger.log_context(
+            action="mcp.remove_chart_from_dashboard.db_write"
+        ):
+            update_data: dict[str, Any] = {
+                "position_json": json.dumps(current_layout),
+                "slices": remaining_slices,  # Pass ORM objects, not IDs
+            }
+
+            command = UpdateDashboardCommand(request.dashboard_id, update_data)
+            updated_dashboard = command.run()
+
+        # Re-fetch the dashboard with eager-loaded relationships to avoid
+        # "Instance is not bound to a Session" errors when serializing
+        # chart tags.  The preceding command.run() commit may
+        # invalidate the session in multi-tenant environments; on failure,
+        # return a minimal response using only scalar attributes that are
+        # already loaded — relationship fields (tags, slices) would
+        # trigger lazy-loading on the same dead session.
+        from sqlalchemy.orm import subqueryload
+
+        from superset.daos.dashboard import DashboardDAO
+        from superset.models.dashboard import Dashboard
+        from superset.models.slice import Slice
+
+        try:
+            updated_dashboard = (
+                DashboardDAO.find_by_id(
+                    updated_dashboard.id,
+                    query_options=[
+                        subqueryload(Dashboard.slices).subqueryload(Slice.tags),
+                        subqueryload(Dashboard.tags),
+                    ],
+                )
+                or updated_dashboard
+            )
+        except SQLAlchemyError:
+            logger.warning(
+                "Re-fetch of dashboard %s failed; returning minimal response",
+                updated_dashboard.id,
+                exc_info=True,
+            )
+            try:
+                db.session.rollback()  # pylint: disable=consider-using-transaction
+            except SQLAlchemyError:
+                logger.warning(
+                    "Database rollback failed during dashboard re-fetch error handling",
+                    exc_info=True,
+                )
+            dashboard_url = (
+                f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/"
+            )
+            return RemoveChartFromDashboardResponse(
+                dashboard=DashboardInfo(
+                    id=updated_dashboard.id,
+                    dashboard_title=updated_dashboard.dashboard_title,
+                    published=updated_dashboard.published,
+                    created_on=updated_dashboard.created_on,
+                    changed_on=updated_dashboard.changed_on,
+                    chart_count=len(remaining_slices),
+                    url=dashboard_url,
+                ),
+                dashboard_url=dashboard_url,
+                removed_layout_keys=removed_keys,
+                error=None,
+            )
+
+        # Clean stale chart references from json_metadata without routing
+        # through UpdateDashboardCommand: that path calls
+        # DashboardDAO.set_dash_metadata which, when "positions" is
+        # present in the metadata blob, overwrites dashboard.slices from
+        # layout data and silently drops charts attached via the slices
+        # relationship but absent from position_json.
+        #
+        # Read from the re-fetched dashboard so cleanup is applied to the
+        # latest persisted state rather than the pre-command snapshot,
+        # avoiding silent overwrites of concurrent metadata edits.
+        try:
+            metadata = json.loads(updated_dashboard.json_metadata or "{}")
+        except (json.JSONDecodeError, TypeError):
+            metadata = None
+        metadata_changed = isinstance(metadata, dict) and _clean_json_metadata(
+            metadata, request.chart_id
+        )
+
+        # Best-effort secondary write: the chart has already been removed from
+        # layout and slices (committed above). If this commit fails, log a
+        # warning but return success — stale metadata is preferable to
+        # reporting failure after a successful removal.
+        if metadata_changed and isinstance(metadata, dict):
+            from superset import db
+
+            try:
+                updated_dashboard.json_metadata = json.dumps(metadata)
+                db.session.commit()  # pylint: disable=consider-using-transaction
+            except SQLAlchemyError:
+                logger.warning(
+                    "json_metadata cleanup commit failed for dashboard %s after "
+                    "removing chart %s; chart removal succeeded",
+                    request.dashboard_id,
+                    request.chart_id,
+                    exc_info=True,
+                )
+                try:
+                    db.session.rollback()  # pylint: disable=consider-using-transaction
+                except SQLAlchemyError:
+                    logger.warning(
+                        "Rollback failed during json_metadata cleanup", exc_info=True
+                    )
+
+        # Convert to response format
+        from superset.mcp_service.dashboard.schemas import (
+            serialize_tag_object,
+        )
+
+        include_data_model_metadata = user_can_view_data_model_metadata()
+        dashboard_info = DashboardInfo(
+            id=updated_dashboard.id,
+            dashboard_title=updated_dashboard.dashboard_title,
+            slug=updated_dashboard.slug,
+            description=updated_dashboard.description,
+            published=updated_dashboard.published,
+            created_on=updated_dashboard.created_on,
+            changed_on=updated_dashboard.changed_on,
+            uuid=str(updated_dashboard.uuid) if updated_dashboard.uuid else None,
+            url=f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/",
+            chart_count=len(updated_dashboard.slices),
+            tags=[
+                serialize_tag_object(tag)
+                for tag in getattr(updated_dashboard, "tags", [])
+                if serialize_tag_object(tag) is not None
+            ],
+            charts=[
+                obj
+                for chart in getattr(updated_dashboard, "slices", [])
+                if (
+                    obj := serialize_chart_summary(
+                        chart,
+                        include_data_model_metadata=include_data_model_metadata,
+                    )
+                )
+                is not None
+            ],
+        )
+
+        dashboard_url = (
+            f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/"
+        )
+
+        logger.info(
+            "Removed chart %s from dashboard %s",
+            request.chart_id,
+            request.dashboard_id,
+        )
+
+        return RemoveChartFromDashboardResponse(
+            dashboard=dashboard_info,
+            dashboard_url=dashboard_url,
+            removed_layout_keys=removed_keys,
+            error=None,
+        )
+
+    except (CommandException, SQLAlchemyError, KeyError, ValueError) as e:
+        from superset import db
+
+        try:
+            db.session.rollback()  # pylint: disable=consider-using-transaction
+        except SQLAlchemyError:
+            logger.warning(
+                "Database rollback failed during error handling", exc_info=True
+            )
+        logger.error("Error removing chart from dashboard: %s", e)
+        return RemoveChartFromDashboardResponse(
+            dashboard=None,
+            dashboard_url=None,
+            removed_layout_keys=[],
+            permission_denied=False,
+            error=f"Failed to remove chart from dashboard: {str(e)}",
+        )

superset/mcp_service/docs/tool-search-optimization.md

@@ -34,7 +34,7 @@ Superset MCP tools are categorized with tags to help clients configure optimal l
 | `core` | Essential discovery and health tools | `health_check`, `get_instance_info`, `list_charts`, `list_dashboards`, `list_datasets` | Always load |
 | `discovery` | Detailed resource information and schema | `get_chart_info`, `get_dashboard_info`, `get_dataset_info`, `get_schema` | Can defer |
 | `data` | Data retrieval and previews | `get_chart_preview`, `get_chart_data` | Defer |
-| `mutate` | Create/modify resources | `generate_chart`, `update_chart`, `update_chart_preview`, `generate_dashboard`, `add_chart_to_existing_dashboard`, `execute_sql` | Defer |
+| `mutate` | Create/modify resources | `generate_chart`, `update_chart`, `update_chart_preview`, `generate_dashboard`, `add_chart_to_existing_dashboard`, `remove_chart_from_dashboard`, `execute_sql` | Defer |
 | `explore` | URL generation for exploration | `generate_explore_link`, `open_sql_lab_with_context` | Defer |
 
 ## Client Configuration

superset/mcp_service/middleware.py

@@ -1056,6 +1056,7 @@ class FieldPermissionsMiddleware(Middleware):
         "get_dashboard_info": "dashboard",
         "generate_dashboard": "dashboard",
         "add_chart_to_existing_dashboard": "dashboard",
+        "remove_chart_from_dashboard": "dashboard",
     }
 
     async def on_call_tool(

tests/unit_tests/mcp_service/dashboard/tool/test_remove_chart_from_dashboard.py

@@ -0,0 +1,656 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Unit tests for remove_chart_from_dashboard MCP tool.
+
+Follows the same pattern used in test_add_chart_to_existing_dashboard.py:
+- Tool flows run through the async MCP Client (not direct function calls)
+- Patches applied at source locations (superset.daos.dashboard.*, etc.)
+- auth is mocked via the autouse mock_auth fixture
+
+Covers:
+- Dashboard not found
+- Permission denied (user does not own the dashboard) -> permission_denied=True
+- Chart not present in the dashboard
+- Simple grid removal (chart directly inside a ROW) with empty-row pruning
+- Chart inside a COLUMN (sibling survives; lone chart prunes COLUMN + ROW)
+- Tabbed layout where the chart appears under multiple tabs
+- json_metadata cleanup (expanded_slices, timed_refresh_immune_slices,
+  filter_scopes, default_filters)
+"""
+
+import logging
+from collections.abc import Generator
+from typing import Any
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.dashboard.tool.remove_chart_from_dashboard import (
+    _clean_json_metadata,
+    _remove_chart_from_layout,
+)
+from superset.utils import json
+
+logging.basicConfig(level=logging.DEBUG)
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+def mcp_server() -> object:
+    """Return the FastMCP app instance for use in MCP client tests."""
+    return mcp
+
+
+@pytest.fixture(autouse=True)
+def mock_auth() -> Generator[Mock, None, None]:
+    """Mock authentication for all tests."""
+    with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user:
+        mock_user = Mock()
+        mock_user.id = 1
+        mock_user.username = "admin"
+        mock_get_user.return_value = mock_user
+        yield mock_get_user
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _mock_chart(id: int = 10, slice_name: str = "Test Chart") -> Mock:
+    """Create a minimal mock Slice object with the given ID and name."""
+    chart = Mock()
+    chart.id = id
+    chart.slice_name = slice_name
+    chart.uuid = f"chart-uuid-{id}"
+    chart.tags = []
+    chart.owners = []
+    chart.viz_type = "table"
+    chart.datasource_name = None
+    chart.description = None
+    return chart
+
+
+def _mock_dashboard(
+    id: int = 1,
+    title: str = "Sales Dashboard",
+    slices: list[Mock] | None = None,
+    position_json: str = "{}",
+    json_metadata: str | None = None,
+) -> Mock:
+    """Create a minimal mock Dashboard object."""
+    dashboard = Mock()
+    dashboard.id = id
+    dashboard.dashboard_title = title
+    dashboard.slug = f"test-dashboard-{id}"
+    dashboard.description = None
+    dashboard.published = True
+    dashboard.created_on = None
+    dashboard.changed_on = None
+    dashboard.created_by_name = "test_user"
+    dashboard.changed_by_name = "test_user"
+    dashboard.uuid = f"dashboard-uuid-{id}"
+    dashboard.slices = slices or []
+    dashboard.owners = []
+    dashboard.tags = []
+    dashboard.roles = []
+    dashboard.position_json = position_json
+    dashboard.json_metadata = json_metadata
+    dashboard.css = None
+    dashboard.certified_by = None
+    dashboard.certification_details = None
+    dashboard.is_managed_externally = False
+    dashboard.external_url = None
+    return dashboard
+
+
+def _chart_node(key: str, chart_id: int, parents: list[str]) -> dict[str, Any]:
+    """Build a minimal CHART layout node dict for use in test layouts."""
+    return {
+        "children": [],
+        "id": key,
+        "meta": {"chartId": chart_id, "height": 50, "width": 4},
+        "parents": parents,
+        "type": "CHART",
+    }
+
+
+def _simple_grid_layout() -> dict[str, Any]:
+    """ROOT > GRID > [ROW-1 > CHART-10, ROW-2 > CHART-20]."""
+    return {
+        "DASHBOARD_VERSION_KEY": "v2",
+        "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+        "GRID_ID": {
+            "children": ["ROW-1", "ROW-2"],
+            "id": "GRID_ID",
+            "parents": ["ROOT_ID"],
+            "type": "GRID",
+        },
+        "ROW-1": {
+            "children": ["CHART-aaa"],
+            "id": "ROW-1",
+            "meta": {},
+            "parents": ["ROOT_ID", "GRID_ID"],
+            "type": "ROW",
+        },
+        "CHART-aaa": _chart_node("CHART-aaa", 10, ["ROOT_ID", "GRID_ID", "ROW-1"]),
+        "ROW-2": {
+            "children": ["CHART-bbb"],
+            "id": "ROW-2",
+            "meta": {},
+            "parents": ["ROOT_ID", "GRID_ID"],
+            "type": "ROW",
+        },
+        "CHART-bbb": _chart_node("CHART-bbb", 20, ["ROOT_ID", "GRID_ID", "ROW-2"]),
+    }
+
+
+def _column_layout(column_children: list[tuple[str, int]]) -> dict[str, Any]:
+    """ROOT > GRID > ROW-1 > COLUMN-1 > [charts]."""
+    layout = {
+        "DASHBOARD_VERSION_KEY": "v2",
+        "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+        "GRID_ID": {
+            "children": ["ROW-1"],
+            "id": "GRID_ID",
+            "parents": ["ROOT_ID"],
+            "type": "GRID",
+        },
+        "ROW-1": {
+            "children": ["COLUMN-1"],
+            "id": "ROW-1",
+            "meta": {},
+            "parents": ["ROOT_ID", "GRID_ID"],
+            "type": "ROW",
+        },
+        "COLUMN-1": {
+            "children": [key for key, _ in column_children],
+            "id": "COLUMN-1",
+            "meta": {},
+            "parents": ["ROOT_ID", "GRID_ID", "ROW-1"],
+            "type": "COLUMN",
+        },
+    }
+    for key, chart_id in column_children:
+        layout[key] = _chart_node(
+            key, chart_id, ["ROOT_ID", "GRID_ID", "ROW-1", "COLUMN-1"]
+        )
+    return layout
+
+
+def _tabbed_layout() -> dict[str, Any]:
+    """ROOT > TABS > [TAB-1 > ROW-1 > CHART(10), TAB-2 > ROW-2 > CHART(10)]."""
+    return {
+        "DASHBOARD_VERSION_KEY": "v2",
+        "ROOT_ID": {"children": ["TABS-1"], "id": "ROOT_ID", "type": "ROOT"},
+        "TABS-1": {
+            "children": ["TAB-1", "TAB-2"],
+            "id": "TABS-1",
+            "meta": {},
+            "parents": ["ROOT_ID"],
+            "type": "TABS",
+        },
+        "TAB-1": {
+            "children": ["ROW-1"],
+            "id": "TAB-1",
+            "meta": {"text": "First"},
+            "parents": ["ROOT_ID", "TABS-1"],
+            "type": "TAB",
+        },
+        "ROW-1": {
+            "children": ["CHART-aaa"],
+            "id": "ROW-1",
+            "meta": {},
+            "parents": ["ROOT_ID", "TABS-1", "TAB-1"],
+            "type": "ROW",
+        },
+        "CHART-aaa": _chart_node(
+            "CHART-aaa", 10, ["ROOT_ID", "TABS-1", "TAB-1", "ROW-1"]
+        ),
+        "TAB-2": {
+            "children": ["ROW-2"],
+            "id": "TAB-2",
+            "meta": {"text": "Second"},
+            "parents": ["ROOT_ID", "TABS-1"],
+            "type": "TAB",
+        },
+        "ROW-2": {
+            "children": ["CHART-ccc", "CHART-bbb"],
+            "id": "ROW-2",
+            "meta": {},
+            "parents": ["ROOT_ID", "TABS-1", "TAB-2"],
+            "type": "ROW",
+        },
+        "CHART-ccc": _chart_node(
+            "CHART-ccc", 10, ["ROOT_ID", "TABS-1", "TAB-2", "ROW-2"]
+        ),
+        "CHART-bbb": _chart_node(
+            "CHART-bbb", 20, ["ROOT_ID", "TABS-1", "TAB-2", "ROW-2"]
+        ),
+    }
+
+
+async def _call_remove(
+    mcp_server: object, dashboard_id: int = 1, chart_id: int = 10
+) -> dict[str, Any]:
+    """Call remove_chart_from_dashboard via MCP client; return structured content."""
+    async with Client(mcp_server) as client:
+        result = await client.call_tool(
+            "remove_chart_from_dashboard",
+            {"request": {"dashboard_id": dashboard_id, "chart_id": chart_id}},
+        )
+    return result.structured_content
+
+
+# ---------------------------------------------------------------------------
+# Error-path tests
+# ---------------------------------------------------------------------------
+
+
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_dashboard_not_found(mock_find_by_id: Mock, mcp_server: object) -> None:
+    """Returns a clear error when the target dashboard does not exist."""
+    mock_find_by_id.return_value = None
+
+    content = await _call_remove(mcp_server, dashboard_id=999)
+
+    assert content["dashboard"] is None
+    assert content["dashboard_url"] is None
+    assert content["permission_denied"] is False
+    assert "not found" in (content["error"] or "").lower()
+
+
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_permission_denied(
+    mock_find_by_id: Mock, mock_raise_for_ownership: Mock, mcp_server: object
+) -> None:
+    """Returns permission_denied=True when the user cannot edit the dashboard."""
+    from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+    from superset.exceptions import SupersetSecurityException
+
+    dashboard = _mock_dashboard(id=1, title="Sales Dashboard")
+    mock_find_by_id.return_value = dashboard
+    mock_raise_for_ownership.side_effect = SupersetSecurityException(
+        SupersetError(
+            message="Changing this Dashboard is forbidden",
+            error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
+            level=ErrorLevel.ERROR,
+        )
+    )
+
+    content = await _call_remove(mcp_server)
+
+    assert content["dashboard"] is None
+    assert content["permission_denied"] is True
+    assert content["error"] is not None
+    assert "Sales Dashboard" in content["error"]
+    assert "permission" in content["error"].lower()
+
+
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_chart_not_in_dashboard(
+    mock_find_by_id: Mock, mock_raise_for_ownership: Mock, mcp_server: object
+) -> None:
+    """Returns an error when the chart is in neither layout nor slices."""
+    other_chart = _mock_chart(id=20)
+    dashboard = _mock_dashboard(
+        slices=[other_chart], position_json=json.dumps(_simple_grid_layout())
+    )
+    mock_find_by_id.return_value = dashboard
+    mock_raise_for_ownership.return_value = None
+
+    content = await _call_remove(mcp_server, chart_id=99)
+
+    assert content["dashboard"] is None
+    assert content["permission_denied"] is False
+    assert "99" in (content["error"] or "")
+    assert "not in dashboard" in (content["error"] or "")
+
+
+# ---------------------------------------------------------------------------
+# Success-path tests (layout + slices + json_metadata assertions via the
+# captured UpdateDashboardCommand payload)
+# ---------------------------------------------------------------------------
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_simple_grid_removal_prunes_empty_row(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """Removing a chart that is the only child of a ROW also prunes the ROW."""
+    chart_10 = _mock_chart(id=10)
+    chart_20 = _mock_chart(id=20)
+    dashboard = _mock_dashboard(
+        slices=[chart_10, chart_20],
+        position_json=json.dumps(_simple_grid_layout()),
+    )
+    updated_dashboard = _mock_dashboard(id=1, slices=[chart_20])
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+    assert content["permission_denied"] is False
+    assert content["dashboard_url"] is not None
+    assert "/superset/dashboard/1/" in content["dashboard_url"]
+    assert set(content["removed_layout_keys"]) == {"CHART-aaa", "ROW-1"}
+
+    dashboard_id, update_data = mock_update_cmd_cls.call_args.args
+    assert dashboard_id == 1
+    new_layout = json.loads(update_data["position_json"])
+    assert "CHART-aaa" not in new_layout
+    assert "ROW-1" not in new_layout
+    assert new_layout["GRID_ID"]["children"] == ["ROW-2"]
+    assert "CHART-bbb" in new_layout
+    assert update_data["slices"] == [chart_20]
+    # No stale metadata references -> json_metadata untouched
+    assert "json_metadata" not in update_data
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_chart_in_column_keeps_sibling(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """Removing one chart from a COLUMN keeps the COLUMN and its sibling."""
+    chart_10 = _mock_chart(id=10)
+    chart_20 = _mock_chart(id=20)
+    layout = _column_layout([("CHART-aaa", 10), ("CHART-bbb", 20)])
+    dashboard = _mock_dashboard(
+        slices=[chart_10, chart_20], position_json=json.dumps(layout)
+    )
+    updated_dashboard = _mock_dashboard(id=1, slices=[chart_20])
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+    assert content["removed_layout_keys"] == ["CHART-aaa"]
+
+    _, update_data = mock_update_cmd_cls.call_args.args
+    new_layout = json.loads(update_data["position_json"])
+    assert "CHART-aaa" not in new_layout
+    assert new_layout["COLUMN-1"]["children"] == ["CHART-bbb"]
+    assert new_layout["ROW-1"]["children"] == ["COLUMN-1"]
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_lone_chart_in_column_prunes_column_and_row(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """Removing the only chart in a COLUMN prunes the COLUMN and its ROW."""
+    chart_10 = _mock_chart(id=10)
+    layout = _column_layout([("CHART-aaa", 10)])
+    dashboard = _mock_dashboard(slices=[chart_10], position_json=json.dumps(layout))
+    updated_dashboard = _mock_dashboard(id=1, slices=[])
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+    assert set(content["removed_layout_keys"]) == {"CHART-aaa", "COLUMN-1", "ROW-1"}
+
+    _, update_data = mock_update_cmd_cls.call_args.args
+    new_layout = json.loads(update_data["position_json"])
+    for key in ("CHART-aaa", "COLUMN-1", "ROW-1"):
+        assert key not in new_layout
+    assert new_layout["GRID_ID"]["children"] == []
+    # GRID/ROOT containers are never pruned
+    assert "GRID_ID" in new_layout
+    assert "ROOT_ID" in new_layout
+    assert update_data["slices"] == []
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_tabbed_layout_removes_all_occurrences(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """A chart appearing under multiple tabs is removed everywhere; tabs stay."""
+    chart_10 = _mock_chart(id=10)
+    chart_20 = _mock_chart(id=20)
+    dashboard = _mock_dashboard(
+        slices=[chart_10, chart_20], position_json=json.dumps(_tabbed_layout())
+    )
+    updated_dashboard = _mock_dashboard(id=1, slices=[chart_20])
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+    # CHART-aaa was ROW-1's only child (ROW-1 pruned); CHART-ccc shared
+    # ROW-2 with CHART-bbb (ROW-2 kept).
+    assert set(content["removed_layout_keys"]) == {"CHART-aaa", "ROW-1", "CHART-ccc"}
+
+    _, update_data = mock_update_cmd_cls.call_args.args
+    new_layout = json.loads(update_data["position_json"])
+    for key in ("CHART-aaa", "ROW-1", "CHART-ccc"):
+        assert key not in new_layout
+    # Tabs are never pruned, even when emptied
+    assert "TAB-1" in new_layout
+    assert new_layout["TAB-1"]["children"] == []
+    assert "TAB-2" in new_layout
+    assert new_layout["ROW-2"]["children"] == ["CHART-bbb"]
+    assert update_data["slices"] == [chart_20]
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_json_metadata_cleanup(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """Stale chart references are removed from json_metadata on removal."""
+    chart_10 = _mock_chart(id=10)
+    chart_20 = _mock_chart(id=20)
+    metadata = {
+        "expanded_slices": {"10": True, "20": True},
+        "timed_refresh_immune_slices": [10, 20],
+        "filter_scopes": {
+            "10": {"col": {"scope": ["ROOT_ID"], "immune": []}},
+            "30": {"region": {"scope": ["ROOT_ID"], "immune": [10, 20]}},
+        },
+        "default_filters": json.dumps({"10": {"col": ["val"]}, "20": {"col": ["v2"]}}),
+        "refresh_frequency": 300,
+    }
+    dashboard = _mock_dashboard(
+        slices=[chart_10, chart_20],
+        position_json=json.dumps(_simple_grid_layout()),
+    )
+    # json_metadata is read from the re-fetched dashboard (updated_dashboard)
+    # to avoid overwriting concurrent metadata edits.
+    updated_dashboard = _mock_dashboard(
+        id=1, slices=[chart_20], json_metadata=json.dumps(metadata)
+    )
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+
+    _, update_data = mock_update_cmd_cls.call_args.args
+    # json_metadata is NOT routed through UpdateDashboardCommand to avoid
+    # set_dash_metadata overwriting slices from layout data.
+    assert "json_metadata" not in update_data
+    # Cleaned metadata is written directly to updated_dashboard.json_metadata.
+    new_metadata = json.loads(updated_dashboard.json_metadata)
+    assert new_metadata["expanded_slices"] == {"20": True}
+    assert new_metadata["timed_refresh_immune_slices"] == [20]
+    assert "10" not in new_metadata["filter_scopes"]
+    assert new_metadata["filter_scopes"]["30"]["region"]["immune"] == [20]
+    # default_filters entry for chart 10 is removed; entry for 20 survives
+    new_default_filters = json.loads(new_metadata["default_filters"])
+    assert "10" not in new_default_filters
+    assert "20" in new_default_filters
+    # Unrelated keys are preserved
+    assert new_metadata["refresh_frequency"] == 300
+    # "positions" is never injected into json_metadata.
+    assert "positions" not in new_metadata
+
+
+@patch("superset.commands.dashboard.update.UpdateDashboardCommand")
+@patch("superset.security_manager.raise_for_ownership")
+@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+@pytest.mark.asyncio
+async def test_chart_in_slices_but_not_in_layout(
+    mock_find_by_id: Mock,
+    mock_raise_for_ownership: Mock,
+    mock_update_cmd_cls: Mock,
+    mcp_server: object,
+) -> None:
+    """A chart attached to the dashboard but absent from the layout is
+    still detached from the slices relationship."""
+    chart_10 = _mock_chart(id=10)
+    dashboard = _mock_dashboard(slices=[chart_10], position_json="{}")
+    updated_dashboard = _mock_dashboard(id=1, slices=[])
+    mock_find_by_id.side_effect = [dashboard, updated_dashboard]
+    mock_raise_for_ownership.return_value = None
+
+    mock_update_cmd = Mock()
+    mock_update_cmd.run.return_value = updated_dashboard
+    mock_update_cmd_cls.return_value = mock_update_cmd
+
+    content = await _call_remove(mcp_server, chart_id=10)
+
+    assert content["error"] is None
+    assert content["removed_layout_keys"] == []
+
+    _, update_data = mock_update_cmd_cls.call_args.args
+    assert update_data["slices"] == []
+
+
+# ---------------------------------------------------------------------------
+# Synchronous helper tests
+# ---------------------------------------------------------------------------
+
+
+def test_remove_chart_from_layout_ignores_other_charts() -> None:
+    """Removing a chart ID that is not in the layout is a no-op."""
+    layout = _simple_grid_layout()
+    removed = _remove_chart_from_layout(layout, 99)
+    assert removed == []
+    assert layout == _simple_grid_layout()
+
+
+def test_clean_json_metadata_no_changes_returns_false() -> None:
+    """Metadata without references to the chart is left untouched."""
+    metadata = {
+        "expanded_slices": {"20": True},
+        "timed_refresh_immune_slices": [20],
+        "color_scheme": "supersetColors",
+    }
+    assert _clean_json_metadata(metadata, 10) is False
+    assert metadata == {
+        "expanded_slices": {"20": True},
+        "timed_refresh_immune_slices": [20],
+        "color_scheme": "supersetColors",
+    }
+
+
+def test_clean_json_metadata_handles_string_ids_in_lists() -> None:
+    """timed_refresh_immune_slices entries may be string-typed IDs."""
+    metadata = {"timed_refresh_immune_slices": ["10", 20]}
+    assert _clean_json_metadata(metadata, 10) is True
+    assert metadata["timed_refresh_immune_slices"] == [20]
+
+
+def test_clean_json_metadata_cleans_default_filters() -> None:
+    """default_filters entries for the removed chart are pruned."""
+    metadata = {
+        "default_filters": json.dumps({"10": {"col": ["v"]}, "20": {"col": ["v2"]}}),
+    }
+    assert _clean_json_metadata(metadata, 10) is True
+    remaining = json.loads(metadata["default_filters"])
+    assert "10" not in remaining
+    assert "20" in remaining
+
+
+def test_clean_json_metadata_handles_malformed_sections() -> None:
+    """Malformed metadata sections are skipped without raising."""
+    metadata = {
+        "expanded_slices": "not-a-dict",
+        "timed_refresh_immune_slices": {"not": "a-list"},
+        "filter_scopes": {"10": "not-a-dict", "30": {"col": "not-a-dict"}},
+    }
+    assert _clean_json_metadata(metadata, 10) is True  # filter_scopes["10"] removed
+    assert "10" not in metadata["filter_scopes"]
+    assert metadata["expanded_slices"] == "not-a-dict"