codeant-ai-for-open-source[bot] commented on code in PR #41922: URL: https://github.com/apache/superset/pull/41922#discussion_r3589116801
########## tests/unit_tests/mcp_service/system/resources/test_schema_discovery.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for the schema discovery MCP resources (superset://schema/*).""" + +from unittest.mock import Mock, patch + +import pytest +from fastmcp import Client +from fastmcp.exceptions import McpError + +from superset.mcp_service.app import mcp +from superset.mcp_service.system.resources.schema_discovery import ( + _build_schema_resource, +) +from superset.utils import json + +RESOURCE_URIS = { + "chart": "superset://schema/chart", + "dataset": "superset://schema/dataset", + "dashboard": "superset://schema/dashboard", + "all": "superset://schema/all", +} + + [email protected] +def mcp_server(): Review Comment: **Suggestion:** Add explicit type hints for this fixture's return type to comply with the typing requirement for new Python functions. [custom_rule] **Severity Level:** Minor ๐งน <details> <summary><b>Why it matters? โญ </b></summary> The new fixture `mcp_server` has no return type annotation, which violates the Python type-hints rule for new or modified 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=23dcd231255c47f282cfa7f1a500601b&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=23dcd231255c47f282cfa7f1a500601b&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/system/resources/test_schema_discovery.py **Line:** 40:41 **Comment:** *Custom Rule: Add explicit type hints for this fixture's return type to comply with the typing requirement for new 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%2F41922&comment_hash=6c0bf0bc653afb1032cc756b43911d2f2f54b01a8f669130e45a4b905f92556a&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41922&comment_hash=6c0bf0bc653afb1032cc756b43911d2f2f54b01a8f669130e45a4b905f92556a&reaction=dislike'>๐</a> ########## tests/unit_tests/mcp_service/system/resources/test_schema_discovery.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for the schema discovery MCP resources (superset://schema/*).""" + +from unittest.mock import Mock, patch + +import pytest +from fastmcp import Client +from fastmcp.exceptions import McpError + +from superset.mcp_service.app import mcp +from superset.mcp_service.system.resources.schema_discovery import ( + _build_schema_resource, +) +from superset.utils import json + +RESOURCE_URIS = { + "chart": "superset://schema/chart", + "dataset": "superset://schema/dataset", + "dashboard": "superset://schema/dashboard", + "all": "superset://schema/all", +} + + [email protected] +def mcp_server(): + return mcp + + [email protected](autouse=True) +def mock_auth(): Review Comment: **Suggestion:** Add a return type annotation for this fixture generator so the function signature is fully typed. [custom_rule] **Severity Level:** Minor ๐งน <details> <summary><b>Why it matters? โญ </b></summary> The autouse fixture `mock_auth` is a generator fixture but its signature has no return type annotation, so it violates the type-hints 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=47d9b9ec296a4b959bbe8ab929c9cb25&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=47d9b9ec296a4b959bbe8ab929c9cb25&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/system/resources/test_schema_discovery.py **Line:** 45:46 **Comment:** *Custom Rule: Add a return type annotation for this fixture generator so the function signature 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%2F41922&comment_hash=b9fa3124ee0edaf48bcf8bc2fd584252ac6e05499ea2d940bf3d193ec74f5565&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41922&comment_hash=b9fa3124ee0edaf48bcf8bc2fd584252ac6e05499ea2d940bf3d193ec74f5565&reaction=dislike'>๐</a> ########## tests/unit_tests/mcp_service/system/resources/test_schema_discovery.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for the schema discovery MCP resources (superset://schema/*).""" + +from unittest.mock import Mock, patch + +import pytest +from fastmcp import Client +from fastmcp.exceptions import McpError + +from superset.mcp_service.app import mcp +from superset.mcp_service.system.resources.schema_discovery import ( + _build_schema_resource, +) +from superset.utils import json + +RESOURCE_URIS = { + "chart": "superset://schema/chart", + "dataset": "superset://schema/dataset", + "dashboard": "superset://schema/dashboard", + "all": "superset://schema/all", +} + + [email protected] +def mcp_server(): + return mcp + + [email protected](autouse=True) +def mock_auth(): + """Mock authentication for all tests.""" + with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user: + mock_user = Mock() + mock_user.id = 1 + mock_user.username = "admin" + mock_get_user.return_value = mock_user + yield mock_get_user + + +async def _read_resource(mcp_server, uri): + async with Client(mcp_server) as client: + result = await client.read_resource(uri) + return json.loads(result[0].text) + + +class TestSchemaDiscoveryResourceRegistration: + """URI pattern matching: resources are discoverable under superset://schema/*.""" + + @pytest.mark.asyncio + async def test_all_schema_resources_registered(self, mcp_server): + async with Client(mcp_server) as client: + resources = await client.list_resources() + + resource_uris = {str(resource.uri) for resource in resources} + for uri in RESOURCE_URIS.values(): + assert uri in resource_uris + + +class TestSchemaDiscoveryResourceSuccessPaths: + """Success paths for each model-specific and combined schema resource.""" + + @pytest.mark.asyncio + async def test_chart_schema_resource(self, mcp_server): Review Comment: **Suggestion:** Add explicit annotations for this async test method signature, including fixture parameter type and return type. [custom_rule] **Severity Level:** Minor ๐งน <details> <summary><b>Why it matters? โญ </b></summary> This new async test method has no type annotations on the fixture parameter or return type, so it violates the Python type-hints 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=f4f39b7b308c4e1a81bc13cc2e287114&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=f4f39b7b308c4e1a81bc13cc2e287114&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/system/resources/test_schema_discovery.py **Line:** 78:79 **Comment:** *Custom Rule: Add explicit annotations for this async test method signature, including fixture parameter type and 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%2F41922&comment_hash=747b1901e552504d4db6b96feeb8bf929b99c9e744030af048f6dae71bd062c6&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41922&comment_hash=747b1901e552504d4db6b96feeb8bf929b99c9e744030af048f6dae71bd062c6&reaction=dislike'>๐</a> ########## tests/unit_tests/mcp_service/system/resources/test_schema_discovery.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for the schema discovery MCP resources (superset://schema/*).""" + +from unittest.mock import Mock, patch + +import pytest +from fastmcp import Client +from fastmcp.exceptions import McpError + +from superset.mcp_service.app import mcp +from superset.mcp_service.system.resources.schema_discovery import ( + _build_schema_resource, +) +from superset.utils import json + +RESOURCE_URIS = { + "chart": "superset://schema/chart", + "dataset": "superset://schema/dataset", + "dashboard": "superset://schema/dashboard", + "all": "superset://schema/all", +} + + [email protected] +def mcp_server(): + return mcp + + [email protected](autouse=True) +def mock_auth(): + """Mock authentication for all tests.""" + with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user: + mock_user = Mock() + mock_user.id = 1 + mock_user.username = "admin" + mock_get_user.return_value = mock_user + yield mock_get_user + + +async def _read_resource(mcp_server, uri): Review Comment: **Suggestion:** Add parameter and return type hints to this async helper function, including the server and URI inputs and parsed JSON output. [custom_rule] **Severity Level:** Minor ๐งน <details> <summary><b>Why it matters? โญ </b></summary> The async helper `_read_resource` omits parameter and return type hints even though they are straightforwardly annotatable, so this 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=7f5d7dcdbd4d49f9a32a90bb46a0af41&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=7f5d7dcdbd4d49f9a32a90bb46a0af41&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/system/resources/test_schema_discovery.py **Line:** 56:56 **Comment:** *Custom Rule: Add parameter and return type hints to this async helper function, including the server and URI inputs and parsed JSON output. 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%2F41922&comment_hash=ddc45d8b84f56fe3da7ddaf31e1f777153bc6e051e950b82aa92a8f2c8bcca1e&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41922&comment_hash=ddc45d8b84f56fe3da7ddaf31e1f777153bc6e051e950b82aa92a8f2c8bcca1e&reaction=dislike'>๐</a> ########## tests/unit_tests/mcp_service/system/resources/test_schema_discovery.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for the schema discovery MCP resources (superset://schema/*).""" + +from unittest.mock import Mock, patch + +import pytest +from fastmcp import Client +from fastmcp.exceptions import McpError + +from superset.mcp_service.app import mcp +from superset.mcp_service.system.resources.schema_discovery import ( + _build_schema_resource, +) +from superset.utils import json + +RESOURCE_URIS = { + "chart": "superset://schema/chart", + "dataset": "superset://schema/dataset", + "dashboard": "superset://schema/dashboard", + "all": "superset://schema/all", +} + + [email protected] +def mcp_server(): + return mcp + + [email protected](autouse=True) +def mock_auth(): + """Mock authentication for all tests.""" + with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user: + mock_user = Mock() + mock_user.id = 1 + mock_user.username = "admin" + mock_get_user.return_value = mock_user + yield mock_get_user + + +async def _read_resource(mcp_server, uri): + async with Client(mcp_server) as client: + result = await client.read_resource(uri) + return json.loads(result[0].text) + + +class TestSchemaDiscoveryResourceRegistration: + """URI pattern matching: resources are discoverable under superset://schema/*.""" + + @pytest.mark.asyncio + async def test_all_schema_resources_registered(self, mcp_server): Review Comment: **Suggestion:** Add type hints for this async test method parameters and annotate the return type to satisfy the mandatory typing rule. [custom_rule] **Severity Level:** Minor ๐งน <details> <summary><b>Why it matters? โญ </b></summary> The async test method omits type hints for its fixture parameter and return type, which violates the stated typing rule for 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=51d09026690942fbbceb9ee7a013b605&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=51d09026690942fbbceb9ee7a013b605&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/system/resources/test_schema_discovery.py **Line:** 65:66 **Comment:** *Custom Rule: Add type hints for this async test method parameters and annotate the return type to satisfy the mandatory 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%2F41922&comment_hash=32dc2a00a81add7b0f2888144f421351f317b98957431560350261086dce8dbf&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41922&comment_hash=32dc2a00a81add7b0f2888144f421351f317b98957431560350261086dce8dbf&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]
