codeant-ai-for-open-source[bot] commented on code in PR #40995:
URL: https://github.com/apache/superset/pull/40995#discussion_r3402230225


##########
superset/mcp_service/dashboard/tool/add_native_filter.py:
##########
@@ -0,0 +1,501 @@
+# 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: add_native_filter
+
+This tool adds a native filter to an existing dashboard by updating
+the dashboard's json_metadata.native_filter_configuration.
+"""
+
+import logging
+import uuid
+from datetime import datetime, timezone
+from typing import Any, Dict, List, Literal
+
+from fastmcp import Context
+from pydantic import BaseModel, Field
+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 (
+    DashboardError,
+    DashboardInfo,
+    NativeFilterSummary,
+)
+from superset.mcp_service.utils.url_utils import get_superset_base_url
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Request / Response schemas
+# ---------------------------------------------------------------------------
+
+
+class AddNativeFilterRequest(BaseModel):
+    """Request schema for adding a native filter to a dashboard."""
+
+    dashboard_id: int = Field(
+        ..., description="ID of the dashboard to add the filter to"
+    )
+    filter_name: str = Field(
+        ...,
+        description="Display name for the filter (e.g. 'Ngành hàng', 'Thị 
trường')",
+    )
+    filter_type: Literal[
+        "filter_select",
+        "filter_range",
+        "filter_time",
+        "filter_timecolumn",
+        "filter_timegrain",
+    ] = Field(
+        ...,
+        description=(
+            "Type of native filter to create. "
+            "filter_select: dropdown/multi-select for categorical columns. "
+            "filter_range: numeric range slider. "
+            "filter_time: date/time range picker. "
+            "filter_timecolumn: select which time column to use. "
+            "filter_timegrain: select time grain (day/week/month)."
+        ),
+    )
+    dataset_id: int = Field(
+        ..., description="ID of the dataset the filter targets"
+    )
+    column_name: str = Field(
+        ...,
+        description="Name of the column in the dataset to filter on",
+    )
+    default_value: Any = Field(
+        default=None,
+        description=(
+            "Default selected value(s) for the filter. "
+            "For filter_select with multiple_select=True, use a list. "
+            "For filter_range, use a dict like {'min': 0, 'max': 100}."
+        ),
+    )
+    parent_filter_ids: List[str] = Field(
+        default_factory=list,
+        description=(
+            "List of parent native filter IDs for cascading filters. "
+            "The parent filters must already exist on the dashboard."
+        ),
+    )
+    scope_type: Literal["full", "specific"] = Field(
+        default="full",
+        description=(
+            "Filter scope. 'full' applies the filter to all charts on the "
+            "dashboard (default). 'specific' applies only to charts NOT in "
+            "excluded_chart_ids."
+        ),
+    )
+    excluded_chart_ids: List[int] = Field(
+        default_factory=list,
+        description=(
+            "Chart IDs to exclude from this filter's scope. "
+            "Only used when scope_type='specific'."
+        ),
+    )
+    enable_search: bool = Field(
+        default=True,
+        description="Enable search in select filter dropdown (filter_select 
only)",
+    )
+    multiple_select: bool = Field(
+        default=True,
+        description="Allow selecting multiple values (filter_select only)",
+    )
+
+
+class AddNativeFilterResponse(BaseModel):
+    """Response schema for adding a native filter."""
+
+    filter_id: str | None = Field(
+        None, description="ID of the newly created native filter"
+    )
+    filter_name: str | None = Field(
+        None, description="Display name of the created filter"
+    )
+    filter_type: str | None = Field(
+        None, description="Type of the created filter"
+    )
+    native_filters: List[NativeFilterSummary] = Field(
+        default_factory=list,
+        description="Updated list of all native filters on the dashboard",
+    )
+    dashboard_id: int | None = Field(
+        None, description="ID of the updated dashboard"
+    )
+    dashboard_url: str | None = Field(
+        None, description="URL to view the updated dashboard"
+    )
+    error: str | None = Field(
+        None, description="Error message if operation failed"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Helper functions
+# ---------------------------------------------------------------------------
+
+
+def _generate_filter_id() -> str:
+    """Generate a unique native filter ID matching Superset's frontend 
format."""
+    return f"NATIVE_FILTER-{uuid.uuid4().hex[:20]}"
+
+
+def _build_scope(
+    scope_type: str,
+    excluded_chart_ids: List[int],
+) -> Dict[str, Any]:
+    """Build the scope object for a native filter.
+
+    Args:
+        scope_type: 'full' for all charts, 'specific' for excluding some.
+        excluded_chart_ids: Chart IDs to exclude when scope_type='specific'.
+
+    Returns:
+        Scope dict compatible with Superset's native filter format.
+    """
+    if scope_type == "specific" and excluded_chart_ids:
+        return {
+            "rootPath": ["ROOT_ID"],
+            "excluded": excluded_chart_ids,
+        }
+    # Default: apply to all charts
+    return {
+        "rootPath": ["ROOT_ID"],
+        "excluded": [0],
+    }
+
+
+def _build_control_values(
+    filter_type: str,
+    enable_search: bool,
+    multiple_select: bool,
+) -> Dict[str, Any]:
+    """Build controlValues for the filter based on its type."""
+    if filter_type == "filter_select":
+        return {
+            "enableEmptyFilter": False,
+            "defaultToFirstItem": False,
+            "multiSelect": multiple_select,
+            "searchAllOptions": enable_search,
+            "inverseSelection": False,
+        }
+    elif filter_type == "filter_range":
+        return {
+            "enableEmptyFilter": False,
+        }
+    elif filter_type in ("filter_time", "filter_timecolumn", 
"filter_timegrain"):
+        return {}
+    return {}
+
+
+def _build_default_data_mask(
+    filter_type: str,
+    default_value: Any,
+) -> Dict[str, Any]:
+    """Build defaultDataMask for the filter."""
+    if default_value is None:
+        return {
+            "filterState": {},
+            "extraFormData": {},
+        }
+
+    if filter_type == "filter_select":
+        # default_value should be a list or single value
+        values = default_value if isinstance(default_value, list) else 
[default_value]
+        return {
+            "filterState": {"value": values},
+            "extraFormData": {
+                "filters": [
+                    {
+                        "col": None,  # Will be set by Superset frontend
+                        "op": "IN",
+                        "val": values,
+                    }
+                ]
+            },
+        }
+    elif filter_type == "filter_range":
+        # default_value should be dict with min/max
+        filter_state = {}
+        if isinstance(default_value, dict):
+            filter_state = {"value": [default_value.get("min"), 
default_value.get("max")]}
+        return {
+            "filterState": filter_state,
+            "extraFormData": {},
+        }
+    elif filter_type == "filter_time":
+        return {
+            "filterState": {"value": default_value},
+            "extraFormData": {"time_range": default_value},
+        }
+    return {
+        "filterState": {},
+        "extraFormData": {},
+    }
+
+
+def _build_native_filter_config(
+    request: AddNativeFilterRequest,
+    filter_id: str,
+) -> Dict[str, Any]:
+    """Build the complete native filter configuration dict."""
+    return {
+        "id": filter_id,
+        "name": request.filter_name,
+        "filterType": request.filter_type,
+        "targets": [
+            {
+                "datasetId": request.dataset_id,
+                "column": {"name": request.column_name},
+            }
+        ],

Review Comment:
   **Suggestion:** The request column is written directly into filter targets 
without verifying the column exists on the dataset, so invalid input can be 
persisted as a "successful" filter that later fails/does nothing at runtime. 
Validate `column_name` against dataset columns before building the filter 
config. [incomplete implementation]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Native filters can reference non-existent dataset columns.
   - ⚠️ Dashboards may show filters that never affect data.
   - ⚠️ Troubleshooting broken filters becomes harder for operators.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. A client submits an `AddNativeFilterRequest` with `dataset_id` pointing 
to a real
   dataset but `column_name` containing a typo or non-existent column name, as 
accepted by
   the schema at `add_native_filter.py:80-86`.
   
   2. During validation, the tool only checks that the dataset exists via 
`dataset =
   db.session.get(SqlaTable, request.dataset_id)` and the `if not dataset:` 
guard at
   `add_native_filter.py:403-407`; it does not inspect `dataset.columns` or 
otherwise verify
   `request.column_name`.
   
   3. `_build_native_filter_config` builds the filter targets at
   `add_native_filter.py:267-272`, unconditionally emitting `"column": {"name":
   request.column_name}` regardless of whether that column exists on the 
`SqlaTable`
   instance.
   
   4. The resulting configuration is persisted into `dashboard.json_metadata` 
through
   `UpdateDashboardCommand.run()` at `add_native_filter.py:449-455` and
   `superset/commands/dashboard/update.py:58-79`, so dashboards can store 
native filters
   whose target column is invalid, leading to filters that fail or silently do 
nothing when
   the dashboard is rendered.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=282ba93bb6504a5ea1ad9eeecfb26a74&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=282ba93bb6504a5ea1ad9eeecfb26a74&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/dashboard/tool/add_native_filter.py
   **Line:** 267:272
   **Comment:**
        *Incomplete Implementation: The request column is written directly into 
filter targets without verifying the column exists on the dataset, so invalid 
input can be persisted as a "successful" filter that later fails/does nothing 
at runtime. Validate `column_name` against dataset columns before building the 
filter config.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=c909d3564d5c7f4983fdea97ce4dc3b8129cbc60b98ba8ba4d799d343c32e9e3&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=c909d3564d5c7f4983fdea97ce4dc3b8129cbc60b98ba8ba4d799d343c32e9e3&reaction=dislike'>👎</a>



##########
superset/mcp_service/dashboard/tool/add_native_filter.py:
##########
@@ -0,0 +1,501 @@
+# 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: add_native_filter
+
+This tool adds a native filter to an existing dashboard by updating
+the dashboard's json_metadata.native_filter_configuration.
+"""
+
+import logging
+import uuid
+from datetime import datetime, timezone
+from typing import Any, Dict, List, Literal
+
+from fastmcp import Context
+from pydantic import BaseModel, Field
+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 (
+    DashboardError,
+    DashboardInfo,
+    NativeFilterSummary,
+)
+from superset.mcp_service.utils.url_utils import get_superset_base_url
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Request / Response schemas
+# ---------------------------------------------------------------------------
+
+
+class AddNativeFilterRequest(BaseModel):
+    """Request schema for adding a native filter to a dashboard."""
+
+    dashboard_id: int = Field(
+        ..., description="ID of the dashboard to add the filter to"
+    )
+    filter_name: str = Field(
+        ...,
+        description="Display name for the filter (e.g. 'Ngành hàng', 'Thị 
trường')",
+    )
+    filter_type: Literal[
+        "filter_select",
+        "filter_range",
+        "filter_time",
+        "filter_timecolumn",
+        "filter_timegrain",
+    ] = Field(
+        ...,
+        description=(
+            "Type of native filter to create. "
+            "filter_select: dropdown/multi-select for categorical columns. "
+            "filter_range: numeric range slider. "
+            "filter_time: date/time range picker. "
+            "filter_timecolumn: select which time column to use. "
+            "filter_timegrain: select time grain (day/week/month)."
+        ),
+    )
+    dataset_id: int = Field(
+        ..., description="ID of the dataset the filter targets"
+    )
+    column_name: str = Field(
+        ...,
+        description="Name of the column in the dataset to filter on",
+    )
+    default_value: Any = Field(
+        default=None,
+        description=(
+            "Default selected value(s) for the filter. "
+            "For filter_select with multiple_select=True, use a list. "
+            "For filter_range, use a dict like {'min': 0, 'max': 100}."
+        ),
+    )
+    parent_filter_ids: List[str] = Field(
+        default_factory=list,
+        description=(
+            "List of parent native filter IDs for cascading filters. "
+            "The parent filters must already exist on the dashboard."
+        ),
+    )
+    scope_type: Literal["full", "specific"] = Field(
+        default="full",
+        description=(
+            "Filter scope. 'full' applies the filter to all charts on the "
+            "dashboard (default). 'specific' applies only to charts NOT in "
+            "excluded_chart_ids."
+        ),
+    )
+    excluded_chart_ids: List[int] = Field(
+        default_factory=list,
+        description=(
+            "Chart IDs to exclude from this filter's scope. "
+            "Only used when scope_type='specific'."
+        ),
+    )
+    enable_search: bool = Field(
+        default=True,
+        description="Enable search in select filter dropdown (filter_select 
only)",
+    )
+    multiple_select: bool = Field(
+        default=True,
+        description="Allow selecting multiple values (filter_select only)",
+    )
+
+
+class AddNativeFilterResponse(BaseModel):
+    """Response schema for adding a native filter."""
+
+    filter_id: str | None = Field(
+        None, description="ID of the newly created native filter"
+    )
+    filter_name: str | None = Field(
+        None, description="Display name of the created filter"
+    )
+    filter_type: str | None = Field(
+        None, description="Type of the created filter"
+    )
+    native_filters: List[NativeFilterSummary] = Field(
+        default_factory=list,
+        description="Updated list of all native filters on the dashboard",
+    )
+    dashboard_id: int | None = Field(
+        None, description="ID of the updated dashboard"
+    )
+    dashboard_url: str | None = Field(
+        None, description="URL to view the updated dashboard"
+    )
+    error: str | None = Field(
+        None, description="Error message if operation failed"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Helper functions
+# ---------------------------------------------------------------------------
+
+
+def _generate_filter_id() -> str:
+    """Generate a unique native filter ID matching Superset's frontend 
format."""
+    return f"NATIVE_FILTER-{uuid.uuid4().hex[:20]}"
+
+
+def _build_scope(
+    scope_type: str,
+    excluded_chart_ids: List[int],
+) -> Dict[str, Any]:
+    """Build the scope object for a native filter.
+
+    Args:
+        scope_type: 'full' for all charts, 'specific' for excluding some.
+        excluded_chart_ids: Chart IDs to exclude when scope_type='specific'.
+
+    Returns:
+        Scope dict compatible with Superset's native filter format.
+    """
+    if scope_type == "specific" and excluded_chart_ids:
+        return {
+            "rootPath": ["ROOT_ID"],
+            "excluded": excluded_chart_ids,
+        }
+    # Default: apply to all charts
+    return {
+        "rootPath": ["ROOT_ID"],
+        "excluded": [0],
+    }
+
+
+def _build_control_values(
+    filter_type: str,
+    enable_search: bool,
+    multiple_select: bool,
+) -> Dict[str, Any]:
+    """Build controlValues for the filter based on its type."""
+    if filter_type == "filter_select":
+        return {
+            "enableEmptyFilter": False,
+            "defaultToFirstItem": False,
+            "multiSelect": multiple_select,
+            "searchAllOptions": enable_search,
+            "inverseSelection": False,
+        }
+    elif filter_type == "filter_range":
+        return {
+            "enableEmptyFilter": False,
+        }
+    elif filter_type in ("filter_time", "filter_timecolumn", 
"filter_timegrain"):
+        return {}
+    return {}
+
+
+def _build_default_data_mask(
+    filter_type: str,
+    default_value: Any,
+) -> Dict[str, Any]:
+    """Build defaultDataMask for the filter."""
+    if default_value is None:
+        return {
+            "filterState": {},
+            "extraFormData": {},
+        }
+
+    if filter_type == "filter_select":
+        # default_value should be a list or single value
+        values = default_value if isinstance(default_value, list) else 
[default_value]
+        return {
+            "filterState": {"value": values},
+            "extraFormData": {
+                "filters": [
+                    {
+                        "col": None,  # Will be set by Superset frontend
+                        "op": "IN",
+                        "val": values,
+                    }
+                ]
+            },
+        }
+    elif filter_type == "filter_range":
+        # default_value should be dict with min/max
+        filter_state = {}
+        if isinstance(default_value, dict):
+            filter_state = {"value": [default_value.get("min"), 
default_value.get("max")]}
+        return {
+            "filterState": filter_state,
+            "extraFormData": {},
+        }
+    elif filter_type == "filter_time":
+        return {
+            "filterState": {"value": default_value},
+            "extraFormData": {"time_range": default_value},
+        }
+    return {
+        "filterState": {},
+        "extraFormData": {},
+    }
+
+
+def _build_native_filter_config(
+    request: AddNativeFilterRequest,
+    filter_id: str,
+) -> Dict[str, Any]:
+    """Build the complete native filter configuration dict."""
+    return {
+        "id": filter_id,
+        "name": request.filter_name,
+        "filterType": request.filter_type,
+        "targets": [
+            {
+                "datasetId": request.dataset_id,
+                "column": {"name": request.column_name},
+            }
+        ],
+        "scope": _build_scope(request.scope_type, request.excluded_chart_ids),
+        "controlValues": _build_control_values(
+            request.filter_type, request.enable_search, request.multiple_select
+        ),
+        "defaultDataMask": _build_default_data_mask(
+            request.filter_type, request.default_value
+        ),
+        "cascadeParentIds": list(request.parent_filter_ids),
+        "type": "NATIVE_FILTER",
+        "description": "",
+        "chartsInScope": [],
+        "tabsInScope": [],
+    }
+
+
+def _extract_native_filter_summaries(
+    native_filter_configuration: List[Dict[str, Any]],
+) -> List[NativeFilterSummary]:
+    """Convert raw native filter configs to NativeFilterSummary objects."""
+    summaries = []
+    for f in native_filter_configuration:
+        if not isinstance(f, dict):
+            continue
+        raw_targets = f.get("targets", [])
+        if not isinstance(raw_targets, list):
+            raw_targets = []
+        targets = [t for t in raw_targets if isinstance(t, dict)]
+        summaries.append(
+            NativeFilterSummary(
+                id=f.get("id"),
+                name=f.get("name"),
+                filter_type=f.get("filterType"),
+                targets=targets,
+            )
+        )
+    return summaries
+
+
+# ---------------------------------------------------------------------------
+# MCP Tool
+# ---------------------------------------------------------------------------
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Dashboard",
+    annotations=ToolAnnotations(
+        title="Add native filter to dashboard",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def add_native_filter(
+    request: AddNativeFilterRequest, ctx: Context
+) -> AddNativeFilterResponse:
+    """Add a native filter to an existing dashboard.
+
+    Creates a new native filter (dropdown, range slider, time picker, etc.)
+    on the specified dashboard. The filter targets a specific column in a
+    dataset and can be scoped to all charts or specific charts.
+
+    Supported filter types:
+    - filter_select: Dropdown/multi-select for categorical columns
+    - filter_range: Numeric range slider
+    - filter_time: Date/time range picker
+    - filter_timecolumn: Select which time column to use
+    - filter_timegrain: Select time grain (day/week/month)
+
+    Example usage:
+    ```json
+    {
+        "dashboard_id": 2,
+        "filter_name": "Ngành hàng",
+        "filter_type": "filter_select",
+        "dataset_id": 38,
+        "column_name": "nganh_hang"
+    }
+    ```
+
+    With cascading filters:
+    ```json
+    {
+        "dashboard_id": 2,
+        "filter_name": "Thị trường",
+        "filter_type": "filter_select",
+        "dataset_id": 38,
+        "column_name": "market_type",
+        "parent_filter_ids": ["NATIVE_FILTER-abc123"]
+    }
+    ```
+    """
+    logger.info(
+        "Adding native filter to dashboard: dashboard_id=%s, filter_name=%s, "
+        "filter_type=%s, dataset_id=%s, column=%s",
+        request.dashboard_id,
+        request.filter_name,
+        request.filter_type,
+        request.dataset_id,
+        request.column_name,
+    )
+
+    try:
+        from superset import db, security_manager
+        from superset.commands.dashboard.update import UpdateDashboardCommand
+        from superset.daos.dashboard import DashboardDAO
+        from superset.exceptions import SupersetSecurityException
+
+        # --- Step 1: Validate dashboard exists and user has permission ---
+        with 
event_logger.log_context(action="mcp.add_native_filter.validation"):
+            dashboard = DashboardDAO.find_by_id(request.dashboard_id)
+            if not dashboard:
+                return AddNativeFilterResponse(
+                    error=f"Dashboard with ID {request.dashboard_id} not found"
+                )
+
+            try:
+                security_manager.raise_for_ownership(dashboard)
+            except SupersetSecurityException:
+                return AddNativeFilterResponse(
+                    error=(
+                        f"You don't have permission to edit dashboard "
+                        f"'{dashboard.dashboard_title}' (ID: 
{request.dashboard_id}). "
+                        "Only dashboard owners can add filters."
+                    )
+                )
+
+            # --- Step 2: Validate dataset exists ---
+            from superset.models.slice import Slice
+            from superset.connectors.sqla.models import SqlaTable
+
+            dataset = db.session.get(SqlaTable, request.dataset_id)
+            if not dataset:
+                return AddNativeFilterResponse(
+                    error=f"Dataset with ID {request.dataset_id} not found"
+                )
+
+        # --- Step 3: Generate filter ID and build config ---
+        with event_logger.log_context(action="mcp.add_native_filter.build"):
+            filter_id = _generate_filter_id()
+
+            logger.info("Generated filter ID: %s", filter_id)
+
+            filter_config = _build_native_filter_config(request, filter_id)
+
+            # Read existing json_metadata
+            existing_metadata = json.loads(dashboard.json_metadata or "{}")
+            native_filter_configuration = existing_metadata.get(
+                "native_filter_configuration", []
+            )
+
+            # Validate parent filter IDs exist
+            if request.parent_filter_ids:
+                existing_filter_ids = {
+                    f.get("id")
+                    for f in native_filter_configuration
+                    if isinstance(f, dict) and f.get("id")
+                }
+                missing_parents = [
+                    pid
+                    for pid in request.parent_filter_ids
+                    if pid not in existing_filter_ids
+                ]
+                if missing_parents:
+                    return AddNativeFilterResponse(
+                        error=(
+                            f"Parent filter IDs not found on dashboard: "
+                            f"{', '.join(missing_parents)}. "
+                            "Parent filters must already exist on the 
dashboard."
+                        )
+                    )
+
+            # Append new filter
+            native_filter_configuration.append(filter_config)
+            existing_metadata["native_filter_configuration"] = 
native_filter_configuration
+
+        # --- Step 4: Persist changes via UpdateDashboardCommand ---
+        with event_logger.log_context(action="mcp.add_native_filter.db_write"):
+            update_data = {
+                "json_metadata": json.dumps(existing_metadata),
+            }
+
+            command = UpdateDashboardCommand(request.dashboard_id, update_data)
+            updated_dashboard = command.run()

Review Comment:
   **Suggestion:** This is a read-modify-write update on `json_metadata` 
without any optimistic lock/version guard, so concurrent calls can overwrite 
each other and drop filters (last writer wins). Use a transactional/locking 
update strategy (or merge against latest persisted metadata) before persisting. 
[race condition]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Concurrent add-native-filter calls can drop previously added filters.
   - ⚠️ Dashboard json_metadata native_filter_configuration becomes 
inconsistent.
   - ⚠️ Users may see missing filters after concurrent edits.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Two MCP clients simultaneously call `add_native_filter` for the same 
dashboard ID
   through the tool entrypoint at
   `superset/mcp_service/dashboard/tool/add_native_filter.py:316-327`.
   
   2. In each request, after validation, Step 3 reads the dashboard metadata 
snapshot:
   `existing_metadata = json.loads(dashboard.json_metadata or "{}")` and
   `native_filter_configuration = 
existing_metadata.get("native_filter_configuration", [])`
   at `add_native_filter.py:418-421`, both starting from the same original 
`json_metadata`
   value.
   
   3. Each request appends its own `filter_config` to its in-memory
   `native_filter_configuration` list and reassigns it to
   `existing_metadata["native_filter_configuration"]` at 
`add_native_filter.py:445-446`,
   without re-reading or merging with any concurrent updates.
   
   4. Both requests then call `UpdateDashboardCommand(request.dashboard_id, 
update_data)` and
   `command.run()` at `add_native_filter.py:454-455`, which persists 
`json_metadata` via
   `DashboardDAO.update` and `DashboardDAO.set_dash_metadata` in
   `superset/commands/dashboard/update.py:73-79`; whichever transaction commits 
last
   overwrites the other's changes, so one filter is silently dropped in 
concurrent
   executions.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dd79f31b143c425184fe5f912ae884ea&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dd79f31b143c425184fe5f912ae884ea&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/dashboard/tool/add_native_filter.py
   **Line:** 418:455
   **Comment:**
        *Race Condition: This is a read-modify-write update on `json_metadata` 
without any optimistic lock/version guard, so concurrent calls can overwrite 
each other and drop filters (last writer wins). Use a transactional/locking 
update strategy (or merge against latest persisted metadata) before persisting.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=6e9db92f11aa344d9db6335beb3e88757f3c05743cbc76229d499999b140fb57&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=6e9db92f11aa344d9db6335beb3e88757f3c05743cbc76229d499999b140fb57&reaction=dislike'>👎</a>



##########
superset/mcp_service/dashboard/tool/add_native_filter.py:
##########
@@ -0,0 +1,501 @@
+# 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: add_native_filter
+
+This tool adds a native filter to an existing dashboard by updating
+the dashboard's json_metadata.native_filter_configuration.
+"""
+
+import logging
+import uuid
+from datetime import datetime, timezone
+from typing import Any, Dict, List, Literal
+
+from fastmcp import Context
+from pydantic import BaseModel, Field
+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 (
+    DashboardError,
+    DashboardInfo,
+    NativeFilterSummary,
+)
+from superset.mcp_service.utils.url_utils import get_superset_base_url
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Request / Response schemas
+# ---------------------------------------------------------------------------
+
+
+class AddNativeFilterRequest(BaseModel):
+    """Request schema for adding a native filter to a dashboard."""
+
+    dashboard_id: int = Field(
+        ..., description="ID of the dashboard to add the filter to"
+    )
+    filter_name: str = Field(
+        ...,
+        description="Display name for the filter (e.g. 'Ngành hàng', 'Thị 
trường')",
+    )
+    filter_type: Literal[
+        "filter_select",
+        "filter_range",
+        "filter_time",
+        "filter_timecolumn",
+        "filter_timegrain",
+    ] = Field(
+        ...,
+        description=(
+            "Type of native filter to create. "
+            "filter_select: dropdown/multi-select for categorical columns. "
+            "filter_range: numeric range slider. "
+            "filter_time: date/time range picker. "
+            "filter_timecolumn: select which time column to use. "
+            "filter_timegrain: select time grain (day/week/month)."
+        ),
+    )
+    dataset_id: int = Field(
+        ..., description="ID of the dataset the filter targets"
+    )
+    column_name: str = Field(
+        ...,
+        description="Name of the column in the dataset to filter on",
+    )
+    default_value: Any = Field(
+        default=None,
+        description=(
+            "Default selected value(s) for the filter. "
+            "For filter_select with multiple_select=True, use a list. "
+            "For filter_range, use a dict like {'min': 0, 'max': 100}."
+        ),
+    )
+    parent_filter_ids: List[str] = Field(
+        default_factory=list,
+        description=(
+            "List of parent native filter IDs for cascading filters. "
+            "The parent filters must already exist on the dashboard."
+        ),
+    )
+    scope_type: Literal["full", "specific"] = Field(
+        default="full",
+        description=(
+            "Filter scope. 'full' applies the filter to all charts on the "
+            "dashboard (default). 'specific' applies only to charts NOT in "
+            "excluded_chart_ids."
+        ),
+    )
+    excluded_chart_ids: List[int] = Field(
+        default_factory=list,
+        description=(
+            "Chart IDs to exclude from this filter's scope. "
+            "Only used when scope_type='specific'."
+        ),
+    )
+    enable_search: bool = Field(
+        default=True,
+        description="Enable search in select filter dropdown (filter_select 
only)",
+    )
+    multiple_select: bool = Field(
+        default=True,
+        description="Allow selecting multiple values (filter_select only)",
+    )
+
+
+class AddNativeFilterResponse(BaseModel):
+    """Response schema for adding a native filter."""
+
+    filter_id: str | None = Field(
+        None, description="ID of the newly created native filter"
+    )
+    filter_name: str | None = Field(
+        None, description="Display name of the created filter"
+    )
+    filter_type: str | None = Field(
+        None, description="Type of the created filter"
+    )
+    native_filters: List[NativeFilterSummary] = Field(
+        default_factory=list,
+        description="Updated list of all native filters on the dashboard",
+    )
+    dashboard_id: int | None = Field(
+        None, description="ID of the updated dashboard"
+    )
+    dashboard_url: str | None = Field(
+        None, description="URL to view the updated dashboard"
+    )
+    error: str | None = Field(
+        None, description="Error message if operation failed"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Helper functions
+# ---------------------------------------------------------------------------
+
+
+def _generate_filter_id() -> str:
+    """Generate a unique native filter ID matching Superset's frontend 
format."""
+    return f"NATIVE_FILTER-{uuid.uuid4().hex[:20]}"
+
+
+def _build_scope(
+    scope_type: str,
+    excluded_chart_ids: List[int],
+) -> Dict[str, Any]:
+    """Build the scope object for a native filter.
+
+    Args:
+        scope_type: 'full' for all charts, 'specific' for excluding some.
+        excluded_chart_ids: Chart IDs to exclude when scope_type='specific'.
+
+    Returns:
+        Scope dict compatible with Superset's native filter format.
+    """
+    if scope_type == "specific" and excluded_chart_ids:
+        return {
+            "rootPath": ["ROOT_ID"],
+            "excluded": excluded_chart_ids,
+        }
+    # Default: apply to all charts
+    return {
+        "rootPath": ["ROOT_ID"],
+        "excluded": [0],
+    }
+
+
+def _build_control_values(
+    filter_type: str,
+    enable_search: bool,
+    multiple_select: bool,
+) -> Dict[str, Any]:
+    """Build controlValues for the filter based on its type."""
+    if filter_type == "filter_select":
+        return {
+            "enableEmptyFilter": False,
+            "defaultToFirstItem": False,
+            "multiSelect": multiple_select,
+            "searchAllOptions": enable_search,
+            "inverseSelection": False,
+        }
+    elif filter_type == "filter_range":
+        return {
+            "enableEmptyFilter": False,
+        }
+    elif filter_type in ("filter_time", "filter_timecolumn", 
"filter_timegrain"):
+        return {}
+    return {}
+
+
+def _build_default_data_mask(
+    filter_type: str,
+    default_value: Any,
+) -> Dict[str, Any]:
+    """Build defaultDataMask for the filter."""
+    if default_value is None:
+        return {
+            "filterState": {},
+            "extraFormData": {},
+        }
+
+    if filter_type == "filter_select":
+        # default_value should be a list or single value
+        values = default_value if isinstance(default_value, list) else 
[default_value]
+        return {
+            "filterState": {"value": values},
+            "extraFormData": {
+                "filters": [
+                    {
+                        "col": None,  # Will be set by Superset frontend
+                        "op": "IN",
+                        "val": values,
+                    }
+                ]
+            },
+        }
+    elif filter_type == "filter_range":
+        # default_value should be dict with min/max
+        filter_state = {}
+        if isinstance(default_value, dict):
+            filter_state = {"value": [default_value.get("min"), 
default_value.get("max")]}
+        return {
+            "filterState": filter_state,
+            "extraFormData": {},
+        }
+    elif filter_type == "filter_time":
+        return {
+            "filterState": {"value": default_value},
+            "extraFormData": {"time_range": default_value},
+        }
+    return {
+        "filterState": {},
+        "extraFormData": {},
+    }
+
+
+def _build_native_filter_config(
+    request: AddNativeFilterRequest,
+    filter_id: str,
+) -> Dict[str, Any]:
+    """Build the complete native filter configuration dict."""
+    return {
+        "id": filter_id,
+        "name": request.filter_name,
+        "filterType": request.filter_type,
+        "targets": [
+            {
+                "datasetId": request.dataset_id,
+                "column": {"name": request.column_name},
+            }
+        ],
+        "scope": _build_scope(request.scope_type, request.excluded_chart_ids),
+        "controlValues": _build_control_values(
+            request.filter_type, request.enable_search, request.multiple_select
+        ),
+        "defaultDataMask": _build_default_data_mask(
+            request.filter_type, request.default_value
+        ),
+        "cascadeParentIds": list(request.parent_filter_ids),
+        "type": "NATIVE_FILTER",
+        "description": "",
+        "chartsInScope": [],
+        "tabsInScope": [],
+    }
+
+
+def _extract_native_filter_summaries(
+    native_filter_configuration: List[Dict[str, Any]],
+) -> List[NativeFilterSummary]:
+    """Convert raw native filter configs to NativeFilterSummary objects."""
+    summaries = []
+    for f in native_filter_configuration:
+        if not isinstance(f, dict):
+            continue
+        raw_targets = f.get("targets", [])
+        if not isinstance(raw_targets, list):
+            raw_targets = []
+        targets = [t for t in raw_targets if isinstance(t, dict)]
+        summaries.append(
+            NativeFilterSummary(
+                id=f.get("id"),
+                name=f.get("name"),
+                filter_type=f.get("filterType"),
+                targets=targets,
+            )
+        )
+    return summaries
+
+
+# ---------------------------------------------------------------------------
+# MCP Tool
+# ---------------------------------------------------------------------------
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Dashboard",
+    annotations=ToolAnnotations(
+        title="Add native filter to dashboard",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def add_native_filter(
+    request: AddNativeFilterRequest, ctx: Context
+) -> AddNativeFilterResponse:
+    """Add a native filter to an existing dashboard.
+
+    Creates a new native filter (dropdown, range slider, time picker, etc.)
+    on the specified dashboard. The filter targets a specific column in a
+    dataset and can be scoped to all charts or specific charts.
+
+    Supported filter types:
+    - filter_select: Dropdown/multi-select for categorical columns
+    - filter_range: Numeric range slider
+    - filter_time: Date/time range picker
+    - filter_timecolumn: Select which time column to use
+    - filter_timegrain: Select time grain (day/week/month)
+
+    Example usage:
+    ```json
+    {
+        "dashboard_id": 2,
+        "filter_name": "Ngành hàng",
+        "filter_type": "filter_select",
+        "dataset_id": 38,
+        "column_name": "nganh_hang"
+    }
+    ```
+
+    With cascading filters:
+    ```json
+    {
+        "dashboard_id": 2,
+        "filter_name": "Thị trường",
+        "filter_type": "filter_select",
+        "dataset_id": 38,
+        "column_name": "market_type",
+        "parent_filter_ids": ["NATIVE_FILTER-abc123"]
+    }
+    ```
+    """
+    logger.info(
+        "Adding native filter to dashboard: dashboard_id=%s, filter_name=%s, "
+        "filter_type=%s, dataset_id=%s, column=%s",
+        request.dashboard_id,
+        request.filter_name,
+        request.filter_type,
+        request.dataset_id,
+        request.column_name,
+    )
+
+    try:
+        from superset import db, security_manager
+        from superset.commands.dashboard.update import UpdateDashboardCommand
+        from superset.daos.dashboard import DashboardDAO
+        from superset.exceptions import SupersetSecurityException
+
+        # --- Step 1: Validate dashboard exists and user has permission ---
+        with 
event_logger.log_context(action="mcp.add_native_filter.validation"):
+            dashboard = DashboardDAO.find_by_id(request.dashboard_id)
+            if not dashboard:
+                return AddNativeFilterResponse(
+                    error=f"Dashboard with ID {request.dashboard_id} not found"
+                )
+
+            try:
+                security_manager.raise_for_ownership(dashboard)
+            except SupersetSecurityException:
+                return AddNativeFilterResponse(
+                    error=(
+                        f"You don't have permission to edit dashboard "
+                        f"'{dashboard.dashboard_title}' (ID: 
{request.dashboard_id}). "
+                        "Only dashboard owners can add filters."
+                    )
+                )
+
+            # --- Step 2: Validate dataset exists ---
+            from superset.models.slice import Slice
+            from superset.connectors.sqla.models import SqlaTable
+
+            dataset = db.session.get(SqlaTable, request.dataset_id)
+            if not dataset:
+                return AddNativeFilterResponse(
+                    error=f"Dataset with ID {request.dataset_id} not found"
+                )

Review Comment:
   **Suggestion:** The tool only checks that the dataset exists, but never 
enforces dataset-level access for the current user. A dashboard owner without 
permission on the target dataset can still create a filter referencing it, 
which bypasses the data-level authorization pattern used by other MCP tools. 
Add a dataset access check (using the existing MCP auth utilities) before 
writing metadata. [security]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Add-native-filter MCP tool skips dataset-level access checks.
   - ⚠️ Dashboard owners can target datasets they cannot query.
   - ⚠️ Undermines mcp_service data-level RBAC consistency.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. A client invokes the MCP tool `add_native_filter` (entrypoint at
   `superset/mcp_service/dashboard/tool/add_native_filter.py:316-327`) to add a 
filter to an
   existing dashboard.
   
   2. Inside the tool, Step 1 validation loads the dashboard and enforces only 
dashboard
   ownership via `security_manager.raise_for_ownership(dashboard)` at
   `add_native_filter.py:388-390`, then proceeds to dataset validation at
   `add_native_filter.py:399-407`.
   
   3. The dataset check at `add_native_filter.py:403-407` only verifies 
existence via
   `db.session.get(SqlaTable, request.dataset_id)` and returns an error if it 
is `None`, but
   performs no call to the MCP data-level access helpers such as 
`has_dataset_access` in
   `superset/mcp_service/auth.py:13-43` or `check_chart_data_access` in
   `superset/mcp_service/auth.py:46-61`.
   
   4. For a user who owns the dashboard but lacks permissions on `SqlaTable` 
with
   `id=request.dataset_id`, the code path still builds a filter targeting that 
dataset (see
   `add_native_filter.py:258-272`) and persists it via 
`UpdateDashboardCommand.run()` at
   `superset/commands/dashboard/update.py:58-79`, allowing creation of filters 
against
   datasets the caller cannot access.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=380e78cae2e44773a87a2edd7cc26473&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=380e78cae2e44773a87a2edd7cc26473&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/dashboard/tool/add_native_filter.py
   **Line:** 403:407
   **Comment:**
        *Security: The tool only checks that the dataset exists, but never 
enforces dataset-level access for the current user. A dashboard owner without 
permission on the target dataset can still create a filter referencing it, 
which bypasses the data-level authorization pattern used by other MCP tools. 
Add a dataset access check (using the existing MCP auth utilities) before 
writing metadata.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=291ad636d37fe50a4e2c41a6ad0ddd181b7d750d0ac23cf42a6aedf5d3a93ff9&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=291ad636d37fe50a4e2c41a6ad0ddd181b7d750d0ac23cf42a6aedf5d3a93ff9&reaction=dislike'>👎</a>



##########
superset/mcp_service/dashboard/tool/add_native_filter.py:
##########
@@ -0,0 +1,501 @@
+# 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: add_native_filter
+
+This tool adds a native filter to an existing dashboard by updating
+the dashboard's json_metadata.native_filter_configuration.
+"""
+
+import logging
+import uuid
+from datetime import datetime, timezone
+from typing import Any, Dict, List, Literal
+
+from fastmcp import Context
+from pydantic import BaseModel, Field
+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 (
+    DashboardError,
+    DashboardInfo,
+    NativeFilterSummary,
+)
+from superset.mcp_service.utils.url_utils import get_superset_base_url
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+
+# ---------------------------------------------------------------------------
+# Request / Response schemas
+# ---------------------------------------------------------------------------
+
+
+class AddNativeFilterRequest(BaseModel):
+    """Request schema for adding a native filter to a dashboard."""
+
+    dashboard_id: int = Field(
+        ..., description="ID of the dashboard to add the filter to"
+    )
+    filter_name: str = Field(
+        ...,
+        description="Display name for the filter (e.g. 'Ngành hàng', 'Thị 
trường')",
+    )
+    filter_type: Literal[
+        "filter_select",
+        "filter_range",
+        "filter_time",
+        "filter_timecolumn",
+        "filter_timegrain",
+    ] = Field(
+        ...,
+        description=(
+            "Type of native filter to create. "
+            "filter_select: dropdown/multi-select for categorical columns. "
+            "filter_range: numeric range slider. "
+            "filter_time: date/time range picker. "
+            "filter_timecolumn: select which time column to use. "
+            "filter_timegrain: select time grain (day/week/month)."
+        ),
+    )
+    dataset_id: int = Field(
+        ..., description="ID of the dataset the filter targets"
+    )
+    column_name: str = Field(
+        ...,
+        description="Name of the column in the dataset to filter on",
+    )
+    default_value: Any = Field(
+        default=None,
+        description=(
+            "Default selected value(s) for the filter. "
+            "For filter_select with multiple_select=True, use a list. "
+            "For filter_range, use a dict like {'min': 0, 'max': 100}."
+        ),
+    )
+    parent_filter_ids: List[str] = Field(
+        default_factory=list,
+        description=(
+            "List of parent native filter IDs for cascading filters. "
+            "The parent filters must already exist on the dashboard."
+        ),
+    )
+    scope_type: Literal["full", "specific"] = Field(
+        default="full",
+        description=(
+            "Filter scope. 'full' applies the filter to all charts on the "
+            "dashboard (default). 'specific' applies only to charts NOT in "
+            "excluded_chart_ids."
+        ),
+    )
+    excluded_chart_ids: List[int] = Field(
+        default_factory=list,
+        description=(
+            "Chart IDs to exclude from this filter's scope. "
+            "Only used when scope_type='specific'."
+        ),
+    )
+    enable_search: bool = Field(
+        default=True,
+        description="Enable search in select filter dropdown (filter_select 
only)",
+    )
+    multiple_select: bool = Field(
+        default=True,
+        description="Allow selecting multiple values (filter_select only)",
+    )
+
+
+class AddNativeFilterResponse(BaseModel):
+    """Response schema for adding a native filter."""
+
+    filter_id: str | None = Field(
+        None, description="ID of the newly created native filter"
+    )
+    filter_name: str | None = Field(
+        None, description="Display name of the created filter"
+    )
+    filter_type: str | None = Field(
+        None, description="Type of the created filter"
+    )
+    native_filters: List[NativeFilterSummary] = Field(
+        default_factory=list,
+        description="Updated list of all native filters on the dashboard",
+    )
+    dashboard_id: int | None = Field(
+        None, description="ID of the updated dashboard"
+    )
+    dashboard_url: str | None = Field(
+        None, description="URL to view the updated dashboard"
+    )
+    error: str | None = Field(
+        None, description="Error message if operation failed"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Helper functions
+# ---------------------------------------------------------------------------
+
+
+def _generate_filter_id() -> str:
+    """Generate a unique native filter ID matching Superset's frontend 
format."""
+    return f"NATIVE_FILTER-{uuid.uuid4().hex[:20]}"
+
+
+def _build_scope(
+    scope_type: str,
+    excluded_chart_ids: List[int],
+) -> Dict[str, Any]:
+    """Build the scope object for a native filter.
+
+    Args:
+        scope_type: 'full' for all charts, 'specific' for excluding some.
+        excluded_chart_ids: Chart IDs to exclude when scope_type='specific'.
+
+    Returns:
+        Scope dict compatible with Superset's native filter format.
+    """
+    if scope_type == "specific" and excluded_chart_ids:
+        return {
+            "rootPath": ["ROOT_ID"],
+            "excluded": excluded_chart_ids,
+        }
+    # Default: apply to all charts
+    return {
+        "rootPath": ["ROOT_ID"],
+        "excluded": [0],
+    }

Review Comment:
   **Suggestion:** The default "full scope" branch writes `excluded: [0]` 
instead of an empty exclusion list, which makes the filter look like it has 
exclusions in frontend scope logic (`excluded.length !== 0`) and can produce 
incorrect scope interpretation/UI behavior. Use an empty list for full scope. 
[logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Full-scope native filters stored with synthetic exclusion id.
   - ⚠️ Inconsistent scope encoding versus generate_dashboard tool.
   - ⚠️ Importers misinterpret non-empty excluded list semantics.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. A client calls `add_native_filter` without specifying `scope_type` or
   `excluded_chart_ids`, so `AddNativeFilterRequest` defaults 
`scope_type="full"` and
   `excluded_chart_ids=[]` as defined at `add_native_filter.py:102-116`.
   
   2. `_build_native_filter_config` at `add_native_filter.py:258-285` calls
   `_build_scope(request.scope_type, request.excluded_chart_ids)` at
   `add_native_filter.py:273`, passing `"full"` and an empty list.
   
   3. `_build_scope` executes the default branch at 
`add_native_filter.py:182-186`, returning
   `{"rootPath": ["ROOT_ID"], "excluded": [0]}` for full scope instead of an 
empty exclusion
   list.
   
   4. This representation differs from other Superset MCP and core code that 
model "no
   exclusions" as an empty list, e.g. `generate_dashboard` writes `"excluded": 
[]` in
   `global_chart_configuration.scope` at
   `superset/mcp_service/dashboard/tool/generate_dashboard.py:270-20`, and 
import utilities
   treat a non-empty `scope_excluded` list as meaningful (see `scope_excluded =
   native_filter.get("scope", {}).get("excluded", [])` and `if scope_excluded:` 
at
   `superset/commands/dashboard/importers/v1/utils.py:130-13`), so using `[0]` 
for full scope
   can cause downstream logic to treat the filter as having exclusions.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=85e85ed2cd4e4a18906f76d3362e58c2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=85e85ed2cd4e4a18906f76d3362e58c2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/dashboard/tool/add_native_filter.py
   **Line:** 183:186
   **Comment:**
        *Logic Error: The default "full scope" branch writes `excluded: [0]` 
instead of an empty exclusion list, which makes the filter look like it has 
exclusions in frontend scope logic (`excluded.length !== 0`) and can produce 
incorrect scope interpretation/UI behavior. Use an empty list for full scope.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=de1aff35e0b7ff49f84bbe9ab3752eb2180d5091926cddb92b9ebc13f4473a67&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40995&comment_hash=de1aff35e0b7ff49f84bbe9ab3752eb2180d5091926cddb92b9ebc13f4473a67&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to