Copilot commented on code in PR #41611:
URL: https://github.com/apache/superset/pull/41611#discussion_r3507672542


##########
superset/mcp_service/semantic_layer/tool/list_metrics.py:
##########
@@ -0,0 +1,267 @@
+# 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: list_metrics
+
+Unified metric discovery across built-in datasets and external semantic views.
+"""
+
+import logging
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    DimensionInfo,
+    ListMetricsRequest,
+    MetricInfo,
+    MetricList,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _matches_search(text: str | None, search: str) -> bool:
+    if not text:
+        return False
+    return search.lower() in text.lower()
+
+
+def _builtin_compatible_dims(dataset: Any) -> list[DimensionInfo]:
+    """Return groupby-enabled columns as compatible dimensions for a built-in 
metric."""
+    return [
+        DimensionInfo(
+            name=col.column_name,
+            verbose_name=col.verbose_name or None,
+            description=col.description or None,
+            type=col.type or None,
+            is_dttm=bool(col.is_dttm),
+            groupby=bool(col.groupby),
+            filterable=bool(col.filterable),
+            source="builtin",
+        )
+        for col in dataset.columns
+        if col.groupby
+    ]
+
+
+def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
+    """Collect metrics from built-in SqlaTable datasets."""
+    from sqlalchemy.orm import subqueryload
+
+    from superset.connectors.sqla.models import SqlaTable
+    from superset.daos.dataset import DatasetDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
+        if request.dataset_id is not None:
+            dataset = DatasetDAO.find_by_id(
+                request.dataset_id,
+                query_options=[
+                    subqueryload(SqlaTable.columns),
+                    subqueryload(SqlaTable.metrics),
+                ],
+            )
+            datasets = [dataset] if dataset else []
+        else:
+            datasets = DatasetDAO.find_all()
+
+    results: list[MetricInfo] = []
+    for dataset in datasets:
+        compat_dims = (
+            _builtin_compatible_dims(dataset)
+            if request.include_compatible_dimensions
+            else []
+        )
+        for metric in dataset.metrics:
+            name = metric.metric_name or ""
+            desc = metric.description or ""
+            if request.search and not (
+                _matches_search(name, request.search)
+                or _matches_search(desc, request.search)
+            ):
+                continue
+            results.append(
+                MetricInfo(
+                    name=name,
+                    verbose_name=metric.verbose_name or None,
+                    description=desc or None,
+                    expression=metric.expression or None,
+                    d3format=metric.d3format or None,
+                    warning_text=metric.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                    compatible_dimensions=compat_dims,
+                )
+            )
+    return results
+
+
+async def _collect_external_metrics(
+    request: ListMetricsRequest,
+    ctx: Context,
+) -> list[MetricInfo]:
+    """Collect metrics from external SemanticView models."""
+    from superset.daos.semantic_layer import SemanticViewDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.external_query"):
+        if request.view_id is not None:
+            view = SemanticViewDAO.find_by_id(request.view_id)
+            views = [view] if view else []
+        else:
+            views = SemanticViewDAO.find_all()
+
+    await ctx.debug("Found %d semantic views to scan for metrics" % len(views))
+
+    results: list[MetricInfo] = []
+    for view in views:
+        try:
+            raw_metrics = view.metrics
+            raw_cols = view.columns if request.include_compatible_dimensions 
else []
+            compat_dims = [
+                DimensionInfo(
+                    name=col.column_name,
+                    verbose_name=col.verbose_name,
+                    description=col.description,
+                    type=col.type,
+                    is_dttm=col.is_dttm,
+                    groupby=col.groupby,
+                    filterable=col.filterable,
+                    source="external",
+                )
+                for col in raw_cols
+            ]
+            for metric in raw_metrics:
+                name = metric.metric_name or ""
+                desc = metric.description or ""
+                if request.search and not (
+                    _matches_search(name, request.search)
+                    or _matches_search(desc, request.search)
+                ):
+                    continue
+                results.append(
+                    MetricInfo(
+                        name=name,
+                        description=desc or None,
+                        expression=metric.expression or None,
+                        source="external",
+                        view_id=view.id,
+                        view_name=view.name,
+                        compatible_dimensions=compat_dims,
+                    )
+                )
+        except Exception as exc:  # noqa: BLE001
+            # External registry may be empty in OSS — degrade gracefully
+            await ctx.warning(
+                "Could not load metrics for view id=%s: %s" % (view.id, 
str(exc))
+            )
+    return results
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="List metrics",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def list_metrics(
+    request: ListMetricsRequest | None = None,
+    ctx: Context | None = None,
+) -> MetricList | SemanticLayerError:
+    """List available metrics across built-in datasets and external semantic 
views.
+
+    This is the primary entry point for semantic layer exploration. Returns a
+    unified list of metrics from all data sources the current user can access,
+    with compatible dimensions included inline.
+
+    Workflow:
+    1. list_metrics -> discover available metrics and their compatible 
dimensions
+    2. get_table -> query data using chosen metrics and dimensions
+
+    Use ``search`` to filter by metric name or description. Use ``dataset_id``
+    or ``view_id`` to scope to a specific data source.
+
+    Example:
+    ```json
+    {"search": "revenue", "include_compatible_dimensions": true, "page": 1}
+    ```
+    """
+    if ctx is None:
+        raise RuntimeError("FastMCP context is required for list_metrics")
+
+    request = request or ListMetricsRequest()
+
+    await ctx.info(
+        "Listing metrics: search=%s, dataset_id=%s, view_id=%s, page=%s"
+        % (request.search, request.dataset_id, request.view_id, request.page)
+    )

Review Comment:
   list_metrics currently allows passing both dataset_id and view_id; the tool 
then returns an empty list instead of a clear validation error. This is 
inconsistent with get_table/get_compatible_* and makes client behavior 
ambiguous.



##########
superset/mcp_service/semantic_layer/tool/list_metrics.py:
##########
@@ -0,0 +1,267 @@
+# 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: list_metrics
+
+Unified metric discovery across built-in datasets and external semantic views.
+"""
+
+import logging
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    DimensionInfo,
+    ListMetricsRequest,
+    MetricInfo,
+    MetricList,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _matches_search(text: str | None, search: str) -> bool:
+    if not text:
+        return False
+    return search.lower() in text.lower()
+
+
+def _builtin_compatible_dims(dataset: Any) -> list[DimensionInfo]:
+    """Return groupby-enabled columns as compatible dimensions for a built-in 
metric."""
+    return [
+        DimensionInfo(
+            name=col.column_name,
+            verbose_name=col.verbose_name or None,
+            description=col.description or None,
+            type=col.type or None,
+            is_dttm=bool(col.is_dttm),
+            groupby=bool(col.groupby),
+            filterable=bool(col.filterable),
+            source="builtin",
+        )
+        for col in dataset.columns
+        if col.groupby
+    ]
+
+
+def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
+    """Collect metrics from built-in SqlaTable datasets."""
+    from sqlalchemy.orm import subqueryload
+
+    from superset.connectors.sqla.models import SqlaTable
+    from superset.daos.dataset import DatasetDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
+        if request.dataset_id is not None:
+            dataset = DatasetDAO.find_by_id(
+                request.dataset_id,
+                query_options=[
+                    subqueryload(SqlaTable.columns),
+                    subqueryload(SqlaTable.metrics),
+                ],
+            )
+            datasets = [dataset] if dataset else []
+        else:
+            datasets = DatasetDAO.find_all()
+
+    results: list[MetricInfo] = []
+    for dataset in datasets:
+        compat_dims = (
+            _builtin_compatible_dims(dataset)
+            if request.include_compatible_dimensions
+            else []
+        )
+        for metric in dataset.metrics:
+            name = metric.metric_name or ""
+            desc = metric.description or ""
+            if request.search and not (
+                _matches_search(name, request.search)
+                or _matches_search(desc, request.search)
+            ):
+                continue
+            results.append(
+                MetricInfo(
+                    name=name,
+                    verbose_name=metric.verbose_name or None,
+                    description=desc or None,
+                    expression=metric.expression or None,
+                    d3format=metric.d3format or None,
+                    warning_text=metric.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                    compatible_dimensions=compat_dims,
+                )
+            )
+    return results
+
+
+async def _collect_external_metrics(
+    request: ListMetricsRequest,
+    ctx: Context,
+) -> list[MetricInfo]:
+    """Collect metrics from external SemanticView models."""
+    from superset.daos.semantic_layer import SemanticViewDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.external_query"):
+        if request.view_id is not None:
+            view = SemanticViewDAO.find_by_id(request.view_id)
+            views = [view] if view else []
+        else:
+            views = SemanticViewDAO.find_all()
+
+    await ctx.debug("Found %d semantic views to scan for metrics" % len(views))
+
+    results: list[MetricInfo] = []
+    for view in views:
+        try:
+            raw_metrics = view.metrics
+            raw_cols = view.columns if request.include_compatible_dimensions 
else []

Review Comment:
   External semantic views are scanned without an access check. Since 
SemanticViewDAO.find_all/find_by_id do not apply datasource_access filtering, 
this can expose metric/column metadata for semantic views the user cannot 
access. Call view.raise_for_access() before reading view.metrics/view.columns.



##########
superset/mcp_service/semantic_layer/tool/list_metrics.py:
##########
@@ -0,0 +1,267 @@
+# 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: list_metrics
+
+Unified metric discovery across built-in datasets and external semantic views.
+"""
+
+import logging
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    DimensionInfo,
+    ListMetricsRequest,
+    MetricInfo,
+    MetricList,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _matches_search(text: str | None, search: str) -> bool:
+    if not text:
+        return False
+    return search.lower() in text.lower()
+
+
+def _builtin_compatible_dims(dataset: Any) -> list[DimensionInfo]:
+    """Return groupby-enabled columns as compatible dimensions for a built-in 
metric."""
+    return [
+        DimensionInfo(
+            name=col.column_name,
+            verbose_name=col.verbose_name or None,
+            description=col.description or None,
+            type=col.type or None,
+            is_dttm=bool(col.is_dttm),
+            groupby=bool(col.groupby),
+            filterable=bool(col.filterable),
+            source="builtin",
+        )
+        for col in dataset.columns
+        if col.groupby
+    ]
+
+
+def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
+    """Collect metrics from built-in SqlaTable datasets."""
+    from sqlalchemy.orm import subqueryload
+
+    from superset.connectors.sqla.models import SqlaTable
+    from superset.daos.dataset import DatasetDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
+        if request.dataset_id is not None:
+            dataset = DatasetDAO.find_by_id(
+                request.dataset_id,
+                query_options=[
+                    subqueryload(SqlaTable.columns),
+                    subqueryload(SqlaTable.metrics),
+                ],
+            )
+            datasets = [dataset] if dataset else []
+        else:
+            datasets = DatasetDAO.find_all()

Review Comment:
   When dataset_id is not provided, _collect_builtin_metrics uses 
DatasetDAO.find_all() without eager-loading columns/metrics. Iterating 
dataset.columns and dataset.metrics will then issue N+1 queries and defeats 
pagination by loading everything up front.



##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -0,0 +1,397 @@
+# 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: get_table
+
+Query a data source (built-in dataset or external semantic view) using
+metric and dimension names, returning tabular results.
+"""
+
+import logging
+import time
+from typing import Any
+
+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.exceptions import OAuth2Error, OAuth2RedirectError, 
SupersetException
+from superset.extensions import event_logger
+from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    GetTableRequest,
+    GetTableResponse,
+    SemanticLayerError,
+)
+from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
+from superset.mcp_service.utils.oauth2_utils import 
build_oauth2_redirect_message
+
+logger = logging.getLogger(__name__)
+
+
+def _build_query_dict(
+    request: GetTableRequest,
+    time_col: str | None,
+) -> dict[str, Any]:
+    """Assemble the query dict for QueryContextFactory."""
+    filters: list[dict[str, Any]] = [
+        {"col": f.col, "op": f.op, "val": f.val} for f in request.filters
+    ]
+    if request.time_range and time_col:
+        filters.append(
+            {"col": time_col, "op": "TEMPORAL_RANGE", "val": 
request.time_range}
+        )
+
+    query_dict: dict[str, Any] = {
+        "filters": filters,
+        "columns": request.dimensions,
+        "metrics": request.metrics,
+        "row_limit": request.row_limit,
+        "order_desc": request.order_desc,
+    }
+    if time_col:
+        query_dict["granularity"] = time_col
+    if request.order_by:
+        query_dict["orderby"] = [
+            (col, not request.order_desc) for col in request.order_by
+        ]
+    return query_dict
+
+
+def _format_columns(
+    data: list[dict[str, Any]], raw_columns: list[str]
+) -> list[DataColumn]:
+    stats_rows = data[:5000]
+    columns_meta: list[DataColumn] = []
+    for col_name in raw_columns:
+        sample_values = [
+            row.get(col_name) for row in data[:3] if row.get(col_name) is not 
None
+        ]
+        data_type = "string"
+        if sample_values:
+            if all(isinstance(v, bool) for v in sample_values):
+                data_type = "boolean"
+            elif all(isinstance(v, (int, float)) for v in sample_values):
+                data_type = "numeric"
+
+        null_count = 0
+        unique_vals: set[str] = set()
+        for row in stats_rows:
+            val = row.get(col_name)
+            if val is None:
+                null_count += 1
+            else:
+                unique_vals.add(str(val))
+
+        columns_meta.append(
+            DataColumn(
+                name=col_name,
+                display_name=col_name.replace("_", " ").title(),
+                data_type=data_type,
+                sample_values=sample_values[:3],
+                null_count=null_count,
+                unique_count=len(unique_vals),
+            )
+        )
+    return columns_meta
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get table",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_table(  # noqa: C901
+    request: GetTableRequest,
+    ctx: Context,
+) -> GetTableResponse | SemanticLayerError:
+    """Query a data source using metrics and dimensions, returning tabular 
results.
+
+    Works with both built-in datasets and external semantic views. The
+    ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response.
+
+    Workflow:
+    1. list_metrics -> discover metrics and their compatible_dimensions
+    2. get_table -> query with chosen metrics and dimensions
+
+    Example (built-in):
+    ```json
+    {
+        "dataset_id": 42,
+        "metrics": ["revenue"],
+        "dimensions": ["region", "product_category"],
+        "time_range": "Last 30 days",
+        "row_limit": 500
+    }
+    ```
+
+    Example (external):
+    ```json
+    {
+        "view_id": 5,
+        "metrics": ["bookings"],
+        "dimensions": ["listing__country_name"],
+        "row_limit": 100
+    }
+    ```
+    """
+    await ctx.info(
+        "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, "
+        "dimensions=%s, row_limit=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.metrics,
+            request.dimensions,
+            request.row_limit,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error=(
+                "Provide either dataset_id (built-in dataset) or view_id "
+                "(external semantic view). Both are in the list_metrics 
response."
+            ),
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        from superset.commands.chart.data.get_data_command import 
ChartDataCommand
+        from superset.common.query_context_factory import QueryContextFactory
+
+        is_builtin = request.dataset_id is not None
+        datasource_type = "table" if is_builtin else "semantic_view"
+        if is_builtin:
+            assert request.dataset_id is not None
+            datasource_id = request.dataset_id
+        else:
+            assert request.view_id is not None
+            datasource_id = request.view_id
+
+        # ------------------------------------------------------------------
+        # Resolve datasource for metadata (time column, display name)
+        # ------------------------------------------------------------------
+        await ctx.report_progress(1, 4, "Resolving data source")
+        display_name: str = f"{'Dataset' if is_builtin else 'View'} 
{datasource_id}"
+        time_col: str | None = request.time_column
+        warnings: list[str] = []
+
+        if is_builtin:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            with 
event_logger.log_context(action="mcp.get_table.resolve_dataset"):
+                dataset = DatasetDAO.find_by_id(
+                    datasource_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+            display_name = dataset.table_name
+            if time_col is None and request.time_range:
+                time_col = getattr(dataset, "main_dttm_col", None)
+                if not time_col:
+                    return SemanticLayerError.create(
+                        error=(
+                            "time_range was provided but no temporal column is 
"
+                            "configured. Set time_column explicitly."
+                        ),
+                        error_type="ValidationError",
+                    )
+        else:
+            from superset.daos.semantic_layer import SemanticViewDAO
+
+            with event_logger.log_context(action="mcp.get_table.resolve_view"):
+                view = SemanticViewDAO.find_by_id(datasource_id)
+            if view is None:
+                return SemanticLayerError.create(
+                    error=f"No semantic view found with id: 
{request.view_id}.",
+                    error_type="NotFound",
+                )
+            display_name = view.name

Review Comment:
   get_table loads the SemanticView via SemanticViewDAO.find_by_id and 
immediately reads view.name/view.columns without enforcing datasource_access. 
This can leak semantic view existence and metadata to users who lack access; 
perform an explicit access check and treat access-denied consistently (often as 
NotFound to avoid existence disclosure).



##########
superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py:
##########
@@ -0,0 +1,214 @@
+# 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: get_compatible_dimensions
+
+Returns dimensions compatible with the current metric/dimension selection.
+"""
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    CompatibleDimensionsResponse,
+    DimensionInfo,
+    GetCompatibleDimensionsRequest,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get compatible dimensions",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_compatible_dimensions(
+    request: GetCompatibleDimensionsRequest,
+    ctx: Context,
+) -> CompatibleDimensionsResponse | SemanticLayerError:
+    """Return dimensions compatible with the current metric/dimension 
selection.
+
+    Used to drive progressive disclosure in query builders: after the user
+    selects one or more metrics (and optionally some dimensions), this tool
+    returns the dimensions that can validly be added without breaking the
+    underlying query.
+
+    Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external).
+
+    For built-in datasets, returns all groupby-enabled columns from the same
+    dataset as the selected metrics, filtered to those whose datasets intersect
+    with all selected metrics.
+
+    For external semantic views, delegates to the view's
+    ``get_compatible_dimensions`` implementation.
+
+    Example:
+    ```json
+    {
+        "selected_metrics": ["revenue"],
+        "selected_dimensions": [],
+        "view_id": 5
+    }
+    ```
+    """
+    await ctx.info(
+        "Getting compatible dimensions: dataset_id=%s, view_id=%s, "
+        "metrics=%s, dims=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error="Provide either dataset_id (built-in) or view_id 
(external).",
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        # ------------------------------------------------------------------
+        # Built-in dataset path
+        # ------------------------------------------------------------------
+        if request.dataset_id is not None:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            dataset_id: int = request.dataset_id
+            with event_logger.log_context(
+                action="mcp.get_compatible_dimensions.builtin"
+            ):
+                dataset = DatasetDAO.find_by_id(
+                    dataset_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+
+            # For built-in datasets all groupby columns are always compatible;
+            # there's no per-metric compatibility constraint at the SQL level.
+            dims = [
+                DimensionInfo(
+                    name=col.column_name,
+                    verbose_name=col.verbose_name or None,
+                    description=col.description or None,
+                    type=col.type or None,
+                    is_dttm=bool(col.is_dttm),
+                    groupby=bool(col.groupby),
+                    filterable=bool(col.filterable),
+                    source="builtin",
+                )
+                for col in dataset.columns
+                if col.groupby
+            ]
+
+            await ctx.info("Compatible dimensions (builtin): count=%d" % 
len(dims))
+            return CompatibleDimensionsResponse(
+                compatible_dimensions=dims,
+                source="builtin",
+            )
+
+        # ------------------------------------------------------------------
+        # External semantic view path
+        # ------------------------------------------------------------------
+        from superset.daos.semantic_layer import SemanticViewDAO
+
+        view_id: int = request.view_id  # type: ignore[assignment]
+        with 
event_logger.log_context(action="mcp.get_compatible_dimensions.external"):
+            view = SemanticViewDAO.find_by_id(view_id)
+
+        if view is None:
+            return SemanticLayerError.create(
+                error=f"No semantic view found with id: {view_id}.",
+                error_type="NotFound",
+            )
+
+        compatible_names = view.get_compatible_dimensions(
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+
+        # Enrich with full column metadata
+        all_cols = {col.column_name: col for col in view.columns}
+        dims = [
+            DimensionInfo(
+                name=name,
+                verbose_name=all_cols[name].verbose_name if name in all_cols 
else None,
+                description=all_cols[name].description if name in all_cols 
else None,
+                type=all_cols[name].type if name in all_cols else None,
+                is_dttm=all_cols[name].is_dttm if name in all_cols else False,
+                groupby=all_cols[name].groupby if name in all_cols else True,
+                filterable=all_cols[name].filterable if name in all_cols else 
True,
+                source="external",
+            )
+            for name in compatible_names
+        ]
+
+        await ctx.info(
+            "Compatible dimensions (external view id=%d): count=%d"
+            % (view.id, len(dims))
+        )
+        return CompatibleDimensionsResponse(
+            compatible_dimensions=dims,
+            source="external",
+        )
+
+    except Exception as exc:
+        logger.exception(
+            "Unexpected error in get_compatible_dimensions: %s: %s",
+            type(exc).__name__,
+            str(exc),
+        )
+        await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, 
str(exc)))
+        raise

Review Comment:
   On unexpected exceptions, get_compatible_dimensions re-raises, which will 
surface as an unstructured MCP server error. Return SemanticLayerError (and log 
details) to keep the tool contract consistent.



##########
superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py:
##########
@@ -0,0 +1,214 @@
+# 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: get_compatible_dimensions
+
+Returns dimensions compatible with the current metric/dimension selection.
+"""
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    CompatibleDimensionsResponse,
+    DimensionInfo,
+    GetCompatibleDimensionsRequest,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get compatible dimensions",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_compatible_dimensions(
+    request: GetCompatibleDimensionsRequest,
+    ctx: Context,
+) -> CompatibleDimensionsResponse | SemanticLayerError:
+    """Return dimensions compatible with the current metric/dimension 
selection.
+
+    Used to drive progressive disclosure in query builders: after the user
+    selects one or more metrics (and optionally some dimensions), this tool
+    returns the dimensions that can validly be added without breaking the
+    underlying query.
+
+    Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external).
+
+    For built-in datasets, returns all groupby-enabled columns from the same
+    dataset as the selected metrics, filtered to those whose datasets intersect
+    with all selected metrics.
+
+    For external semantic views, delegates to the view's
+    ``get_compatible_dimensions`` implementation.
+
+    Example:
+    ```json
+    {
+        "selected_metrics": ["revenue"],
+        "selected_dimensions": [],
+        "view_id": 5
+    }
+    ```
+    """
+    await ctx.info(
+        "Getting compatible dimensions: dataset_id=%s, view_id=%s, "
+        "metrics=%s, dims=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error="Provide either dataset_id (built-in) or view_id 
(external).",
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        # ------------------------------------------------------------------
+        # Built-in dataset path
+        # ------------------------------------------------------------------
+        if request.dataset_id is not None:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            dataset_id: int = request.dataset_id
+            with event_logger.log_context(
+                action="mcp.get_compatible_dimensions.builtin"
+            ):
+                dataset = DatasetDAO.find_by_id(
+                    dataset_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+
+            # For built-in datasets all groupby columns are always compatible;
+            # there's no per-metric compatibility constraint at the SQL level.
+            dims = [
+                DimensionInfo(
+                    name=col.column_name,
+                    verbose_name=col.verbose_name or None,
+                    description=col.description or None,
+                    type=col.type or None,
+                    is_dttm=bool(col.is_dttm),
+                    groupby=bool(col.groupby),
+                    filterable=bool(col.filterable),
+                    source="builtin",
+                )
+                for col in dataset.columns
+                if col.groupby
+            ]
+
+            await ctx.info("Compatible dimensions (builtin): count=%d" % 
len(dims))
+            return CompatibleDimensionsResponse(
+                compatible_dimensions=dims,
+                source="builtin",
+            )
+
+        # ------------------------------------------------------------------
+        # External semantic view path
+        # ------------------------------------------------------------------
+        from superset.daos.semantic_layer import SemanticViewDAO
+
+        view_id: int = request.view_id  # type: ignore[assignment]
+        with 
event_logger.log_context(action="mcp.get_compatible_dimensions.external"):
+            view = SemanticViewDAO.find_by_id(view_id)
+
+        if view is None:
+            return SemanticLayerError.create(
+                error=f"No semantic view found with id: {view_id}.",
+                error_type="NotFound",
+            )
+
+        compatible_names = view.get_compatible_dimensions(
+            request.selected_metrics,
+            request.selected_dimensions,
+        )

Review Comment:
   External semantic views are fetched via SemanticViewDAO without enforcing 
datasource_access, then view.get_compatible_dimensions/view.columns are 
accessed. Add an explicit view.raise_for_access() check and avoid revealing 
existence to unauthorized users (e.g. return NotFound on access denied).



##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -0,0 +1,397 @@
+# 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: get_table
+
+Query a data source (built-in dataset or external semantic view) using
+metric and dimension names, returning tabular results.
+"""
+
+import logging
+import time
+from typing import Any
+
+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.exceptions import OAuth2Error, OAuth2RedirectError, 
SupersetException
+from superset.extensions import event_logger
+from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    GetTableRequest,
+    GetTableResponse,
+    SemanticLayerError,
+)
+from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
+from superset.mcp_service.utils.oauth2_utils import 
build_oauth2_redirect_message
+
+logger = logging.getLogger(__name__)
+
+
+def _build_query_dict(
+    request: GetTableRequest,
+    time_col: str | None,
+) -> dict[str, Any]:
+    """Assemble the query dict for QueryContextFactory."""
+    filters: list[dict[str, Any]] = [
+        {"col": f.col, "op": f.op, "val": f.val} for f in request.filters
+    ]
+    if request.time_range and time_col:
+        filters.append(
+            {"col": time_col, "op": "TEMPORAL_RANGE", "val": 
request.time_range}
+        )
+
+    query_dict: dict[str, Any] = {
+        "filters": filters,
+        "columns": request.dimensions,
+        "metrics": request.metrics,
+        "row_limit": request.row_limit,
+        "order_desc": request.order_desc,
+    }
+    if time_col:
+        query_dict["granularity"] = time_col
+    if request.order_by:
+        query_dict["orderby"] = [
+            (col, not request.order_desc) for col in request.order_by
+        ]
+    return query_dict
+
+
+def _format_columns(
+    data: list[dict[str, Any]], raw_columns: list[str]
+) -> list[DataColumn]:
+    stats_rows = data[:5000]
+    columns_meta: list[DataColumn] = []
+    for col_name in raw_columns:
+        sample_values = [
+            row.get(col_name) for row in data[:3] if row.get(col_name) is not 
None
+        ]
+        data_type = "string"
+        if sample_values:
+            if all(isinstance(v, bool) for v in sample_values):
+                data_type = "boolean"
+            elif all(isinstance(v, (int, float)) for v in sample_values):
+                data_type = "numeric"
+
+        null_count = 0
+        unique_vals: set[str] = set()
+        for row in stats_rows:
+            val = row.get(col_name)
+            if val is None:
+                null_count += 1
+            else:
+                unique_vals.add(str(val))
+
+        columns_meta.append(
+            DataColumn(
+                name=col_name,
+                display_name=col_name.replace("_", " ").title(),
+                data_type=data_type,
+                sample_values=sample_values[:3],
+                null_count=null_count,
+                unique_count=len(unique_vals),
+            )
+        )
+    return columns_meta
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get table",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_table(  # noqa: C901
+    request: GetTableRequest,
+    ctx: Context,
+) -> GetTableResponse | SemanticLayerError:
+    """Query a data source using metrics and dimensions, returning tabular 
results.
+
+    Works with both built-in datasets and external semantic views. The
+    ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response.
+
+    Workflow:
+    1. list_metrics -> discover metrics and their compatible_dimensions
+    2. get_table -> query with chosen metrics and dimensions
+
+    Example (built-in):
+    ```json
+    {
+        "dataset_id": 42,
+        "metrics": ["revenue"],
+        "dimensions": ["region", "product_category"],
+        "time_range": "Last 30 days",
+        "row_limit": 500
+    }
+    ```
+
+    Example (external):
+    ```json
+    {
+        "view_id": 5,
+        "metrics": ["bookings"],
+        "dimensions": ["listing__country_name"],
+        "row_limit": 100
+    }
+    ```
+    """
+    await ctx.info(
+        "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, "
+        "dimensions=%s, row_limit=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.metrics,
+            request.dimensions,
+            request.row_limit,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error=(
+                "Provide either dataset_id (built-in dataset) or view_id "
+                "(external semantic view). Both are in the list_metrics 
response."
+            ),
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        from superset.commands.chart.data.get_data_command import 
ChartDataCommand
+        from superset.common.query_context_factory import QueryContextFactory
+
+        is_builtin = request.dataset_id is not None
+        datasource_type = "table" if is_builtin else "semantic_view"
+        if is_builtin:
+            assert request.dataset_id is not None
+            datasource_id = request.dataset_id
+        else:
+            assert request.view_id is not None
+            datasource_id = request.view_id
+
+        # ------------------------------------------------------------------
+        # Resolve datasource for metadata (time column, display name)
+        # ------------------------------------------------------------------
+        await ctx.report_progress(1, 4, "Resolving data source")
+        display_name: str = f"{'Dataset' if is_builtin else 'View'} 
{datasource_id}"
+        time_col: str | None = request.time_column
+        warnings: list[str] = []
+
+        if is_builtin:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            with 
event_logger.log_context(action="mcp.get_table.resolve_dataset"):
+                dataset = DatasetDAO.find_by_id(
+                    datasource_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+            display_name = dataset.table_name
+            if time_col is None and request.time_range:
+                time_col = getattr(dataset, "main_dttm_col", None)
+                if not time_col:
+                    return SemanticLayerError.create(
+                        error=(
+                            "time_range was provided but no temporal column is 
"
+                            "configured. Set time_column explicitly."
+                        ),
+                        error_type="ValidationError",
+                    )
+        else:
+            from superset.daos.semantic_layer import SemanticViewDAO
+
+            with event_logger.log_context(action="mcp.get_table.resolve_view"):
+                view = SemanticViewDAO.find_by_id(datasource_id)
+            if view is None:
+                return SemanticLayerError.create(
+                    error=f"No semantic view found with id: 
{request.view_id}.",
+                    error_type="NotFound",
+                )
+            display_name = view.name
+            if time_col is None and request.time_range:
+                # Use first datetime dimension as the time column
+                dttm_cols = [c for c in view.columns if c.is_dttm]
+                if dttm_cols:
+                    time_col = dttm_cols[0].column_name
+                else:
+                    warnings.append(
+                        "time_range provided but no datetime dimension found; "
+                        "time filter will not be applied."
+                    )
+                    time_col = None
+
+        # ------------------------------------------------------------------
+        # Build and execute query
+        # ------------------------------------------------------------------
+        await ctx.report_progress(2, 4, "Building query")
+        query_dict = _build_query_dict(request, time_col)
+
+        await ctx.debug("Query dict: %s" % (sorted(query_dict.keys()),))
+        await ctx.report_progress(3, 4, "Executing query")
+        start_time = time.time()
+
+        with event_logger.log_context(action="mcp.get_table.execute"):
+            factory = QueryContextFactory()
+            query_context = factory.create(
+                datasource={"id": datasource_id, "type": datasource_type},
+                queries=[query_dict],
+                form_data={},
+                force=not request.use_cache or request.force_refresh,
+            )
+            command = ChartDataCommand(query_context)
+            command.validate()
+            result = command.run()
+
+        query_duration_ms = int((time.time() - start_time) * 1000)
+
+        if not result or "queries" not in result or not result["queries"]:
+            return SemanticLayerError.create(
+                error="Query returned no results.",
+                error_type="EmptyQuery",
+            )
+
+        # ------------------------------------------------------------------
+        # Format response
+        # ------------------------------------------------------------------
+        await ctx.report_progress(4, 4, "Formatting results")
+        query_result = result["queries"][0]
+        data = query_result.get("data", [])
+        raw_columns = query_result.get("colnames", [])
+
+        if not data:
+            return GetTableResponse(
+                columns=[],
+                data=[],
+                row_count=0,
+                total_rows=0,
+                summary=f"'{display_name}': query returned no data.",
+                source="builtin" if is_builtin else "external",
+                dataset_id=request.dataset_id,
+                dataset_name=display_name if is_builtin else None,
+                view_id=request.view_id,
+                view_name=display_name if not is_builtin else None,
+                performance=PerformanceMetadata(
+                    query_duration_ms=query_duration_ms,
+                    cache_status="no_data",
+                ),
+                cache_status=get_cache_status_from_result(
+                    query_result, force_refresh=request.force_refresh
+                ),
+                warnings=warnings,
+            )
+
+        columns_meta = _format_columns(data, raw_columns)
+        cache_status = get_cache_status_from_result(
+            query_result, force_refresh=request.force_refresh
+        )
+        cache_label = "cached" if cache_status and cache_status.cache_hit else 
"fresh"
+        summary = (
+            f"'{display_name}': {len(data)} rows, "
+            f"{len(raw_columns)} columns ({cache_label})."
+        )
+
+        await ctx.info(
+            "get_table complete: rows=%d, columns=%d, duration=%dms"
+            % (len(data), len(raw_columns), query_duration_ms)
+        )
+
+        return GetTableResponse(
+            columns=columns_meta,
+            data=data,
+            row_count=len(data),
+            total_rows=query_result.get("rowcount"),
+            summary=summary,
+            source="builtin" if is_builtin else "external",
+            dataset_id=request.dataset_id,
+            dataset_name=display_name if is_builtin else None,
+            view_id=request.view_id,
+            view_name=display_name if not is_builtin else None,
+            performance=PerformanceMetadata(
+                query_duration_ms=query_duration_ms,
+                cache_status=cache_label,
+            ),
+            cache_status=cache_status,
+            warnings=warnings,
+        )
+
+    except OAuth2RedirectError as exc:
+        redirect_msg = build_oauth2_redirect_message(exc)
+        await ctx.error("OAuth2 redirect required: %s" % redirect_msg)
+        return SemanticLayerError.create(
+            error=redirect_msg,
+            error_type="OAuth2Redirect",
+        )
+
+    except OAuth2Error as exc:
+        await ctx.error("OAuth2 error: %s" % str(exc))
+        return SemanticLayerError.create(
+            error=f"OAuth2 authentication error: {exc}",
+            error_type="OAuth2Error",
+        )
+
+    except (CommandException, SupersetException) as exc:
+        await ctx.error("Query failed: %s" % str(exc))
+        return SemanticLayerError.create(
+            error=f"Query execution failed: {exc}",
+            error_type="QueryError",
+        )
+
+    except SQLAlchemyError as exc:
+        await ctx.error("Database error: %s" % str(exc))
+        return SemanticLayerError.create(
+            error=f"Database error: {exc}",
+            error_type="DatabaseError",
+        )
+
+    except Exception as exc:
+        logger.exception(
+            "Unexpected error in get_table: %s: %s", type(exc).__name__, 
str(exc)
+        )
+        await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, 
str(exc)))
+        raise

Review Comment:
   On unexpected exceptions, get_table re-raises, which will surface as an 
unstructured MCP server error to clients. Align with other MCP tools by 
returning a SemanticLayerError with a generic message and error_type (while 
logging server-side details).



##########
superset/mcp_service/semantic_layer/tool/list_metrics.py:
##########
@@ -0,0 +1,267 @@
+# 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: list_metrics
+
+Unified metric discovery across built-in datasets and external semantic views.
+"""
+
+import logging
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    DimensionInfo,
+    ListMetricsRequest,
+    MetricInfo,
+    MetricList,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _matches_search(text: str | None, search: str) -> bool:
+    if not text:
+        return False
+    return search.lower() in text.lower()
+
+
+def _builtin_compatible_dims(dataset: Any) -> list[DimensionInfo]:
+    """Return groupby-enabled columns as compatible dimensions for a built-in 
metric."""
+    return [
+        DimensionInfo(
+            name=col.column_name,
+            verbose_name=col.verbose_name or None,
+            description=col.description or None,
+            type=col.type or None,
+            is_dttm=bool(col.is_dttm),
+            groupby=bool(col.groupby),
+            filterable=bool(col.filterable),
+            source="builtin",
+        )
+        for col in dataset.columns
+        if col.groupby
+    ]
+
+
+def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
+    """Collect metrics from built-in SqlaTable datasets."""
+    from sqlalchemy.orm import subqueryload
+
+    from superset.connectors.sqla.models import SqlaTable
+    from superset.daos.dataset import DatasetDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
+        if request.dataset_id is not None:
+            dataset = DatasetDAO.find_by_id(
+                request.dataset_id,
+                query_options=[
+                    subqueryload(SqlaTable.columns),
+                    subqueryload(SqlaTable.metrics),
+                ],
+            )
+            datasets = [dataset] if dataset else []
+        else:
+            datasets = DatasetDAO.find_all()
+
+    results: list[MetricInfo] = []
+    for dataset in datasets:
+        compat_dims = (
+            _builtin_compatible_dims(dataset)
+            if request.include_compatible_dimensions
+            else []
+        )
+        for metric in dataset.metrics:
+            name = metric.metric_name or ""
+            desc = metric.description or ""
+            if request.search and not (
+                _matches_search(name, request.search)
+                or _matches_search(desc, request.search)
+            ):
+                continue
+            results.append(
+                MetricInfo(
+                    name=name,
+                    verbose_name=metric.verbose_name or None,
+                    description=desc or None,
+                    expression=metric.expression or None,
+                    d3format=metric.d3format or None,
+                    warning_text=metric.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                    compatible_dimensions=compat_dims,
+                )
+            )
+    return results
+
+
+async def _collect_external_metrics(
+    request: ListMetricsRequest,
+    ctx: Context,
+) -> list[MetricInfo]:
+    """Collect metrics from external SemanticView models."""
+    from superset.daos.semantic_layer import SemanticViewDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.external_query"):
+        if request.view_id is not None:
+            view = SemanticViewDAO.find_by_id(request.view_id)
+            views = [view] if view else []
+        else:
+            views = SemanticViewDAO.find_all()
+
+    await ctx.debug("Found %d semantic views to scan for metrics" % len(views))
+
+    results: list[MetricInfo] = []
+    for view in views:
+        try:
+            raw_metrics = view.metrics
+            raw_cols = view.columns if request.include_compatible_dimensions 
else []
+            compat_dims = [
+                DimensionInfo(
+                    name=col.column_name,
+                    verbose_name=col.verbose_name,
+                    description=col.description,
+                    type=col.type,
+                    is_dttm=col.is_dttm,
+                    groupby=col.groupby,
+                    filterable=col.filterable,
+                    source="external",
+                )
+                for col in raw_cols
+            ]
+            for metric in raw_metrics:
+                name = metric.metric_name or ""
+                desc = metric.description or ""
+                if request.search and not (
+                    _matches_search(name, request.search)
+                    or _matches_search(desc, request.search)
+                ):
+                    continue
+                results.append(
+                    MetricInfo(
+                        name=name,
+                        description=desc or None,
+                        expression=metric.expression or None,
+                        source="external",
+                        view_id=view.id,
+                        view_name=view.name,
+                        compatible_dimensions=compat_dims,
+                    )
+                )
+        except Exception as exc:  # noqa: BLE001
+            # External registry may be empty in OSS — degrade gracefully
+            await ctx.warning(
+                "Could not load metrics for view id=%s: %s" % (view.id, 
str(exc))
+            )
+    return results
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="List metrics",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def list_metrics(
+    request: ListMetricsRequest | None = None,
+    ctx: Context | None = None,
+) -> MetricList | SemanticLayerError:
+    """List available metrics across built-in datasets and external semantic 
views.
+
+    This is the primary entry point for semantic layer exploration. Returns a
+    unified list of metrics from all data sources the current user can access,
+    with compatible dimensions included inline.
+
+    Workflow:
+    1. list_metrics -> discover available metrics and their compatible 
dimensions
+    2. get_table -> query data using chosen metrics and dimensions
+
+    Use ``search`` to filter by metric name or description. Use ``dataset_id``
+    or ``view_id`` to scope to a specific data source.
+
+    Example:
+    ```json
+    {"search": "revenue", "include_compatible_dimensions": true, "page": 1}
+    ```
+    """
+    if ctx is None:
+        raise RuntimeError("FastMCP context is required for list_metrics")
+
+    request = request or ListMetricsRequest()
+
+    await ctx.info(
+        "Listing metrics: search=%s, dataset_id=%s, view_id=%s, page=%s"
+        % (request.search, request.dataset_id, request.view_id, request.page)
+    )
+
+    if not user_can_view_data_model_metadata():
+        await ctx.warning("Metric listing blocked by data-model privacy 
controls")
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    try:
+        all_metrics: list[MetricInfo] = []
+
+        if request.view_id is None:
+            all_metrics.extend(_collect_builtin_metrics(request))
+            await ctx.debug("Collected %d built-in metrics" % len(all_metrics))
+
+        if request.dataset_id is None:
+            external = await _collect_external_metrics(request, ctx)
+            all_metrics.extend(external)
+            await ctx.debug("Collected %d external metrics" % len(external))
+
+        total_count = len(all_metrics)
+        total_pages = max(1, (total_count + request.page_size - 1) // 
request.page_size)
+        start = (request.page - 1) * request.page_size
+        page_metrics = all_metrics[start : start + request.page_size]
+
+        await ctx.info(
+            "Metrics listed: total=%d, page=%d/%d, returned=%d"
+            % (total_count, request.page, total_pages, len(page_metrics))
+        )
+
+        return MetricList(
+            metrics=page_metrics,
+            total_count=total_count,
+            page=request.page,
+            page_size=request.page_size,
+            total_pages=total_pages,
+        )
+
+    except Exception as exc:
+        logger.exception(
+            "Unexpected error in list_metrics: %s: %s", type(exc).__name__, 
str(exc)
+        )
+        await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, 
str(exc)))
+        raise

Review Comment:
   On unexpected exceptions, list_metrics re-raises, which will surface as an 
unstructured MCP server error to clients. Other MCP tools (e.g. query_dataset) 
return a typed error object instead; list_metrics should return 
SemanticLayerError for unexpected failures.



##########
superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py:
##########
@@ -0,0 +1,214 @@
+# 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: get_compatible_dimensions
+
+Returns dimensions compatible with the current metric/dimension selection.
+"""
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    CompatibleDimensionsResponse,
+    DimensionInfo,
+    GetCompatibleDimensionsRequest,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get compatible dimensions",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_compatible_dimensions(
+    request: GetCompatibleDimensionsRequest,
+    ctx: Context,
+) -> CompatibleDimensionsResponse | SemanticLayerError:
+    """Return dimensions compatible with the current metric/dimension 
selection.
+
+    Used to drive progressive disclosure in query builders: after the user
+    selects one or more metrics (and optionally some dimensions), this tool
+    returns the dimensions that can validly be added without breaking the
+    underlying query.
+
+    Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external).
+
+    For built-in datasets, returns all groupby-enabled columns from the same
+    dataset as the selected metrics, filtered to those whose datasets intersect
+    with all selected metrics.

Review Comment:
   The built-in dataset description says compatible dimensions are filtered by 
intersection across selected metrics' datasets, but the implementation ignores 
selected_metrics/selected_dimensions and simply returns all groupby-enabled 
columns for the dataset_id. The docstring should be updated to match the 
implemented behavior.



##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -0,0 +1,397 @@
+# 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: get_table
+
+Query a data source (built-in dataset or external semantic view) using
+metric and dimension names, returning tabular results.
+"""
+
+import logging
+import time
+from typing import Any
+
+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.exceptions import OAuth2Error, OAuth2RedirectError, 
SupersetException
+from superset.extensions import event_logger
+from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    GetTableRequest,
+    GetTableResponse,
+    SemanticLayerError,
+)
+from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
+from superset.mcp_service.utils.oauth2_utils import 
build_oauth2_redirect_message
+
+logger = logging.getLogger(__name__)
+
+
+def _build_query_dict(
+    request: GetTableRequest,
+    time_col: str | None,
+) -> dict[str, Any]:
+    """Assemble the query dict for QueryContextFactory."""
+    filters: list[dict[str, Any]] = [
+        {"col": f.col, "op": f.op, "val": f.val} for f in request.filters
+    ]
+    if request.time_range and time_col:
+        filters.append(
+            {"col": time_col, "op": "TEMPORAL_RANGE", "val": 
request.time_range}
+        )
+
+    query_dict: dict[str, Any] = {
+        "filters": filters,
+        "columns": request.dimensions,
+        "metrics": request.metrics,
+        "row_limit": request.row_limit,
+        "order_desc": request.order_desc,
+    }
+    if time_col:
+        query_dict["granularity"] = time_col
+    if request.order_by:
+        query_dict["orderby"] = [
+            (col, not request.order_desc) for col in request.order_by
+        ]
+    return query_dict
+
+
+def _format_columns(
+    data: list[dict[str, Any]], raw_columns: list[str]
+) -> list[DataColumn]:
+    stats_rows = data[:5000]
+    columns_meta: list[DataColumn] = []
+    for col_name in raw_columns:
+        sample_values = [
+            row.get(col_name) for row in data[:3] if row.get(col_name) is not 
None
+        ]
+        data_type = "string"
+        if sample_values:
+            if all(isinstance(v, bool) for v in sample_values):
+                data_type = "boolean"
+            elif all(isinstance(v, (int, float)) for v in sample_values):
+                data_type = "numeric"
+
+        null_count = 0
+        unique_vals: set[str] = set()
+        for row in stats_rows:
+            val = row.get(col_name)
+            if val is None:
+                null_count += 1
+            else:
+                unique_vals.add(str(val))
+
+        columns_meta.append(
+            DataColumn(
+                name=col_name,
+                display_name=col_name.replace("_", " ").title(),
+                data_type=data_type,
+                sample_values=sample_values[:3],
+                null_count=null_count,
+                unique_count=len(unique_vals),
+            )
+        )
+    return columns_meta
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get table",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_table(  # noqa: C901
+    request: GetTableRequest,
+    ctx: Context,
+) -> GetTableResponse | SemanticLayerError:
+    """Query a data source using metrics and dimensions, returning tabular 
results.
+
+    Works with both built-in datasets and external semantic views. The
+    ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response.
+
+    Workflow:
+    1. list_metrics -> discover metrics and their compatible_dimensions
+    2. get_table -> query with chosen metrics and dimensions
+
+    Example (built-in):
+    ```json
+    {
+        "dataset_id": 42,
+        "metrics": ["revenue"],
+        "dimensions": ["region", "product_category"],
+        "time_range": "Last 30 days",
+        "row_limit": 500
+    }
+    ```
+
+    Example (external):
+    ```json
+    {
+        "view_id": 5,
+        "metrics": ["bookings"],
+        "dimensions": ["listing__country_name"],
+        "row_limit": 100
+    }
+    ```
+    """
+    await ctx.info(
+        "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, "
+        "dimensions=%s, row_limit=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.metrics,
+            request.dimensions,
+            request.row_limit,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error=(
+                "Provide either dataset_id (built-in dataset) or view_id "
+                "(external semantic view). Both are in the list_metrics 
response."
+            ),
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        from superset.commands.chart.data.get_data_command import 
ChartDataCommand
+        from superset.common.query_context_factory import QueryContextFactory
+
+        is_builtin = request.dataset_id is not None
+        datasource_type = "table" if is_builtin else "semantic_view"
+        if is_builtin:
+            assert request.dataset_id is not None
+            datasource_id = request.dataset_id
+        else:
+            assert request.view_id is not None
+            datasource_id = request.view_id
+
+        # ------------------------------------------------------------------
+        # Resolve datasource for metadata (time column, display name)
+        # ------------------------------------------------------------------
+        await ctx.report_progress(1, 4, "Resolving data source")
+        display_name: str = f"{'Dataset' if is_builtin else 'View'} 
{datasource_id}"
+        time_col: str | None = request.time_column
+        warnings: list[str] = []
+
+        if is_builtin:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            with 
event_logger.log_context(action="mcp.get_table.resolve_dataset"):
+                dataset = DatasetDAO.find_by_id(
+                    datasource_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+            display_name = dataset.table_name
+            if time_col is None and request.time_range:
+                time_col = getattr(dataset, "main_dttm_col", None)
+                if not time_col:
+                    return SemanticLayerError.create(
+                        error=(
+                            "time_range was provided but no temporal column is 
"
+                            "configured. Set time_column explicitly."
+                        ),
+                        error_type="ValidationError",
+                    )

Review Comment:
   When time_range is provided and time_column is inferred from 
dataset.main_dttm_col, get_table does not validate that the column exists (or 
is marked as datetime). query_dataset performs these validations and provides 
clearer errors/warnings; get_table should mirror that behavior to avoid cryptic 
downstream query errors.



##########
superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py:
##########
@@ -0,0 +1,214 @@
+# 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: get_compatible_metrics
+
+Returns metrics compatible with the current dimension/metric selection.
+"""
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    CompatibleMetricsResponse,
+    GetCompatibleMetricsRequest,
+    MetricInfo,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get compatible metrics",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_compatible_metrics(
+    request: GetCompatibleMetricsRequest,
+    ctx: Context,
+) -> CompatibleMetricsResponse | SemanticLayerError:
+    """Return metrics compatible with the current dimension/metric selection.
+
+    Used to progressively refine a query: given a set of already-selected
+    metrics and dimensions, returns the additional metrics that can be
+    combined without breaking the underlying semantic constraints.
+
+    Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external).
+
+    For built-in datasets, all metrics from the dataset are considered
+    compatible (SQL GROUP BY imposes no metric-level constraints).
+
+    For external semantic views, delegates to the view's
+    ``get_compatible_metrics`` implementation.
+
+    Example:
+    ```json
+    {
+        "selected_metrics": [],
+        "selected_dimensions": ["region"],
+        "view_id": 5
+    }
+    ```
+    """
+    await ctx.info(
+        "Getting compatible metrics: dataset_id=%s, view_id=%s, "
+        "metrics=%s, dims=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error="Provide either dataset_id (built-in) or view_id 
(external).",
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        # ------------------------------------------------------------------
+        # Built-in dataset path
+        # ------------------------------------------------------------------
+        if request.dataset_id is not None:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            with 
event_logger.log_context(action="mcp.get_compatible_metrics.builtin"):
+                dataset = DatasetDAO.find_by_id(
+                    request.dataset_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+
+            # All metrics on a SQL dataset are always mutually compatible.
+            compatible = [
+                MetricInfo(
+                    name=m.metric_name,
+                    verbose_name=m.verbose_name or None,
+                    description=m.description or None,
+                    expression=m.expression or None,
+                    d3format=m.d3format or None,
+                    warning_text=m.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                )
+                for m in dataset.metrics
+            ]
+
+            await ctx.info("Compatible metrics (builtin): count=%d" % 
len(compatible))
+            return CompatibleMetricsResponse(
+                compatible_metrics=compatible,
+                source="builtin",
+            )
+
+        # ------------------------------------------------------------------
+        # External semantic view path
+        # ------------------------------------------------------------------
+        from superset.daos.semantic_layer import SemanticViewDAO
+
+        view_id: int = request.view_id  # type: ignore[assignment]
+        with 
event_logger.log_context(action="mcp.get_compatible_metrics.external"):
+            view = SemanticViewDAO.find_by_id(view_id)
+
+        if view is None:
+            return SemanticLayerError.create(
+                error=f"No semantic view found with id: {view_id}.",
+                error_type="NotFound",
+            )
+
+        compatible_names = view.get_compatible_metrics(
+            request.selected_metrics,
+            request.selected_dimensions,
+        )

Review Comment:
   External semantic views are fetched via SemanticViewDAO without enforcing 
datasource_access, then view.get_compatible_metrics/view.metrics are accessed. 
Add an explicit view.raise_for_access() check and avoid revealing existence to 
unauthorized users (e.g. return NotFound on access denied).



##########
superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py:
##########
@@ -0,0 +1,214 @@
+# 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: get_compatible_metrics
+
+Returns metrics compatible with the current dimension/metric selection.
+"""
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    CompatibleMetricsResponse,
+    GetCompatibleMetricsRequest,
+    MetricInfo,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="Get compatible metrics",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def get_compatible_metrics(
+    request: GetCompatibleMetricsRequest,
+    ctx: Context,
+) -> CompatibleMetricsResponse | SemanticLayerError:
+    """Return metrics compatible with the current dimension/metric selection.
+
+    Used to progressively refine a query: given a set of already-selected
+    metrics and dimensions, returns the additional metrics that can be
+    combined without breaking the underlying semantic constraints.
+
+    Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external).
+
+    For built-in datasets, all metrics from the dataset are considered
+    compatible (SQL GROUP BY imposes no metric-level constraints).
+
+    For external semantic views, delegates to the view's
+    ``get_compatible_metrics`` implementation.
+
+    Example:
+    ```json
+    {
+        "selected_metrics": [],
+        "selected_dimensions": ["region"],
+        "view_id": 5
+    }
+    ```
+    """
+    await ctx.info(
+        "Getting compatible metrics: dataset_id=%s, view_id=%s, "
+        "metrics=%s, dims=%s"
+        % (
+            request.dataset_id,
+            request.view_id,
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+    )
+
+    if not user_can_view_data_model_metadata():
+        return SemanticLayerError.create(
+            error="You don't have permission to access dataset details for 
your role.",
+            error_type=DATA_MODEL_METADATA_ERROR_TYPE,
+        )
+
+    if request.dataset_id is None and request.view_id is None:
+        return SemanticLayerError.create(
+            error="Provide either dataset_id (built-in) or view_id 
(external).",
+            error_type="ValidationError",
+        )
+    if request.dataset_id is not None and request.view_id is not None:
+        return SemanticLayerError.create(
+            error="Provide only one of dataset_id or view_id, not both.",
+            error_type="ValidationError",
+        )
+
+    try:
+        # ------------------------------------------------------------------
+        # Built-in dataset path
+        # ------------------------------------------------------------------
+        if request.dataset_id is not None:
+            from sqlalchemy.orm import subqueryload
+
+            from superset.connectors.sqla.models import SqlaTable
+            from superset.daos.dataset import DatasetDAO
+
+            with 
event_logger.log_context(action="mcp.get_compatible_metrics.builtin"):
+                dataset = DatasetDAO.find_by_id(
+                    request.dataset_id,
+                    query_options=[
+                        subqueryload(SqlaTable.columns),
+                        subqueryload(SqlaTable.metrics),
+                    ],
+                )
+
+            if dataset is None:
+                return SemanticLayerError.create(
+                    error=f"No dataset found with id: {request.dataset_id}.",
+                    error_type="NotFound",
+                )
+
+            # All metrics on a SQL dataset are always mutually compatible.
+            compatible = [
+                MetricInfo(
+                    name=m.metric_name,
+                    verbose_name=m.verbose_name or None,
+                    description=m.description or None,
+                    expression=m.expression or None,
+                    d3format=m.d3format or None,
+                    warning_text=m.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                )
+                for m in dataset.metrics
+            ]
+
+            await ctx.info("Compatible metrics (builtin): count=%d" % 
len(compatible))
+            return CompatibleMetricsResponse(
+                compatible_metrics=compatible,
+                source="builtin",
+            )
+
+        # ------------------------------------------------------------------
+        # External semantic view path
+        # ------------------------------------------------------------------
+        from superset.daos.semantic_layer import SemanticViewDAO
+
+        view_id: int = request.view_id  # type: ignore[assignment]
+        with 
event_logger.log_context(action="mcp.get_compatible_metrics.external"):
+            view = SemanticViewDAO.find_by_id(view_id)
+
+        if view is None:
+            return SemanticLayerError.create(
+                error=f"No semantic view found with id: {view_id}.",
+                error_type="NotFound",
+            )
+
+        compatible_names = view.get_compatible_metrics(
+            request.selected_metrics,
+            request.selected_dimensions,
+        )
+
+        # Enrich with full metric metadata
+        all_metrics_map = {m.metric_name: m for m in view.metrics}
+        compatible = [
+            MetricInfo(
+                name=name,
+                description=(
+                    all_metrics_map[name].description
+                    if name in all_metrics_map
+                    else None
+                ),
+                expression=(
+                    all_metrics_map[name].expression
+                    if name in all_metrics_map
+                    else None
+                ),
+                source="external",
+                view_id=view.id,
+                view_name=view.name,
+            )
+            for name in compatible_names
+        ]
+
+        await ctx.info(
+            "Compatible metrics (external view id=%d): count=%d"
+            % (view.id, len(compatible))
+        )
+        return CompatibleMetricsResponse(
+            compatible_metrics=compatible,
+            source="external",
+        )
+
+    except Exception as exc:
+        logger.exception(
+            "Unexpected error in get_compatible_metrics: %s: %s",
+            type(exc).__name__,
+            str(exc),
+        )
+        await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, 
str(exc)))
+        raise

Review Comment:
   On unexpected exceptions, get_compatible_metrics re-raises, which will 
surface as an unstructured MCP server error. Return SemanticLayerError (and log 
details) to keep the tool contract consistent.



##########
superset/mcp_service/semantic_layer/tool/list_metrics.py:
##########
@@ -0,0 +1,267 @@
+# 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: list_metrics
+
+Unified metric discovery across built-in datasets and external semantic views.
+"""
+
+import logging
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.privacy import (
+    DATA_MODEL_METADATA_ERROR_TYPE,
+    requires_data_model_metadata_access,
+    user_can_view_data_model_metadata,
+)
+from superset.mcp_service.semantic_layer.schemas import (
+    DimensionInfo,
+    ListMetricsRequest,
+    MetricInfo,
+    MetricList,
+    SemanticLayerError,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _matches_search(text: str | None, search: str) -> bool:
+    if not text:
+        return False
+    return search.lower() in text.lower()
+
+
+def _builtin_compatible_dims(dataset: Any) -> list[DimensionInfo]:
+    """Return groupby-enabled columns as compatible dimensions for a built-in 
metric."""
+    return [
+        DimensionInfo(
+            name=col.column_name,
+            verbose_name=col.verbose_name or None,
+            description=col.description or None,
+            type=col.type or None,
+            is_dttm=bool(col.is_dttm),
+            groupby=bool(col.groupby),
+            filterable=bool(col.filterable),
+            source="builtin",
+        )
+        for col in dataset.columns
+        if col.groupby
+    ]
+
+
+def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
+    """Collect metrics from built-in SqlaTable datasets."""
+    from sqlalchemy.orm import subqueryload
+
+    from superset.connectors.sqla.models import SqlaTable
+    from superset.daos.dataset import DatasetDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
+        if request.dataset_id is not None:
+            dataset = DatasetDAO.find_by_id(
+                request.dataset_id,
+                query_options=[
+                    subqueryload(SqlaTable.columns),
+                    subqueryload(SqlaTable.metrics),
+                ],
+            )
+            datasets = [dataset] if dataset else []
+        else:
+            datasets = DatasetDAO.find_all()
+
+    results: list[MetricInfo] = []
+    for dataset in datasets:
+        compat_dims = (
+            _builtin_compatible_dims(dataset)
+            if request.include_compatible_dimensions
+            else []
+        )
+        for metric in dataset.metrics:
+            name = metric.metric_name or ""
+            desc = metric.description or ""
+            if request.search and not (
+                _matches_search(name, request.search)
+                or _matches_search(desc, request.search)
+            ):
+                continue
+            results.append(
+                MetricInfo(
+                    name=name,
+                    verbose_name=metric.verbose_name or None,
+                    description=desc or None,
+                    expression=metric.expression or None,
+                    d3format=metric.d3format or None,
+                    warning_text=metric.warning_text or None,
+                    source="builtin",
+                    dataset_id=dataset.id,
+                    dataset_name=dataset.table_name,
+                    compatible_dimensions=compat_dims,
+                )
+            )
+    return results
+
+
+async def _collect_external_metrics(
+    request: ListMetricsRequest,
+    ctx: Context,
+) -> list[MetricInfo]:
+    """Collect metrics from external SemanticView models."""
+    from superset.daos.semantic_layer import SemanticViewDAO
+
+    with event_logger.log_context(action="mcp.list_metrics.external_query"):
+        if request.view_id is not None:
+            view = SemanticViewDAO.find_by_id(request.view_id)
+            views = [view] if view else []
+        else:
+            views = SemanticViewDAO.find_all()
+
+    await ctx.debug("Found %d semantic views to scan for metrics" % len(views))
+
+    results: list[MetricInfo] = []
+    for view in views:
+        try:
+            raw_metrics = view.metrics
+            raw_cols = view.columns if request.include_compatible_dimensions 
else []
+            compat_dims = [
+                DimensionInfo(
+                    name=col.column_name,
+                    verbose_name=col.verbose_name,
+                    description=col.description,
+                    type=col.type,
+                    is_dttm=col.is_dttm,
+                    groupby=col.groupby,
+                    filterable=col.filterable,
+                    source="external",
+                )
+                for col in raw_cols
+            ]
+            for metric in raw_metrics:
+                name = metric.metric_name or ""
+                desc = metric.description or ""
+                if request.search and not (
+                    _matches_search(name, request.search)
+                    or _matches_search(desc, request.search)
+                ):
+                    continue
+                results.append(
+                    MetricInfo(
+                        name=name,
+                        description=desc or None,
+                        expression=metric.expression or None,
+                        source="external",
+                        view_id=view.id,
+                        view_name=view.name,
+                        compatible_dimensions=compat_dims,
+                    )
+                )
+        except Exception as exc:  # noqa: BLE001
+            # External registry may be empty in OSS — degrade gracefully
+            await ctx.warning(
+                "Could not load metrics for view id=%s: %s" % (view.id, 
str(exc))
+            )
+    return results
+
+
+@tool(
+    tags=["data", "semantic"],
+    class_permission_name="Dataset",
+    annotations=ToolAnnotations(
+        title="List metrics",
+        readOnlyHint=True,
+        destructiveHint=False,
+    ),
+)
+@requires_data_model_metadata_access
+async def list_metrics(
+    request: ListMetricsRequest | None = None,
+    ctx: Context | None = None,
+) -> MetricList | SemanticLayerError:

Review Comment:
   These new semantic-layer tools add significant behavior (metric discovery + 
querying) but there are no corresponding unit tests, while other MCP tools are 
covered under tests/unit_tests/mcp_service/**. Adding unit tests would help 
catch permission-filtering, validation, and error-surface regressions.



-- 
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