test(mcp): close systematic test-coverage gaps in mcp_service (#41924)

tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py

@@ -22,6 +22,7 @@ from __future__ import annotations
 import importlib
 from collections.abc import Generator
 from types import ModuleType
+from typing import Any
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
@@ -282,3 +283,91 @@ async def test_get_compatible_dimensions_external_not_found(
 
     assert data["success"] is False
     assert data["error_type"] == "NotFound"
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_dimensions_builtin_empty_selection(
+    mcp_server: FastMCP,
+) -> None:
+    """Explicitly empty selected_metrics/selected_dimensions is not an error.
+
+    An empty selection is the natural starting state of a query builder
+    (nothing picked yet), so it must return the full groupby-enabled column
+    set rather than a validation failure.
+    """
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_dimensions",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "selected_metrics": [],
+                        "selected_dimensions": [],
+                    }
+                },
+            )
+        data: dict[str, Any] = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    names: set[str] = {d["name"] for d in data["compatible_dimensions"]}
+    assert names == {"region", "category"}
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_dimensions_external_empty_selection(
+    mcp_server: FastMCP,
+) -> None:
+    """External views handle an explicitly empty selection without error."""
+    mock_view: MagicMock = _make_view(5)
+    mock_view.get_compatible_dimensions = MagicMock(return_value=[])
+
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=mock_view,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_dimensions",
+                {
+                    "request": {
+                        "view_id": 5,
+                        "selected_metrics": [],
+                        "selected_dimensions": [],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["compatible_dimensions"] == []
+    mock_view.get_compatible_dimensions.assert_called_once_with([], [])
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_dimensions_unicode_unknown_selection_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """Unicode/special-character names in an unknown selection surface cleanly."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_dimensions",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "selected_metrics": ["日本語_metric"],
+                        "selected_dimensions": ["special!chars?"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown metric: '日本語_metric'" in data["error"]
+    assert "Unknown dimension: 'special!chars?'" in data["error"]

tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py

@@ -22,6 +22,7 @@ from __future__ import annotations
 import importlib
 from collections.abc import Generator
 from types import ModuleType
+from typing import Any
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
@@ -278,3 +279,91 @@ async def test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
 
     assert data["success"] is False
     assert data["error_type"] == "NotFound"
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_metrics_builtin_empty_selection(
+    mcp_server: FastMCP,
+) -> None:
+    """Explicitly empty selected_metrics/selected_dimensions is not an error.
+
+    An empty selection is the natural starting state of a query builder
+    (nothing picked yet), so it must return all dataset metrics rather than
+    a validation failure.
+    """
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_metrics",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "selected_metrics": [],
+                        "selected_dimensions": [],
+                    }
+                },
+            )
+        data: dict[str, Any] = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    names: set[str] = {m["name"] for m in data["compatible_metrics"]}
+    assert names == {"count", "revenue"}
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_metrics_external_empty_selection(
+    mcp_server: FastMCP,
+) -> None:
+    """External views handle an explicitly empty selection without error."""
+    mock_view: MagicMock = _make_view(5)
+    mock_view.get_compatible_metrics = MagicMock(return_value=[])
+
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=mock_view,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_metrics",
+                {
+                    "request": {
+                        "view_id": 5,
+                        "selected_metrics": [],
+                        "selected_dimensions": [],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["compatible_metrics"] == []
+    mock_view.get_compatible_metrics.assert_called_once_with([], [])
+
+
+@pytest.mark.asyncio
+async def test_get_compatible_metrics_unicode_unknown_selection_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """Unicode/special-character names in an unknown selection surface cleanly."""
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_compatible_metrics",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "selected_metrics": ["日本語_metric"],
+                        "selected_dimensions": ["special!chars?"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown metric: '日本語_metric'" in data["error"]
+    assert "Unknown dimension: 'special!chars?'" in data["error"]

tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py

@@ -22,6 +22,7 @@ from __future__ import annotations
 import importlib
 from collections.abc import Generator
 from types import ModuleType
+from typing import Any
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
@@ -297,3 +298,231 @@ async def test_get_table_external_time_range_without_dttm_validation_error(
     assert data["success"] is False
     assert data["error_type"] == "ValidationError"
     assert "no datetime dimension" in data["message"]
+
+
+@pytest.mark.asyncio
+async def test_get_table_dataset_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when dataset_id doesn't resolve to a dataset."""
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=None):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"dataset_id": 999999, "metrics": ["revenue"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
+@pytest.mark.asyncio
+async def test_get_table_view_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when view_id doesn't resolve to a view."""
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=None,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"view_id": 999999, "metrics": ["bookings"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
+@pytest.mark.asyncio
+async def test_get_table_invalid_filter_column_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when a filter references an unknown column."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [{"col": "bogus_col", "op": "==", "val": "x"}],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown filter column: 'bogus_col'" in data["error"]
+
+
+@pytest.mark.asyncio
+async def test_get_table_invalid_order_by_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when order_by references an unknown column/metric."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "order_by": ["bogus_order_col"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown order_by: 'bogus_order_col'" in data["error"]
+
+
+@pytest.mark.asyncio
+async def test_get_table_unknown_filter_operator_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """An operator string outside the documented set is not schema-validated.
+
+    ``GetTableFilter.op`` is a plain ``str`` field (not a Literal/Enum), so
+    the tool does not reject unrecognized operator values itself -- it
+    forwards them verbatim to the query layer, which is responsible for
+    interpreting/rejecting them.
+    """
+    mock_ds = _make_dataset(42)
+    query_result: dict[str, Any] = {
+        "queries": [
+            {
+                "data": [{"region": "west", "revenue": 100}],
+                "colnames": ["region", "revenue"],
+                "rowcount": 1,
+            }
+        ]
+    }
+
+    with (
+        patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds),
+        patch(
+            "superset.commands.chart.data.get_data_command.ChartDataCommand"
+        ) as mock_command_cls,
+        patch(
+            "superset.common.query_context_factory.QueryContextFactory"
+        ) as mock_factory_cls,
+    ):
+        mock_command_cls.return_value.run.return_value = query_result
+        mock_factory_cls.return_value.create.return_value = MagicMock()
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [
+                            {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": "x"}
+                        ],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+        create_kwargs: dict[str, Any] = (
+            mock_factory_cls.return_value.create.call_args.kwargs
+        )
+        forwarded_filters: list[dict[str, Any]] = create_kwargs["queries"][0]["filters"]
+
+    assert data["success"] is True
+    assert {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": "x"} in forwarded_filters
+
+
+@pytest.mark.asyncio
+async def test_get_table_unicode_filter_value_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """Unicode filter values are forwarded to the query layer unmodified."""
+    mock_ds = _make_dataset(42)
+    query_result: dict[str, Any] = {
+        "queries": [
+            {
+                "data": [{"region": "west", "revenue": 100}],
+                "colnames": ["region", "revenue"],
+                "rowcount": 1,
+            }
+        ]
+    }
+    unicode_val: str = "日本語 café €"
+
+    with (
+        patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds),
+        patch(
+            "superset.commands.chart.data.get_data_command.ChartDataCommand"
+        ) as mock_command_cls,
+        patch(
+            "superset.common.query_context_factory.QueryContextFactory"
+        ) as mock_factory_cls,
+    ):
+        mock_command_cls.return_value.run.return_value = query_result
+        mock_factory_cls.return_value.create.return_value = MagicMock()
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [{"col": "region", "op": "==", "val": unicode_val}],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+        create_kwargs: dict[str, Any] = (
+            mock_factory_cls.return_value.create.call_args.kwargs
+        )
+        forwarded_filters: list[dict[str, Any]] = create_kwargs["queries"][0]["filters"]
+
+    assert data["success"] is True
+    assert {"col": "region", "op": "==", "val": unicode_val} in forwarded_filters
+
+
+@pytest.mark.asyncio
+async def test_get_table_builtin_time_range_without_configured_dttm_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table rejects time_range on a builtin dataset with no main_dttm_col.
+
+    Mirrors the external-view "no datetime dimension" case, but for the
+    builtin path where the datetime column is inferred from
+    ``dataset.main_dttm_col`` instead of scanning columns.
+    """
+    mock_ds = _make_dataset(42)
+    mock_ds.main_dttm_col = None
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "time_range": "Last 7 days",
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "no temporal column is configured" in data["message"]

tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py

@@ -19,13 +19,16 @@
 
 from __future__ import annotations
 
+import contextlib
 import importlib
 from collections.abc import Generator
 from types import ModuleType
+from typing import Any
 from unittest.mock import call, MagicMock, Mock, patch
 
 import pytest
 from fastmcp import Client, FastMCP
+from fastmcp.exceptions import ToolError
 
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import SupersetSecurityException
@@ -112,18 +115,54 @@ def _access_denied_exc(message: str = "Access denied") -> SupersetSecurityExcept
     )
 
 
-@pytest.mark.asyncio
-async def test_list_metrics_builtin_happy_path(mcp_server: FastMCP) -> None:
-    """list_metrics returns builtin metrics when only datasets exist."""
-    mock_ds = _make_dataset(42)
+@contextlib.contextmanager
+def _patched_dataset_lookup(
+    dataset: MagicMock | None,
+) -> Generator[tuple[MagicMock, MagicMock], None, None]:
+    """Patch the ``dataset_id``-driven lookup path.
 
+    Covers ``DatasetDAO.find_by_id`` (direct lookup by id) plus the
+    ``SemanticViewDAO.find_accessible`` call every ``list_metrics`` request
+    makes regardless of scope.
+    """
     with (
         patch.object(list_metrics_module, "DatasetDAO") as mock_dao,
         patch.object(list_metrics_module, "SemanticViewDAO") as mock_view_dao,
     ):
-        mock_dao.find_by_id.return_value = mock_ds
+        mock_dao.find_by_id.return_value = dataset
         mock_view_dao.find_accessible.return_value = []
+        yield mock_dao, mock_view_dao
 
+
+@contextlib.contextmanager
+def _patched_dataset_search(
+    datasets: list[MagicMock],
+) -> Generator[tuple[MagicMock, MagicMock, MagicMock], None, None]:
+    """Patch the search-driven lookup path.
+
+    Covers the unscoped/searched query path: ``DatasetDAO._apply_base_filter``
+    applied to a ``db.session.query(...).options(...)`` chain, as used when no
+    ``dataset_id``/``view_id`` narrows the request.
+    """
+    with (
+        patch.object(list_metrics_module, "DatasetDAO") as mock_dao,
+        patch.object(list_metrics_module, "SemanticViewDAO") as mock_view_dao,
+        patch.object(list_metrics_module, "db") as mock_db,
+    ):
+        mock_view_dao.find_accessible.return_value = []
+        mock_query: MagicMock = MagicMock()
+        mock_db.session.query.return_value.options.return_value = mock_query
+        mock_dao._apply_base_filter.return_value = mock_query
+        mock_query.all.return_value = datasets
+        yield mock_dao, mock_view_dao, mock_db
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_builtin_happy_path(mcp_server: FastMCP) -> None:
+    """list_metrics returns builtin metrics when only datasets exist."""
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with _patched_dataset_lookup(mock_ds):
         async with Client(mcp_server) as client:
             result = await client.call_tool(
                 "list_metrics",
@@ -174,17 +213,7 @@ async def test_list_metrics_search_filter(mcp_server: FastMCP) -> None:
     """list_metrics filters metrics by search term."""
     mock_ds: MagicMock = _make_dataset(1)
 
-    with (
-        patch.object(list_metrics_module, "DatasetDAO") as mock_dao,
-        patch.object(list_metrics_module, "SemanticViewDAO") as mock_view_dao,
-        patch.object(list_metrics_module, "db") as mock_db,
-    ):
-        mock_view_dao.find_accessible.return_value = []
-        mock_query: MagicMock = MagicMock()
-        mock_db.session.query.return_value.options.return_value = mock_query
-        mock_dao._apply_base_filter.return_value = mock_query
-        mock_query.all.return_value = [mock_ds]
-
+    with _patched_dataset_search([mock_ds]):
         async with Client(mcp_server) as client:
             result = await client.call_tool(
                 "list_metrics",
@@ -280,23 +309,13 @@ async def test_list_metrics_external_per_metric_compatible_dimensions(
 @pytest.mark.asyncio
 async def test_list_metrics_pagination_is_stable(mcp_server: FastMCP) -> None:
     """Metrics are sorted deterministically before pagination is applied."""
-    mock_ds = MagicMock()
+    mock_ds: MagicMock = MagicMock()
     mock_ds.id = 1
     mock_ds.table_name = "table_1"
     mock_ds.metrics = [_make_metric("zzz_metric"), _make_metric("aaa_metric")]
     mock_ds.columns = []
 
-    with (
-        patch.object(list_metrics_module, "DatasetDAO") as mock_dao,
-        patch.object(list_metrics_module, "SemanticViewDAO") as mock_view_dao,
-        patch.object(list_metrics_module, "db") as mock_db,
-    ):
-        mock_view_dao.find_accessible.return_value = []
-        mock_query = MagicMock()
-        mock_db.session.query.return_value.options.return_value = mock_query
-        mock_dao._apply_base_filter.return_value = mock_query
-        mock_query.all.return_value = [mock_ds]
-
+    with _patched_dataset_search([mock_ds]):
         async with Client(mcp_server) as client:
             page_1 = await client.call_tool(
                 "list_metrics", {"request": {"page": 1, "page_size": 1}}
@@ -309,3 +328,194 @@ async def test_list_metrics_pagination_is_stable(mcp_server: FastMCP) -> None:
 
     assert data_1["metrics"][0]["name"] == "aaa_metric"
     assert data_2["metrics"][0]["name"] == "zzz_metric"
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_search_no_match_returns_empty(mcp_server: FastMCP) -> None:
+    """A search term that matches nothing returns an empty (not error) result."""
+    mock_ds: MagicMock = _make_dataset(1)
+
+    with _patched_dataset_search([mock_ds]):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"search": "no_such_metric_anywhere"}},
+            )
+        data: dict[str, Any] = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["metrics"] == []
+    assert data["total_count"] == 0
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_nonexistent_dataset_id_returns_empty(
+    mcp_server: FastMCP,
+) -> None:
+    """A dataset_id that doesn't resolve to a dataset returns an empty result.
+
+    The tool degrades gracefully (empty list) rather than raising NotFound,
+    since dataset_id here is a scoping filter, not a required lookup key.
+    """
+    with patch.object(list_metrics_module, "DatasetDAO") as mock_dao:
+        mock_dao.find_by_id.return_value = None
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"dataset_id": 999999}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["metrics"] == []
+    assert data["total_count"] == 0
+    mock_dao.find_by_id.assert_called_once()
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_nonexistent_view_id_returns_empty(
+    mcp_server: FastMCP,
+) -> None:
+    """A view_id that doesn't resolve to a view returns an empty result."""
+    with patch.object(list_metrics_module, "SemanticViewDAO") as mock_view_dao:
+        mock_view_dao.find_by_id.return_value = None
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"view_id": 999999}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["metrics"] == []
+    assert data["total_count"] == 0
+    mock_view_dao.find_by_id.assert_called_once()
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_search_unicode_matches(mcp_server: FastMCP) -> None:
+    """Unicode search strings match against unicode descriptions correctly."""
+    mock_ds: MagicMock = _make_dataset(1)
+    mock_ds.metrics[1].description = "café blend revenue – daily"
+
+    with _patched_dataset_search([mock_ds]):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"search": "café"}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    metrics: list[dict[str, Any]] = data["metrics"]
+    assert len(metrics) == 1
+    assert metrics[0]["name"] == "revenue"
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_search_special_characters_no_crash(
+    mcp_server: FastMCP,
+) -> None:
+    """Search strings with regex-special characters are treated as plain text."""
+    mock_ds = _make_dataset(1)
+
+    with _patched_dataset_search([mock_ds]):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"search": "rev$enue%^&*()[.*]"}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["metrics"] == []
+    assert data["total_count"] == 0
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+#
+# list_metrics hand-rolls its own pagination (list slicing) instead of using
+# ModelListCore, but the request schema still enforces page >= 1 and
+# 1 <= page_size <= 500 (superset/mcp_service/semantic_layer/schemas.py).
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_page_zero_rejected(mcp_server: FastMCP) -> None:
+    """page must be >= 1; page=0 is rejected before the tool body runs."""
+    async with Client(mcp_server) as client:
+        with pytest.raises(ToolError, match="greater than or equal to 1"):
+            await client.call_tool("list_metrics", {"request": {"page": 0}})
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_negative_page_rejected(mcp_server: FastMCP) -> None:
+    """Negative page numbers are rejected the same way as page=0."""
+    async with Client(mcp_server) as client:
+        with pytest.raises(ToolError, match="greater than or equal to 1"):
+            await client.call_tool("list_metrics", {"request": {"page": -1}})
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_page_size_zero_rejected(mcp_server: FastMCP) -> None:
+    """page_size must be >= 1; page_size=0 is rejected before the tool body
+    runs, surfacing as a structured ToolError rather than a raw 500."""
+    async with Client(mcp_server) as client:
+        with pytest.raises(ToolError, match="greater than or equal to 1"):
+            await client.call_tool("list_metrics", {"request": {"page_size": 0}})
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_page_size_over_max_rejected(mcp_server: FastMCP) -> None:
+    """page_size above the 500 ceiling is rejected, not silently clamped."""
+    async with Client(mcp_server) as client:
+        with pytest.raises(ToolError, match="less than or equal to 500"):
+            await client.call_tool("list_metrics", {"request": {"page_size": 501}})
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_page_size_at_max_accepted(mcp_server: FastMCP) -> None:
+    """page_size == 500 (the max) is accepted and echoed back."""
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with _patched_dataset_lookup(mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"dataset_id": 42, "page_size": 500}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["page_size"] == 500
+    assert data["total_count"] == 2
+
+
+@pytest.mark.asyncio
+async def test_list_metrics_page_beyond_last_page_returns_empty(
+    mcp_server: FastMCP,
+) -> None:
+    """Requesting a page past the end returns an empty page, not an error.
+
+    Unlike the ModelListCore-backed list tools, MetricList has no
+    has_next/has_previous fields — only metrics, total_count, page,
+    page_size, and total_pages.
+    """
+    mock_ds: MagicMock = _make_dataset(42)
+
+    with _patched_dataset_lookup(mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "list_metrics",
+                {"request": {"dataset_id": 42, "page": 9999, "page_size": 50}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is True
+    assert data["metrics"] == []
+    assert data["total_count"] == 2
+    assert data["page"] == 9999
+    assert data["total_pages"] == 1