aminghadersohi commented on code in PR #41924:
URL: https://github.com/apache/superset/pull/41924#discussion_r3573707631
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py:
##########
@@ -309,3 +310,236 @@ 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"
+
+
[email protected]
+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 = _make_dataset(1)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py:
##########
@@ -309,3 +310,236 @@ 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"
+
+
[email protected]
+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 = _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]
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_metrics",
+ {"request": {"search": "no_such_metric_anywhere"}},
+ )
+ data = json.loads(result.content[0].text)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py:
##########
@@ -309,3 +310,236 @@ 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"
+
+
[email protected]
+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 = _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]
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_metrics",
+ {"request": {"search": "no_such_metric_anywhere"}},
+ )
+ data = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ assert data["metrics"] == []
+ assert data["total_count"] == 0
+
+
[email protected]
+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()
+
+
[email protected]
+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()
+
+
[email protected]
+async def test_list_metrics_search_unicode_matches(mcp_server: FastMCP) ->
None:
+ """Unicode search strings match against unicode descriptions correctly."""
+ mock_ds = _make_dataset(1)
+ mock_ds.metrics[1].description = "café blend revenue – daily"
+
+ 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]
+
+ 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 = data["metrics"]
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py:
##########
@@ -309,3 +310,236 @@ 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"
+
+
[email protected]
+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 = _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]
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_metrics",
+ {"request": {"search": "no_such_metric_anywhere"}},
+ )
+ data = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ assert data["metrics"] == []
+ assert data["total_count"] == 0
+
+
[email protected]
+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()
+
+
[email protected]
+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()
+
+
[email protected]
+async def test_list_metrics_search_unicode_matches(mcp_server: FastMCP) ->
None:
+ """Unicode search strings match against unicode descriptions correctly."""
+ mock_ds = _make_dataset(1)
+ mock_ds.metrics[1].description = "café blend revenue – daily"
+
+ 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]
+
+ 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 = data["metrics"]
+ assert len(metrics) == 1
+ assert metrics[0]["name"] == "revenue"
+
+
[email protected]
+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 (
+ 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]
+
+ 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).
+# ---------------------------------------------------------------------------
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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 = _make_dataset(42)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py:
##########
@@ -309,3 +310,236 @@ 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"
+
+
[email protected]
+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 = _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]
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "list_metrics",
+ {"request": {"search": "no_such_metric_anywhere"}},
+ )
+ data = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ assert data["metrics"] == []
+ assert data["total_count"] == 0
+
+
[email protected]
+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()
+
+
[email protected]
+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()
+
+
[email protected]
+async def test_list_metrics_search_unicode_matches(mcp_server: FastMCP) ->
None:
+ """Unicode search strings match against unicode descriptions correctly."""
+ mock_ds = _make_dataset(1)
+ mock_ds.metrics[1].description = "café blend revenue – daily"
+
+ 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]
+
+ 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 = data["metrics"]
+ assert len(metrics) == 1
+ assert metrics[0]["name"] == "revenue"
+
+
[email protected]
+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 (
+ 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]
+
+ 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).
+# ---------------------------------------------------------------------------
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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}})
+
+
[email protected]
+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 = _make_dataset(42)
+
+ 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_view_dao.find_accessible.return_value = []
+
+ 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
+
+
[email protected]
+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 = _make_dataset(42)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py:
##########
@@ -278,3 +278,91 @@ async def
test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _make_dataset(42)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py:
##########
@@ -278,3 +278,91 @@ async def
test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py:
##########
@@ -278,3 +278,91 @@ async def
test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ names = {m["name"] for m in data["compatible_metrics"]}
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py:
##########
@@ -278,3 +278,91 @@ async def
test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ names = {m["name"] for m in data["compatible_metrics"]}
+ assert names == {"count", "revenue"}
+
+
[email protected]
+async def test_get_compatible_metrics_external_empty_selection(
+ mcp_server: FastMCP,
+) -> None:
+ """External views handle an explicitly empty selection without error."""
+ mock_view = _make_view(5)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py:
##########
@@ -278,3 +278,91 @@ async def
test_get_compatible_metrics_not_found(mcp_server: FastMCP) -> None:
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ names = {m["name"] for m in data["compatible_metrics"]}
+ assert names == {"count", "revenue"}
+
+
[email protected]
+async def test_get_compatible_metrics_external_empty_selection(
+ mcp_server: FastMCP,
+) -> None:
+ """External views handle an explicitly empty selection without error."""
+ mock_view = _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([], [])
+
+
[email protected]
+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 = _make_dataset(42)
Review Comment:
Added the requested explicit local type annotation in 8c643dd92d.
--
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]
