codeant-ai-for-open-source[bot] commented on code in PR #41924:
URL: https://github.com/apache/superset/pull/41924#discussion_r3561947252
##########
tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py:
##########
@@ -266,3 +267,65 @@ async def
test_list_tags_default_columns_are_id_name_type(mock_list, mcp_server)
assert "type" in tag_obj
assert "description" not in tag_obj
assert "changed_on" not in tag_obj
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListTagsRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=0)
+
+ def test_negative_page_rejected(self):
Review Comment:
**Suggestion:** Add type annotations for the method parameters and return
type so this new test method follows the enforced typing convention.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This new method is untyped, so it violates the custom rule requiring type
hints on new or modified Python functions and methods.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8fb0008b098d4f7e911e98038ebf42de&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8fb0008b098d4f7e911e98038ebf42de&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py
**Line:** 285:285
**Comment:**
*Custom Rule: Add type annotations for the method parameters and return
type so this new test method follows the enforced typing convention.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=788de975bd3979f6c95f18dec81aa4c76a1d286784a81277cfab66cff0970c7d&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=788de975bd3979f6c95f18dec81aa4c76a1d286784a81277cfab66cff0970c7d&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py:
##########
@@ -266,3 +267,65 @@ async def
test_list_tags_default_columns_are_id_name_type(mock_list, mcp_server)
assert "type" in tag_obj
assert "description" not in tag_obj
assert "changed_on" not in tag_obj
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListTagsRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
Review Comment:
**Suggestion:** Add type annotations for the method parameters and return
type so this new test method complies with the required typing rule.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly added Python method in the test file and it omits type hints
on both the parameter and return type, which matches the stated typing rule
violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ba877fddb6de4e4b8f541c41924e3ef6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ba877fddb6de4e4b8f541c41924e3ef6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py
**Line:** 281:281
**Comment:**
*Custom Rule: Add type annotations for the method parameters and return
type so this new test method complies with the required typing rule.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=f00b3200bb6f5075c798a1a65896440e4b17f363ce365a4d4366ed0c12a740f9&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=f00b3200bb6f5075c798a1a65896440e4b17f363ce365a4d4366ed0c12a740f9&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py:
##########
@@ -266,3 +267,65 @@ async def
test_list_tags_default_columns_are_id_name_type(mock_list, mcp_server)
assert "type" in tag_obj
assert "description" not in tag_obj
assert "changed_on" not in tag_obj
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListTagsRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
+ with pytest.raises(
+ ValidationError,
+ match=f"less than or equal to {MAX_PAGE_SIZE}",
+ ):
+ ListTagsRequest(page_size=MAX_PAGE_SIZE + 1)
+
+ def test_page_size_at_max_accepted(self):
+ request = ListTagsRequest(page_size=MAX_PAGE_SIZE)
+ assert request.page_size == MAX_PAGE_SIZE
+
+
[email protected]
Review Comment:
**Suggestion:** Add parameter and return type annotations to this async test
function so the new function definition is fully typed. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The async test function is newly added and lacks parameter and return type
hints, which is a direct match for the custom typing rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c18063df26cf45a0bfb2bdd585f1567e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c18063df26cf45a0bfb2bdd585f1567e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py
**Line:** 305:305
**Comment:**
*Custom Rule: Add parameter and return type annotations to this async
test function so the new function definition is fully typed.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=bef95d5373204af3be220174c51141e2ffa90ab2f147a4198db0337fb62c3e36&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=bef95d5373204af3be220174c51141e2ffa90ab2f147a4198db0337fb62c3e36&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py:
##########
@@ -266,3 +267,65 @@ async def
test_list_tags_default_columns_are_id_name_type(mock_list, mcp_server)
assert "type" in tag_obj
assert "description" not in tag_obj
assert "changed_on" not in tag_obj
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListTagsRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
Review Comment:
**Suggestion:** Add explicit typing for the method signature, including
`self` and the return type, to satisfy the mandatory type-hints rule.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is another newly added test method without type annotations, so the
typing rule is genuinely violated here.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=beb3e6cf27db40d08aaccffed8a5a532&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=beb3e6cf27db40d08aaccffed8a5a532&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py
**Line:** 293:293
**Comment:**
*Custom Rule: Add explicit typing for the method signature, including
`self` and the return type, to satisfy the mandatory type-hints rule.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=9b820e2290c96223826c5d7bc926e9994eaa3c9578719a52e06916c757ee2fc9&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=9b820e2290c96223826c5d7bc926e9994eaa3c9578719a52e06916c757ee2fc9&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/user/tool/test_user_tools.py:
##########
@@ -119,6 +120,34 @@ def test_valid_filter_column_accepted(self):
assert f.col == "username"
+class TestListUsersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
Review Comment:
**Suggestion:** Add explicit type hints to this new test method signature,
including the return type. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The rule requires type hints on modified Python functions/methods when they
can be annotated. This added test method has no return type annotation, so the
violation is real.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=14bb476c386d4da0b5c0d9bae04c097c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=14bb476c386d4da0b5c0d9bae04c097c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/user/tool/test_user_tools.py
**Line:** 127:127
**Comment:**
*Custom Rule: Add explicit type hints to this new test method
signature, including the return type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=46959e971ba56d3f13396b97e8f84c15ed182093f451ac25ed846fe37a4770f4&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=46959e971ba56d3f13396b97e8f84c15ed182093f451ac25ed846fe37a4770f4&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py:
##########
@@ -266,3 +267,65 @@ async def
test_list_tags_default_columns_are_id_name_type(mock_list, mcp_server)
assert "type" in tag_obj
assert "description" not in tag_obj
assert "changed_on" not in tag_obj
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListTagsRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListTagsRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
+ with pytest.raises(
+ ValidationError,
+ match=f"less than or equal to {MAX_PAGE_SIZE}",
+ ):
+ ListTagsRequest(page_size=MAX_PAGE_SIZE + 1)
+
+ def test_page_size_at_max_accepted(self):
+ request = ListTagsRequest(page_size=MAX_PAGE_SIZE)
+ assert request.page_size == MAX_PAGE_SIZE
+
+
[email protected]
+async def test_list_tags_invalid_page_size_surfaces_as_tool_error(mcp_server):
+ """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 0"):
+ await client.call_tool("list_tags", {"request": {"page_size": 0}})
+
+
+@patch("superset.daos.tag.TagDAO.list")
Review Comment:
**Suggestion:** Add explicit type hints for all parameters and the return
type on this newly introduced async test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly introduced async test function omits type hints for its
parameters and return type, so it violates the required Python typing rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b3bb38a05ee04e94baf1a4888a4a145d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b3bb38a05ee04e94baf1a4888a4a145d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/tag/tool/test_tag_tools.py
**Line:** 314:314
**Comment:**
*Custom Rule: Add explicit type hints for all parameters and the return
type on this newly introduced async test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=902836c5b1c9930674bbafb196ebc7f1e93313c77b566f36f1e081c2a4b715f6&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=902836c5b1c9930674bbafb196ebc7f1e93313c77b566f36f1e081c2a4b715f6&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/user/tool/test_user_tools.py:
##########
@@ -290,6 +319,35 @@ async def
test_list_users_search_and_filters_mutually_exclusive(mcp_server):
)
[email protected]
+async def test_list_users_invalid_page_size_surfaces_as_tool_error(mcp_server):
Review Comment:
**Suggestion:** Add parameter and return type annotations to this newly
added async test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This new async test function omits type annotations for its parameter and
return type, which violates the Python type-hint requirement.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=01c746c1e2874174bad07dfbe95f96bd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=01c746c1e2874174bad07dfbe95f96bd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/user/tool/test_user_tools.py
**Line:** 323:323
**Comment:**
*Custom Rule: Add parameter and return type annotations to this newly
added async test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=8a07ab294d5dbf21d468efa5a337c8e6eb0b377f7e41a61592e19b8d2d21e60b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=8a07ab294d5dbf21d468efa5a337c8e6eb0b377f7e41a61592e19b8d2d21e60b&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/database/tool/test_database_tools.py:
##########
@@ -378,3 +379,69 @@ async def
test_list_databases_does_not_expose_sensitive_credential_columns(
# Verify the exploit path: DAO must never receive sensitive column names.
dao_columns = mock_list.call_args.kwargs["columns"]
assert not sensitive.intersection(dao_columns)
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListDatabasesRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
Review Comment:
**Suggestion:** Add explicit type hints to this new test method signature,
including a return annotation. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added test method omits type hints on both the instance parameter
and the return type, which violates the Python type-hint requirement for new or
modified code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=748bfd2900294ada9a45e68b77748441&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=748bfd2900294ada9a45e68b77748441&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/database/tool/test_database_tools.py
**Line:** 393:393
**Comment:**
*Custom Rule: Add explicit type hints to this new test method
signature, including a return annotation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=46b633b6fd0b252d45415d49cc063b1122e4cc36dd7b66658b38719c2d969de2&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=46b633b6fd0b252d45415d49cc063b1122e4cc36dd7b66658b38719c2d969de2&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py:
##########
@@ -272,3 +274,69 @@ async def
test_get_rls_filter_info_guest_denied(mcp_server):
data = json.loads(result.content[0].text)
assert data["error_type"] == "Forbidden"
assert "guest" in data["error"].lower()
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListRlsFiltersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
Review Comment:
**Suggestion:** Add a return type annotation to this method to align with
the mandatory type-hint rule for new Python code. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This added test method omits a return type annotation, which is a direct
violation of the type-hint rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eaa18e585551486fa4f9034d79f1eb7b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eaa18e585551486fa4f9034d79f1eb7b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
**Line:** 296:296
**Comment:**
*Custom Rule: Add a return type annotation to this method to align with
the mandatory type-hint rule for new Python code.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7e13e8fa74db6aaa9fb10ec1511b88d63e31e55a2fa238a505eb72fc6dbb16cf&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7e13e8fa74db6aaa9fb10ec1511b88d63e31e55a2fa238a505eb72fc6dbb16cf&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py:
##########
@@ -272,3 +274,69 @@ async def
test_get_rls_filter_info_guest_denied(mcp_server):
data = json.loads(result.content[0].text)
assert data["error_type"] == "Forbidden"
assert "guest" in data["error"].lower()
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListRlsFiltersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
+ with pytest.raises(
+ ValidationError,
+ match=f"less than or equal to {MAX_PAGE_SIZE}",
+ ):
+ ListRlsFiltersRequest(page_size=MAX_PAGE_SIZE + 1)
+
+ def test_page_size_at_max_accepted(self):
+ request = ListRlsFiltersRequest(page_size=MAX_PAGE_SIZE)
+ assert request.page_size == MAX_PAGE_SIZE
+
+
[email protected]
+async def
test_list_rls_filters_invalid_page_size_surfaces_as_tool_error(mcp_server):
+ """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 0"):
+ await client.call_tool("list_rls_filters", {"request":
{"page_size": 0}})
+
+
+@patch("superset.daos.security.RLSDAO.list")
[email protected]
+async def test_list_rls_filters_page_beyond_last_page_returns_empty(
+ mock_list, mcp_server
+):
Review Comment:
**Suggestion:** Add type hints for both parameters and an explicit return
type on this async test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added async test omits parameter type hints and a return type
annotation, which is exactly the kind of omission the rule flags.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7edea4e833c249baae742a8c9ce2bc80&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7edea4e833c249baae742a8c9ce2bc80&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
**Line:** 323:325
**Comment:**
*Custom Rule: Add type hints for both parameters and an explicit return
type on this async test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=45a75c2157ab309793d91512589dc55eb444f45fe8452f6e750ae3fc71c1f192&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=45a75c2157ab309793d91512589dc55eb444f45fe8452f6e750ae3fc71c1f192&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py:
##########
@@ -272,3 +274,69 @@ async def
test_get_rls_filter_info_guest_denied(mcp_server):
data = json.loads(result.content[0].text)
assert data["error_type"] == "Forbidden"
assert "guest" in data["error"].lower()
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListRlsFiltersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
Review Comment:
**Suggestion:** Add a return type hint to this test method so it complies
with the project rule requiring function annotations. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added test method has no return type annotation, which violates
the stated Python type-hint requirement for new or modified code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2453c197c57745cb9e44457d666cc56e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2453c197c57745cb9e44457d666cc56e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
**Line:** 288:288
**Comment:**
*Custom Rule: Add a return type hint to this test method so it complies
with the project rule requiring function annotations.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=bed30b6c6bf6e49bf359cc0e14e3c2a6a16da98c52b1e0428d15a66c56f1c48a&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=bed30b6c6bf6e49bf359cc0e14e3c2a6a16da98c52b1e0428d15a66c56f1c48a&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py:
##########
@@ -272,3 +274,69 @@ async def
test_get_rls_filter_info_guest_denied(mcp_server):
data = json.loads(result.content[0].text)
assert data["error_type"] == "Forbidden"
assert "guest" in data["error"].lower()
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListRlsFiltersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=0)
+
+ def test_negative_page_rejected(self):
Review Comment:
**Suggestion:** Add a return type hint to this test method to satisfy the
type-hint requirement for newly added Python functions. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This test method is newly added and lacks a return type hint, so it matches
the custom rule requiring annotations on Python functions.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8deaebf4f030491fbff56afa99bfdc19&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8deaebf4f030491fbff56afa99bfdc19&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
**Line:** 292:292
**Comment:**
*Custom Rule: Add a return type hint to this test method to satisfy the
type-hint requirement for newly added Python functions.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=3484586a8f0127a7906883644fd03a239dc8d2dde975428cde9e585da6d7b60e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=3484586a8f0127a7906883644fd03a239dc8d2dde975428cde9e585da6d7b60e&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/database/tool/test_database_tools.py:
##########
@@ -378,3 +379,69 @@ async def
test_list_databases_does_not_expose_sensitive_credential_columns(
# Verify the exploit path: DAO must never receive sensitive column names.
dao_columns = mock_list.call_args.kwargs["columns"]
assert not sensitive.intersection(dao_columns)
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListDatabasesRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListDatabasesRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListDatabasesRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListDatabasesRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
+ with pytest.raises(
+ ValidationError,
+ match=f"less than or equal to {MAX_PAGE_SIZE}",
+ ):
+ ListDatabasesRequest(page_size=MAX_PAGE_SIZE + 1)
+
+ def test_page_size_at_max_accepted(self):
+ request = ListDatabasesRequest(page_size=MAX_PAGE_SIZE)
+ assert request.page_size == MAX_PAGE_SIZE
+
+
[email protected]
+async def
test_list_databases_invalid_page_size_surfaces_as_tool_error(mcp_server):
Review Comment:
**Suggestion:** Add type annotations for the fixture parameter and return
type on this new async test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This async test function is newly added and lacks type annotations for its
fixture parameter and return type, matching the type-hint rule violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=80cdbd2897b740d2baf6bbc9bfb86ff6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=80cdbd2897b740d2baf6bbc9bfb86ff6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/database/tool/test_database_tools.py
**Line:** 418:418
**Comment:**
*Custom Rule: Add type annotations for the fixture parameter and return
type on this new async test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7419dd7654fa2eeb192aa9b43b956ebb9f3e7c98fa9120dba56c22120e02be71&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7419dd7654fa2eeb192aa9b43b956ebb9f3e7c98fa9120dba56c22120e02be71&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py:
##########
@@ -272,3 +274,69 @@ async def
test_get_rls_filter_info_guest_denied(mcp_server):
data = json.loads(result.content[0].text)
assert data["error_type"] == "Forbidden"
assert "guest" in data["error"].lower()
+
+
+# ---------------------------------------------------------------------------
+# Pagination edge cases
+# ---------------------------------------------------------------------------
+
+
+class TestListRlsFiltersRequestPagination:
+ """Schema-level pagination boundary tests โ ``page`` is PositiveInt and
+ ``page_size`` is constrained to (0, MAX_PAGE_SIZE]."""
+
+ def test_page_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=0)
+
+ def test_negative_page_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page=-1)
+
+ def test_page_size_zero_rejected(self):
+ with pytest.raises(ValidationError, match="greater than 0"):
+ ListRlsFiltersRequest(page_size=0)
+
+ def test_page_size_over_max_rejected(self):
+ with pytest.raises(
+ ValidationError,
+ match=f"less than or equal to {MAX_PAGE_SIZE}",
+ ):
+ ListRlsFiltersRequest(page_size=MAX_PAGE_SIZE + 1)
+
+ def test_page_size_at_max_accepted(self):
+ request = ListRlsFiltersRequest(page_size=MAX_PAGE_SIZE)
+ assert request.page_size == MAX_PAGE_SIZE
+
+
[email protected]
+async def
test_list_rls_filters_invalid_page_size_surfaces_as_tool_error(mcp_server):
+ """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 0"):
+ await client.call_tool("list_rls_filters", {"request":
{"page_size": 0}})
Review Comment:
**Suggestion:** Add type hints for the fixture parameter and explicit return
type on this async test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This async test function has an untyped fixture parameter and no return type
annotation, so it violates the Python type-hint requirement.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e611549944044324926b2c5569e7f51e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e611549944044324926b2c5569e7f51e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/rls/tool/test_rls_tools.py
**Line:** 313:318
**Comment:**
*Custom Rule: Add type hints for the fixture parameter and explicit
return type on this async test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=31e89476c4f17c54977322c83e8411290a00118cd4090adda5ee167b71dbdfdd&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=31e89476c4f17c54977322c83e8411290a00118cd4090adda5ee167b71dbdfdd&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py:
##########
@@ -282,3 +282,91 @@ async def
test_get_compatible_dimensions_external_not_found(
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _make_dataset(42)
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
local variable to satisfy the type-hinting requirement for relevant variables.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is newly added Python code, and `mock_ds` is a local variable that
could be type-annotated. The custom rule requires type hints on relevant
variables in new or modified code, so the omission is a real violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=776e004cf89b4d3e984e6f897b2670fb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=776e004cf89b4d3e984e6f897b2670fb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py
**Line:** 297:297
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
local variable to satisfy the type-hinting requirement for relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=8af0179aa0e05e8e5775492e6ebe62e31ab454eca58f81ed15e0e445455a5afb&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=8af0179aa0e05e8e5775492e6ebe62e31ab454eca58f81ed15e0e445455a5afb&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add a concrete type annotation for this parsed JSON payload
variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This added local variable is a Python value whose type could be annotated,
but no type hint is present. The suggestion correctly identifies a type-hint
omission.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2a6305616bff4645819b171ad550aa3e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2a6305616bff4645819b171ad550aa3e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py
**Line:** 336:336
**Comment:**
*Custom Rule: Add a concrete type annotation for this parsed JSON
payload variable.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=e10385db8667d27e5006ccc507a087009da089de4692e9e8574f210ea26ffa56&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=e10385db8667d27e5006ccc507a087009da089de4692e9e8574f210ea26ffa56&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py:
##########
@@ -282,3 +282,91 @@ async def
test_get_compatible_dimensions_external_not_found(
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ names = {d["name"] for d in data["compatible_dimensions"]}
+ assert names == {"region", "category"}
+
+
[email protected]
+async def test_get_compatible_dimensions_external_empty_selection(
+ mcp_server: FastMCP,
+) -> None:
+ """External views handle an explicitly empty selection without error."""
+ mock_view = _make_view(5)
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
mocked object variable to satisfy required typing standards. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a new Python local variable assignment in the added test code, and
the type-hint rule applies to variables that can be annotated. The omission is
therefore a real violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f658795fc5c14d09b2fc97bb83782dae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f658795fc5c14d09b2fc97bb83782dae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py
**Line:** 323:323
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
mocked object variable to satisfy required typing standards.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=af6448b66bed26186856235ac5e25f0392e1831f84efb243b40bd445a64ea7ee&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=af6448b66bed26186856235ac5e25f0392e1831f84efb243b40bd445a64ea7ee&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py:
##########
@@ -282,3 +282,91 @@ async def
test_get_compatible_dimensions_external_not_found(
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
+
+ assert data["success"] is True
+ names = {d["name"] for d in data["compatible_dimensions"]}
Review Comment:
**Suggestion:** Provide an explicit type annotation for this newly added
collection variable to comply with the type-hint rule. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added local variable is inferable and can be annotated, so it
falls under the requirement to add type hints to relevant variables in new
Python code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9b0a3efea56142f0a5a15dabb3a79e3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9b0a3efea56142f0a5a15dabb3a79e3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py
**Line:** 314:314
**Comment:**
*Custom Rule: Provide an explicit type annotation for this newly added
collection variable to comply with the type-hint rule.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=30925cb953b295b44f6f85503863f6b531628b228af44c8efcc85227e54d5dee&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=30925cb953b295b44f6f85503863f6b531628b228af44c8efcc85227e54d5dee&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Annotate this newly added mock dataset local with an
explicit type. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a new Python local variable with an inferable type and no
annotation. The omission fits the stated type-hint rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e920cd5e551547cdb80832ad8a830c92&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e920cd5e551547cdb80832ad8a830c92&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py
**Line:** 494:494
**Comment:**
*Custom Rule: Annotate this newly added mock dataset local with an
explicit type.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7f73f0fea5cc1f04b7791696cae2dad75c45eedfe5448da4493b96ce46d8ce60&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=7f73f0fea5cc1f04b7791696cae2dad75c45eedfe5448da4493b96ce46d8ce60&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit local type annotation for this dataset mock
variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly added local variable in Python code and its type is
inferable, but it has no explicit annotation. That matches the type-hint rule
for relevant variables.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4cbbb56e9f22454e85ce29e90f14d4a7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4cbbb56e9f22454e85ce29e90f14d4a7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py
**Line:** 318:318
**Comment:**
*Custom Rule: Add an explicit local type annotation for this dataset
mock variable.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=c3339b78e8a59bcb1faf46efde847c3a9f910931cf0e63e9d96b5c81b3a1d903&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=c3339b78e8a59bcb1faf46efde847c3a9f910931cf0e63e9d96b5c81b3a1d903&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit type annotation for this derived metrics
collection variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly introduced local collection variable is inferable and could be
annotated, but the code does not include a type hint. That is a real violation
of the type-hint rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=92da6340056b45948745c2883c16dbdd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=92da6340056b45948745c2883c16dbdd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py
**Line:** 414:414
**Comment:**
*Custom Rule: Add an explicit type annotation for this derived metrics
collection variable.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b123316b913080a30cf9037f303f91affb870ea54712ded964175c45b93e415c&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b123316b913080a30cf9037f303f91affb870ea54712ded964175c45b93e415c&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit local type annotation for this newly
introduced mock dataset variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This added local variable is inferable from the call result and lacks an
explicit type annotation, so the type-hint rule applies.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c7ceeff5909f4e159b2d6ab7b615f441&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c7ceeff5909f4e159b2d6ab7b615f441&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py
**Line:** 525:525
**Comment:**
*Custom Rule: Add an explicit local type annotation for this newly
introduced mock dataset variable.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b36138f5f86ffe9c89d03ce6c02e98e3a03d56a0f7ee63b992fbe4e65d427ffd&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b36138f5f86ffe9c89d03ce6c02e98e3a03d56a0f7ee63b992fbe4e65d427ffd&reaction=dislike'>๐</a>
##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py:
##########
@@ -282,3 +282,91 @@ async def
test_get_compatible_dimensions_external_not_found(
assert data["success"] is False
assert data["error_type"] == "NotFound"
+
+
[email protected]
+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 = _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 = json.loads(result.content[0].text)
Review Comment:
**Suggestion:** Add a type annotation for this parsed payload variable so
the new code follows the required type-hint coverage. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly introduced local variable in Python code and can be
annotated. Under the type-hint rule, omitting a type annotation here is a valid
violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c3a9771390fa4592bd2bf2eb43fc606d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c3a9771390fa4592bd2bf2eb43fc606d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py
**Line:** 311:311
**Comment:**
*Custom Rule: Add a type annotation for this parsed payload variable so
the new code follows the required type-hint coverage.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b432e90657b42685fe4de73243ef2b0467dd78be60850efeddd65e5e91d916ab&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41924&comment_hash=b432e90657b42685fe4de73243ef2b0467dd78be60850efeddd65e5e91d916ab&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
